From 640a338cf422cff7c2516d3d40ffedbde98db811 Mon Sep 17 00:00:00 2001 From: Camden Cheek Date: Tue, 20 Jun 2023 15:14:29 -0600 Subject: [PATCH] Settings: make DB calls sequentially (#53803) We're getting a [concurrent transaction error](https://sourcegraph.sentry.io/issues/4263654659/?project=6583153&referrer=slack) from settings because somewhere we're passing a transaction as the `database.DB` then using it concurrently in this method. This updates the DB calls to run sequentially. This comes with a perf penalty, but I'd prefer a small perf impact to erroring/crashing/silently doing the wrong thing. As a followup, I will add a DB method that allows you to request settings for multiple subjects at once. That might not come until after 5.1 though, so I'd at least like to get this merged and backported. --- internal/settings/BUILD.bazel | 1 - internal/settings/settings.go | 11 +++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/internal/settings/BUILD.bazel b/internal/settings/BUILD.bazel index 6c474295d21..6a617badb7c 100644 --- a/internal/settings/BUILD.bazel +++ b/internal/settings/BUILD.bazel @@ -14,7 +14,6 @@ go_library( "//internal/trace", "//lib/errors", "//schema", - "@com_github_sourcegraph_conc//iter", ], ) diff --git a/internal/settings/settings.go b/internal/settings/settings.go index 3b79894d066..9a44a1a5dc6 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -5,7 +5,6 @@ import ( "reflect" "sort" - "github.com/sourcegraph/conc/iter" "github.com/sourcegraph/sourcegraph/lib/errors" "github.com/sourcegraph/sourcegraph/internal/actor" @@ -84,9 +83,13 @@ func (s *service) ForSubject(ctx context.Context, subject api.SettingsSubject) ( return nil, err } - allSettings, err := iter.MapErr(subjects, func(subject *api.SettingsSubject) (*schema.Settings, error) { - return latest(ctx, s.db, *subject) - }) + allSettings := make([]*schema.Settings, len(subjects)) + for i, subject := range subjects { + allSettings[i], err = latest(ctx, s.db, subject) + if err != nil { + return nil, err + } + } return mergeSettings(allSettings...), nil }