From 50ec75d17b00c9cc598ab45f6d62731dd675354f Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Fri, 16 Jul 2021 11:23:09 -0500 Subject: [PATCH 1/2] Update soft delete query. --- .../codeintel/stores/dbstore/uploads.go | 46 +++++++-- .../codeintel/stores/dbstore/uploads_test.go | 97 +++++++++++++------ internal/database/schema.md | 4 +- ...528395852_lsif_add_covering_index.down.sql | 9 ++ .../1528395852_lsif_add_covering_index.up.sql | 9 ++ 5 files changed, 127 insertions(+), 38 deletions(-) create mode 100755 migrations/frontend/1528395852_lsif_add_covering_index.down.sql create mode 100755 migrations/frontend/1528395852_lsif_add_covering_index.up.sql diff --git a/enterprise/internal/codeintel/stores/dbstore/uploads.go b/enterprise/internal/codeintel/stores/dbstore/uploads.go index 7c4a9b38d8ce..c68efb4cdb68 100644 --- a/enterprise/internal/codeintel/stores/dbstore/uploads.go +++ b/enterprise/internal/codeintel/stores/dbstore/uploads.go @@ -660,7 +660,7 @@ func (s *Store) SoftDeleteOldUploads(ctx context.Context, maxAge time.Duration, defer func() { err = tx.Done(err) }() seconds := strconv.Itoa(int(maxAge / time.Second)) - repositories, err := scanCounts(tx.Store.Query(ctx, sqlf.Sprintf(softDeleteOldUploadsQuery, now, seconds, now, seconds))) + repositories, err := scanCounts(tx.Store.Query(ctx, sqlf.Sprintf(softDeleteOldUploadsQuery, now, seconds))) if err != nil { return 0, err } @@ -684,19 +684,47 @@ func (s *Store) SoftDeleteOldUploads(ctx context.Context, maxAge time.Duration, const softDeleteOldUploadsQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/uploads.go:SoftDeleteOldUploads -WITH u AS ( +WITH RECURSIVE +protected_uploads AS ( + ( + -- Base case: select all upload records that are yonger than the configured + -- retention age, as well as all upload records visible from a non-stale + -- branch or tag. These form the roots of our dependency graph traversal. + + SELECT u.id FROM lsif_uploads u + WHERE %s - COALESCE(u.finished_at, u.uploaded_at) <= (%s || ' second')::interval + UNION + SELECT upload_id as id FROM lsif_uploads_visible_at_tip + ) UNION ( + -- Iterative case: expand the working set of protected uploads by traversing + -- the dependency graph: select all upload records that define an LSIF package + -- that is referenced by an upload already in the working set. We skip any + -- self-imports here, which may occur on some older Sourcegraph instances. + + SELECT p.dump_id as id + FROM protected_uploads pu + JOIN lsif_references r ON r.dump_id = pu.id + JOIN lsif_packages p ON p.scheme = r.scheme AND p.name = r.name AND p.version = r.version AND p.dump_id != r.dump_id + ) +), +candidates AS ( + -- Find the inverse of protected_uploads, which contains each upload record + -- that is older than the configured retention age and is not reachable via + -- the dependencies of any upload in protected_uploads. We also order the + -- candidates here to try to acquire the locks in the following update in + -- a determinstic order so that we do not deadlock with another query updating + -- overlapping records. + + (SELECT id FROM lsif_uploads EXCEPT SELECT id FROM protected_uploads) ORDER BY id +), +updated AS ( UPDATE lsif_uploads u SET state = 'deleted' WHERE - ( - %s - u.finished_at > (%s || ' second')::interval OR - (u.finished_at IS NULL AND %s - u.uploaded_at > (%s || ' second')::interval) - ) AND - -- Anything visible from a non-stale branch or tag is protected from expiration - u.id NOT IN (SELECT uvt.upload_id FROM lsif_uploads_visible_at_tip uvt WHERE uvt.repository_id = u.repository_id) + u.id IN (SELECT id FROM candidates) RETURNING id, repository_id ) -SELECT u.repository_id, count(*) FROM u GROUP BY u.repository_id +SELECT u.repository_id, count(*) FROM updated u GROUP BY u.repository_id ` // GetOldestCommitDate returns the oldest commit date for all uploads for the given repository. If there are no diff --git a/enterprise/internal/codeintel/stores/dbstore/uploads_test.go b/enterprise/internal/codeintel/stores/dbstore/uploads_test.go index 5d3943c5c085..079296f7391d 100644 --- a/enterprise/internal/codeintel/stores/dbstore/uploads_test.go +++ b/enterprise/internal/codeintel/stores/dbstore/uploads_test.go @@ -13,6 +13,7 @@ import ( "github.com/sourcegraph/sourcegraph/cmd/frontend/globals" "github.com/sourcegraph/sourcegraph/internal/database/basestore" "github.com/sourcegraph/sourcegraph/internal/database/dbtesting" + "github.com/sourcegraph/sourcegraph/lib/codeintel/semantic" "github.com/sourcegraph/sourcegraph/schema" ) @@ -827,42 +828,84 @@ func TestSoftDeleteOldUploads(t *testing.T) { t3 := t1.Add(time.Minute * 4) t4 := t1.Add(time.Minute * 6) - insertUploads(t, db, - Upload{ID: 1, State: "completed", FinishedAt: &t1}, - Upload{ID: 2, State: "completed", FinishedAt: &t2}, // visible - Upload{ID: 3, State: "errored", FinishedAt: &t2}, - Upload{ID: 4, State: "completed", FinishedAt: &t3}, // visible - Upload{ID: 5, State: "completed", FinishedAt: &t3}, - Upload{ID: 6, State: "completed", FinishedAt: &t4}, // too new - Upload{ID: 7, State: "errored", FinishedAt: &t4}, // too new - Upload{ID: 8, State: "uploaded", UploadedAt: t3}, - Upload{ID: 9, State: "uploaded", UploadedAt: t4}, // too new - Upload{ID: 10, State: "completed", FinishedAt: &t2}, // protected on non-default branch - ) - insertVisibleAtTip(t, db, 50, 2, 4) - insertVisibleAtTipNonDefaultBranch(t, db, 50, 10) + tests := []struct { + upload Upload + expectedState string + }{ + // too new + {upload: Upload{ID: 11, State: "uploaded", UploadedAt: t4}, expectedState: "uploaded"}, + {upload: Upload{ID: 12, State: "errored", FinishedAt: &t4}, expectedState: "errored"}, + {upload: Upload{ID: 13, State: "completed", FinishedAt: &t4}, expectedState: "completed"}, + + // protected on a branch + {upload: Upload{ID: 14, State: "completed", FinishedAt: &t2}, expectedState: "completed"}, + {upload: Upload{ID: 15, State: "completed", FinishedAt: &t3}, expectedState: "completed"}, + {upload: Upload{ID: 16, State: "completed", FinishedAt: &t2}, expectedState: "completed"}, + + // old and only reachable from other deletion candidates + {upload: Upload{ID: 17, State: "uploaded", UploadedAt: t3}, expectedState: "deleted"}, + {upload: Upload{ID: 18, State: "errored", FinishedAt: &t2}, expectedState: "deleted"}, + {upload: Upload{ID: 19, State: "completed", FinishedAt: &t1}, expectedState: "deleted"}, + {upload: Upload{ID: 20, State: "completed", FinishedAt: &t3}, expectedState: "deleted"}, // dependency of 19 + + // old, but dependency of a non-deletion candidate + {upload: Upload{ID: 21, State: "completed", FinishedAt: &t1}, expectedState: "completed"}, // dependency of 13 + {upload: Upload{ID: 22, State: "completed", FinishedAt: &t1}, expectedState: "completed"}, // dependency of 14 + {upload: Upload{ID: 23, State: "completed", FinishedAt: &t1}, expectedState: "completed"}, // dependency of 16 + {upload: Upload{ID: 24, State: "completed", FinishedAt: &t1}, expectedState: "completed"}, // dependency of 16 (via 23) + } + + var uploads []Upload + for _, test := range tests { + uploads = append(uploads, test.upload) + } + + insertUploads(t, db, uploads...) + insertVisibleAtTip(t, db, 50, 14, 15) + insertVisibleAtTipNonDefaultBranch(t, db, 50, 16) + + packages := map[int][]semantic.Package{ + 20: {{Scheme: "s0", Name: "n0", Version: "v0"}}, + 21: {{Scheme: "s1", Name: "n1", Version: "v1"}}, + 22: {{Scheme: "s2", Name: "n2", Version: "v2"}}, + 23: {{Scheme: "s3", Name: "n3", Version: "v3"}}, + 24: {{Scheme: "s4", Name: "n4", Version: "v4"}}, + } + references := map[int][]semantic.PackageReference{ + 13: {{Package: semantic.Package{Scheme: "s1", Name: "n1", Version: "v1"}}}, + 14: {{Package: semantic.Package{Scheme: "s2", Name: "n2", Version: "v2"}}}, + 16: {{Package: semantic.Package{Scheme: "s3", Name: "n3", Version: "v3"}}}, + 19: {{Package: semantic.Package{Scheme: "s0", Name: "n0", Version: "v0"}}}, + 23: {{Package: semantic.Package{Scheme: "s4", Name: "n4", Version: "v4"}}}, + } + + for id, packages := range packages { + if err := store.UpdatePackages(context.Background(), id, packages); err != nil { + t.Fatalf("unexpected error updating packages: %s", err) + } + } + for id, references := range references { + if err := store.UpdatePackageReferences(context.Background(), id, references); err != nil { + t.Fatalf("unexpected error updating package references: %s", err) + } + } if count, err := store.SoftDeleteOldUploads(context.Background(), time.Minute, t1.Add(time.Minute*6)); err != nil { - t.Fatalf("unexpected error pruning uploads: %s", err) + t.Fatalf("unexpected error soft deleting uploads: %s", err) } else if count != 4 { t.Fatalf("unexpected number of uploads deleted: want=%d have=%d", 4, count) } - expectedStates := map[int]string{ - 1: "deleted", - 2: "completed", - 3: "deleted", - 4: "completed", - 5: "deleted", - 6: "completed", - 7: "errored", - 8: "deleted", - 9: "uploaded", - 10: "completed", + var uploadIDs []int + expectedStates := map[int]string{} + for _, test := range tests { + id := test.upload.ID + uploadIDs = append(uploadIDs, id) + expectedStates[id] = test.expectedState } // Ensure record was deleted - if states, err := getUploadStates(db, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10); err != nil { + if states, err := getUploadStates(db, uploadIDs...); err != nil { t.Fatalf("unexpected error getting states: %s", err) } else if diff := cmp.Diff(expectedStates, states); diff != "" { t.Errorf("unexpected upload states (-want +got):\n%s", diff) diff --git a/internal/database/schema.md b/internal/database/schema.md index b62445629021..6c59bc6443dd 100755 --- a/internal/database/schema.md +++ b/internal/database/schema.md @@ -982,7 +982,7 @@ Associates commits with the closest ancestor commit with usable upload data. Tog dump_id | integer | | not null | Indexes: "lsif_packages_pkey" PRIMARY KEY, btree (id) - "lsif_packages_scheme_name_version" btree (scheme, name, version) + "lsif_packages_scheme_name_version_dump_id" btree (scheme, name, version, dump_id) Foreign-key constraints: "lsif_packages_dump_id_fkey" FOREIGN KEY (dump_id) REFERENCES lsif_uploads(id) ON DELETE CASCADE @@ -1010,7 +1010,7 @@ Associates an upload with the set of packages they provide within a given packag dump_id | integer | | not null | Indexes: "lsif_references_pkey" PRIMARY KEY, btree (id) - "lsif_references_package" btree (scheme, name, version) + "lsif_references_scheme_name_version_dump_id" btree (scheme, name, version, dump_id) Foreign-key constraints: "lsif_references_dump_id_fkey" FOREIGN KEY (dump_id) REFERENCES lsif_uploads(id) ON DELETE CASCADE diff --git a/migrations/frontend/1528395852_lsif_add_covering_index.down.sql b/migrations/frontend/1528395852_lsif_add_covering_index.down.sql new file mode 100755 index 000000000000..9cc5c3b37b06 --- /dev/null +++ b/migrations/frontend/1528395852_lsif_add_covering_index.down.sql @@ -0,0 +1,9 @@ +BEGIN; + +CREATE INDEX lsif_packages_scheme_name_version ON lsif_packages(scheme, name, version); +DROP INDEX lsif_packages_scheme_name_version_dump_id; + +CREATE INDEX lsif_references_package ON lsif_references(scheme, name, version); +DROP INDEX lsif_references_scheme_name_version_dump_id; + +COMMIT; diff --git a/migrations/frontend/1528395852_lsif_add_covering_index.up.sql b/migrations/frontend/1528395852_lsif_add_covering_index.up.sql new file mode 100755 index 000000000000..a9a683384a07 --- /dev/null +++ b/migrations/frontend/1528395852_lsif_add_covering_index.up.sql @@ -0,0 +1,9 @@ +BEGIN; + +CREATE INDEX lsif_packages_scheme_name_version_dump_id ON lsif_packages(scheme, name, version, dump_id); +DROP INDEX lsif_packages_scheme_name_version; + +CREATE INDEX lsif_references_scheme_name_version_dump_id ON lsif_references(scheme, name, version, dump_id); +DROP INDEX lsif_references_package; + +COMMIT; From f1fef7f9881b755e3daf54734a01a08b21661917 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Fri, 16 Jul 2021 14:25:00 -0500 Subject: [PATCH 2/2] Order all bulk modifications to code intelligence tables. (#22932) --- .../codeintel/stores/dbstore/dumps.go | 39 +++++------ .../codeintel/stores/dbstore/indexes.go | 39 +++++++---- .../codeintel/stores/dbstore/janitor.go | 39 +++++++++-- .../codeintel/stores/dbstore/uploads.go | 66 ++++++++++++------- .../codeintel/stores/dbstore/xrepo.go | 3 +- .../codeintel/stores/lsifstore/clear.go | 6 ++ 6 files changed, 134 insertions(+), 58 deletions(-) diff --git a/enterprise/internal/codeintel/stores/dbstore/dumps.go b/enterprise/internal/codeintel/stores/dbstore/dumps.go index 4ec510b18c6b..469dd66eee93 100644 --- a/enterprise/internal/codeintel/stores/dbstore/dumps.go +++ b/enterprise/internal/codeintel/stores/dbstore/dumps.go @@ -164,7 +164,8 @@ func (s *Store) FindClosestDumps(ctx context.Context, repositoryID int, commit, const findClosestDumpsQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/dumps.go:FindClosestDumps -WITH visible_uploads AS (%s) +WITH +visible_uploads AS (%s) SELECT u.id, u.commit, @@ -245,7 +246,8 @@ func (s *Store) FindClosestDumpsFromGraphFragment(ctx context.Context, repositor const findClosestDumpsFromGraphFragmentCommitGraphQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/dumps.go:FindClosestDumpsFromGraphFragment -WITH visible_uploads AS (%s) +WITH +visible_uploads AS (%s) SELECT vu.upload_id, encode(vu.commit_bytea, 'hex'), @@ -379,26 +381,25 @@ func (s *Store) DeleteOverlappingDumps(ctx context.Context, repositoryID int, co const deleteOverlappingDumpsQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/dumps.go:DeleteOverlappingDumps -WITH overlapping_dumps AS ( - SELECT id - FROM lsif_uploads +WITH +candidates AS ( + SELECT u.id + FROM lsif_uploads u WHERE - state = 'completed' AND - repository_id = %s AND - commit = %s AND - root = %s AND - indexer = %s - - -- Lock these rows in a deterministic order before the update - -- below. If we don't do this then we run into a pretty high - -- deadlock rate during upload processing as multiple workers - -- issue commands for the same set of records, but upload locks - -- records nondeterministically. - ORDER BY id FOR UPDATE + u.state = 'completed' AND + u.repository_id = %s AND + u.commit = %s AND + u.root = %s AND + u.indexer = %s + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_uploads table. + ORDER BY u.id FOR UPDATE ), updated AS ( - UPDATE lsif_uploads SET state = 'deleted' - WHERE id IN (SELECT id FROM overlapping_dumps) + UPDATE lsif_uploads u + SET state = 'deleted' + WHERE id IN (SELECT id FROM candidates) RETURNING 1 ) SELECT COUNT(*) FROM updated diff --git a/enterprise/internal/codeintel/stores/dbstore/indexes.go b/enterprise/internal/codeintel/stores/dbstore/indexes.go index 98da094417f7..937af54b7574 100644 --- a/enterprise/internal/codeintel/stores/dbstore/indexes.go +++ b/enterprise/internal/codeintel/stores/dbstore/indexes.go @@ -499,17 +499,23 @@ func (s *Store) DeleteIndexesWithoutRepository(ctx context.Context, now time.Tim const deleteIndexesWithoutRepositoryQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/indexes.go:DeleteIndexesWithoutRepository -WITH deleted_repos AS ( - SELECT r.id AS id FROM repo r - WHERE - %s - r.deleted_at >= %s * interval '1 second' AND - EXISTS (SELECT 1 from lsif_indexes u WHERE u.repository_id = r.id) +WITH +candidates AS ( + SELECT u.id + FROM repo r + JOIN lsif_indexes u ON u.repository_id = r.id + WHERE %s - r.deleted_at >= %s * interval '1 second' + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_indexes table. + ORDER BY u.id FOR UPDATE ), -deleted_uploads AS ( - DELETE FROM lsif_indexes u WHERE repository_id IN (SELECT id FROM deleted_repos) +deleted AS ( + DELETE FROM lsif_indexes u + WHERE id IN (SELECT id FROM candidates) RETURNING u.id, u.repository_id ) -SELECT d.repository_id, COUNT(*) FROM deleted_uploads d GROUP BY d.repository_id +SELECT d.repository_id, COUNT(*) FROM deleted d GROUP BY d.repository_id ` // DeleteOldIndexes deletes indexes older than the given age. @@ -543,9 +549,20 @@ func (s *Store) DeleteOldIndexes(ctx context.Context, maxAge time.Duration, now const deleteOldIndexesQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/indexes.go:DeleteOldIndexes -WITH deleted_indexes AS ( - DELETE FROM lsif_indexes u WHERE %s - u.queued_at > (%s || ' second')::interval +WITH +candidates AS ( + SELECT u.id + FROM lsif_indexes u + WHERE %s - u.queued_at > (%s || ' second')::interval + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_indexes table. + ORDER BY u.id FOR UPDATE +), +deleted AS ( + DELETE FROM lsif_indexes u + WHERE id IN (SELECT id FROM candidates) RETURNING u.id, u.repository_id ) -SELECT d.repository_id, COUNT(*) FROM deleted_indexes d GROUP BY d.repository_id +SELECT d.repository_id, COUNT(*) FROM deleted d GROUP BY d.repository_id ` diff --git a/enterprise/internal/codeintel/stores/dbstore/janitor.go b/enterprise/internal/codeintel/stores/dbstore/janitor.go index 31480dfdccfe..808246123499 100644 --- a/enterprise/internal/codeintel/stores/dbstore/janitor.go +++ b/enterprise/internal/codeintel/stores/dbstore/janitor.go @@ -89,7 +89,8 @@ func (s *Store) StaleSourcedCommits(ctx context.Context, minimumTimeSinceLastChe const staleSourcedCommitsQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/janitor.go:StaleSourcedCommits -WITH candidates AS (%s UNION %s) +WITH +candidates AS (%s UNION %s) SELECT r.id, r.name, c.commit FROM candidates c JOIN repo r ON r.id = c.repository_id @@ -141,8 +142,8 @@ func (s *Store) RefreshCommitResolvability(ctx context.Context, repositoryID int rows, err := s.Query(ctx, sqlf.Sprintf( refreshCommitResolvabilityQuery, - assignmentExpression, repositoryID, commit, - assignmentExpression, repositoryID, commit, + repositoryID, commit, assignmentExpression, + repositoryID, commit, assignmentExpression, )) if err != nil { return 0, 0, err @@ -167,8 +168,36 @@ func (s *Store) RefreshCommitResolvability(ctx context.Context, repositoryID int const refreshCommitResolvabilityQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/janitor.go:RefreshCommitResolvability WITH - update_uploads AS (UPDATE lsif_uploads SET %s WHERE repository_id = %s AND commit = %s RETURNING 1), - update_indexes AS (UPDATE lsif_indexes SET %s WHERE repository_id = %s AND commit = %s RETURNING 1) +candidate_uploads AS ( + SELECT u.id + FROM lsif_uploads u + WHERE u.repository_id = %s AND u.commit = %s + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_uploads table. + ORDER BY u.id FOR UPDATE +), +update_uploads AS ( + UPDATE lsif_uploads u + SET %s + WHERE id IN (SELECT id FROM candidate_uploads) + RETURNING 1 +), +candidate_indexes AS ( + SELECT u.id + FROM lsif_indexes u + WHERE u.repository_id = %s AND u.commit = %s + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_indexes table. + ORDER BY u.id FOR UPDATE +), +update_indexes AS ( + UPDATE lsif_indexes u + SET %s + WHERE id IN (SELECT id FROM candidate_indexes) + RETURNING 1 +) SELECT (SELECT COUNT(*) FROM update_uploads) AS num_uploads, (SELECT COUNT(*) FROM update_indexes) AS num_indexes diff --git a/enterprise/internal/codeintel/stores/dbstore/uploads.go b/enterprise/internal/codeintel/stores/dbstore/uploads.go index c68efb4cdb68..228d2a38497c 100644 --- a/enterprise/internal/codeintel/stores/dbstore/uploads.go +++ b/enterprise/internal/codeintel/stores/dbstore/uploads.go @@ -3,6 +3,7 @@ package dbstore import ( "context" "database/sql" + "sort" "strconv" "strings" "time" @@ -257,11 +258,21 @@ func (s *Store) DeleteUploadsStuckUploading(ctx context.Context, uploadedBefore const deleteUploadsStuckUploadingQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/uploads.go:DeleteUploadsStuckUploading -WITH deleted AS ( - UPDATE lsif_uploads +WITH +candidates AS ( + SELECT u.id + FROM lsif_uploads u + WHERE u.state = 'uploading' AND u.uploaded_at < %s + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_uploads table. + ORDER BY u.id FOR UPDATE +), +deleted AS ( + UPDATE lsif_uploads u SET state = 'deleted' - WHERE state = 'uploading' AND uploaded_at < %s - RETURNING repository_id + WHERE id IN (SELECT id FROM candidates) + RETURNING u.repository_id ) SELECT count(*) FROM deleted ` @@ -604,19 +615,24 @@ func (s *Store) DeleteUploadsWithoutRepository(ctx context.Context, now time.Tim const deleteUploadsWithoutRepositoryQuery = ` -- source: enterprise/internal/codeintel/stores/dbstore/uploads.go:DeleteUploadsWithoutRepository -WITH deleted_repos AS ( - SELECT r.id AS id FROM repo r - WHERE - %s - r.deleted_at >= %s * interval '1 second' AND - EXISTS (SELECT 1 from lsif_uploads u WHERE u.repository_id = r.id) +WITH +candidates AS ( + SELECT u.id + FROM repo r + JOIN lsif_uploads u ON u.repository_id = r.id + WHERE %s - r.deleted_at >= %s * interval '1 second' + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_uploads table. + ORDER BY u.id FOR UPDATE ), -deleted_uploads AS ( +deleted AS ( UPDATE lsif_uploads u SET state = 'deleted' - WHERE u.repository_id IN (SELECT id FROM deleted_repos) + WHERE u.id IN (SELECT id FROM candidates) RETURNING u.id, u.repository_id ) -SELECT d.repository_id, COUNT(*) FROM deleted_uploads d GROUP BY d.repository_id +SELECT d.repository_id, COUNT(*) FROM deleted d GROUP BY d.repository_id ` // HardDeleteUploadByID deletes the upload record with the given identifier. @@ -631,6 +647,11 @@ func (s *Store) HardDeleteUploadByID(ctx context.Context, ids ...int) (err error return nil } + // Ensure ids are sorted so that we take row locks during the + // DELETE query in a determinstic order. This should prevent + // deadlocks with other queries that mass update lsif_uploads. + sort.Ints(ids) + var idQueries []*sqlf.Query for _, id := range ids { idQueries = append(idQueries, sqlf.Sprintf("%s", id)) @@ -710,19 +731,20 @@ protected_uploads AS ( candidates AS ( -- Find the inverse of protected_uploads, which contains each upload record -- that is older than the configured retention age and is not reachable via - -- the dependencies of any upload in protected_uploads. We also order the - -- candidates here to try to acquire the locks in the following update in - -- a determinstic order so that we do not deadlock with another query updating - -- overlapping records. - - (SELECT id FROM lsif_uploads EXCEPT SELECT id FROM protected_uploads) ORDER BY id + -- the dependencies of any upload in protected_uploads. + SELECT u.id + FROM lsif_uploads u + WHERE u.id NOT IN (SELECT id FROM protected_uploads) + + -- Lock these rows in a deterministic order so that we don't + -- deadlock with other processes updating the lsif_uploads table. + ORDER BY u.id FOR UPDATE ), updated AS ( UPDATE lsif_uploads u - SET state = 'deleted' - WHERE - u.id IN (SELECT id FROM candidates) - RETURNING id, repository_id + SET state = 'deleted' + WHERE u.id IN (SELECT id FROM candidates) + RETURNING u.id, u.repository_id ) SELECT u.repository_id, count(*) FROM updated u GROUP BY u.repository_id ` diff --git a/enterprise/internal/codeintel/stores/dbstore/xrepo.go b/enterprise/internal/codeintel/stores/dbstore/xrepo.go index 4cc92bc97670..897f2a187abd 100644 --- a/enterprise/internal/codeintel/stores/dbstore/xrepo.go +++ b/enterprise/internal/codeintel/stores/dbstore/xrepo.go @@ -123,7 +123,8 @@ func (s *Store) ReferenceIDsAndFilters(ctx context.Context, repositoryID int, co const referenceIDsAndFiltersCTEDefinitions = ` -- source: enterprise/internal/codeintel/stores/dbstore/xrepo.go:ReferenceIDsAndFilters -WITH visible_uploads AS ( +WITH +visible_uploads AS ( (%s) UNION (SELECT uvt.upload_id FROM lsif_uploads_visible_at_tip uvt WHERE uvt.repository_id != %s AND uvt.is_default_branch) diff --git a/enterprise/internal/codeintel/stores/lsifstore/clear.go b/enterprise/internal/codeintel/stores/lsifstore/clear.go index 060539441f44..d6b2bea2a6a5 100644 --- a/enterprise/internal/codeintel/stores/lsifstore/clear.go +++ b/enterprise/internal/codeintel/stores/lsifstore/clear.go @@ -2,6 +2,7 @@ package lsifstore import ( "context" + "sort" "strconv" "strings" @@ -33,6 +34,11 @@ func (s *Store) Clear(ctx context.Context, bundleIDs ...int) (err error) { return nil } + // Ensure ids are sorted so that we take row locks during the + // DELETE query in a determinstic order. This should prevent + // deadlocks with other queries that mass update the same table. + sort.Ints(bundleIDs) + var ids []*sqlf.Query for _, bundleID := range bundleIDs { ids = append(ids, sqlf.Sprintf("%d", bundleID))