artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 15:12:02 +00:00
Code Issues Packages Projects Releases Wiki Activity

Check for scheduled or processing jobs when scheduling permission syncs (#61024)

Browse Source
This commit is contained in:
Petri-Johan Last 2024-03-12 15:04:52 +02:00 committed by GitHub
parent c549dbd92f
commit 810289a0f1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 34 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -30,6 +30,7 @@ All notable changes to Sourcegraph are documented in this file.
- Code Monitors now properly ignores monitors associated with soft-deleted users, which previously would have led to an error on the overview page. [#60405](https://github.com/sourcegraph/sourcegraph/pull/60405)
- Fixed a bug where clicking "Exclude Repo" on Azure DevOps or Gerrit repositories would not work. [#60509](https://github.com/sourcegraph/sourcegraph/pull/60509)
- Links in codeintel popovers respect the revision from the URL. [#60545](https://github.com/sourcegraph/sourcegraph/pull/60545)
- Fixed a bug where permission syncs could be scheduled for repositories or users even when a sync is already scheduled or in progress, leading to significant delays in the permissions sync system as a whole. [#61024](https://github.com/sourcegraph/sourcegraph/pull/61024)
### Removed

1
cmd/worker/internal/authz/perms_syncer.go
View File

@ -175,7 +175,6 @@ func (s *permsSyncerImpl) syncRepoPerms(ctx context.Context, repoID api.RepoID,
ServiceID: provider.ServiceID(),
AccountIDs: accountIDs,
})
if err != nil {
return result, providerStates, errors.Wrapf(err, "get user IDs by external accounts for repository %q (id: %d)", repo.Name, repo.ID)
}

13
internal/database/perms_store.go
View File

@ -1607,13 +1607,17 @@ SELECT u.id as user_id, MAX(p.finished_at) as finished_at
FROM users u
LEFT JOIN permission_sync_jobs p ON u.id = p.user_id AND p.user_id IS NOT NULL
WHERE u.deleted_at IS NULL AND (%s)
AND NOT EXISTS (
SELECT 1 FROM permission_sync_jobs p2
WHERE p2.user_id = u.id AND (p2.state = 'queued' OR p2.state = 'processing')
)
GROUP BY u.id
ORDER BY finished_at ASC NULLS FIRST, user_id ASC
LIMIT %d;
`
// UserIDsWithOldestPerms lists the users with the oldest synced perms, limited
// to limit. If age is non-zero, users that have synced within "age" since now
// to limit, for which there is no sync job scheduled at the moment. If age is non-zero, users that have synced within "age" since now
// will be filtered out.
func (s *permsStore) UserIDsWithOldestPerms(ctx context.Context, limit int, age time.Duration) (map[int32]time.Time, error) {
q := sqlf.Sprintf(usersWithOldestPermsQuery, s.getCutoffClause(age), limit)
@ -1625,11 +1629,18 @@ SELECT r.id as repo_id, MAX(p.finished_at) as finished_at
FROM repo r
LEFT JOIN permission_sync_jobs p ON r.id = p.repository_id AND p.repository_id IS NOT NULL
WHERE r.private AND r.deleted_at IS NULL AND (%s)
AND NOT EXISTS (
SELECT 1 FROM permission_sync_jobs p2
WHERE p2.repository_id = r.id AND (p2.state = 'queued' OR p2.state = 'processing')
)
GROUP BY r.id
ORDER BY finished_at ASC NULLS FIRST, repo_id ASC
LIMIT %d;
`
// ReposIDsWithOldestPerms lists the repositories with the oldest synced perms, limited
// to limit, for which there is no sync job scheduled at the moment. If age is non-zero, repos that have synced within "age" since now
// will be filtered out.
func (s *permsStore) ReposIDsWithOldestPerms(ctx context.Context, limit int, age time.Duration) (map[api.RepoID]time.Time, error) {
q := sqlf.Sprintf(reposWithOldestPermsQuery, s.getCutoffClause(age), limit)

28
internal/database/perms_store_test.go
View File

@ -3388,6 +3388,8 @@ func TestPermsStore_UserIDsWithOldestPerms(t *testing.T) {
sqlf.Sprintf(`INSERT INTO users(id, username) VALUES(1, 'alice')`),
sqlf.Sprintf(`INSERT INTO users(id, username) VALUES(2, 'bob')`),
sqlf.Sprintf(`INSERT INTO users(id, username, deleted_at) VALUES(3, 'cindy', NOW())`),
sqlf.Sprintf(`INSERT INTO users(id, username) VALUES(4, 'david')`),
sqlf.Sprintf(`INSERT INTO users(id, username) VALUES(5, 'erica')`),
}
for _, q := range qs {
executeQuery(t, ctx, s, q)
@ -3397,15 +3399,20 @@ func TestPermsStore_UserIDsWithOldestPerms(t *testing.T) {
user1UpdatedAt := clock().Add(-15 * time.Minute)
user2UpdatedAt := clock().Add(-5 * time.Minute)
user3UpdatedAt := clock().Add(-11 * time.Minute)
q := sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, finished_at, reason) VALUES(%d, %s, %s)`, 1, user1UpdatedAt, ReasonUserOutdatedPermissions)
q := sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 1, user1UpdatedAt, ReasonUserOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, finished_at, reason) VALUES(%d, %s, %s)`, 2, user2UpdatedAt, ReasonUserOutdatedPermissions)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 2, user2UpdatedAt, ReasonUserOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, finished_at, reason) VALUES(%d, %s, %s)`, 3, user3UpdatedAt, ReasonUserOutdatedPermissions)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 3, user3UpdatedAt, ReasonUserOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, state, reason) VALUES(%d, 'queued', %s)`, 4, ReasonUserOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(user_id, state, reason) VALUES(%d, 'processing', %s)`, 5, ReasonUserOutdatedPermissions)
executeQuery(t, ctx, s, q)
t.Run("One result when limit is 1", func(t *testing.T) {
t.Run("One result when limit is 1. Queued user ignored", func(t *testing.T) {
// Should only get user 1 back, because limit is 1
// Users with queued and processing permissions syncs are ignored
results, err := s.UserIDsWithOldestPerms(ctx, 1, 0)
if err != nil {
t.Fatal(err)
@ -3559,6 +3566,8 @@ func TestPermsStore_ReposIDsWithOldestPerms(t *testing.T) {
sqlf.Sprintf(`INSERT INTO repo(id, name, private) VALUES(1, 'private_repo_1', TRUE)`), // id=1
sqlf.Sprintf(`INSERT INTO repo(id, name, private) VALUES(2, 'private_repo_2', TRUE)`), // id=2
sqlf.Sprintf(`INSERT INTO repo(id, name, private, deleted_at) VALUES(3, 'private_repo_3', TRUE, NOW())`), // id=3
sqlf.Sprintf(`INSERT INTO repo(id, name, private) VALUES(4, 'private_repo_4', TRUE)`), // id=2
sqlf.Sprintf(`INSERT INTO repo(id, name, private) VALUES(5, 'private_repo_5', TRUE)`), // id=2
}
for _, q := range qs {
executeQuery(t, ctx, s, q)
@ -3568,15 +3577,20 @@ func TestPermsStore_ReposIDsWithOldestPerms(t *testing.T) {
repo1UpdatedAt := clock().Add(-15 * time.Minute)
repo2UpdatedAt := clock().Add(-5 * time.Minute)
repo3UpdatedAt := clock().Add(-10 * time.Minute)
q := sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, finished_at, reason) VALUES(%d, %s, %s)`, 1, repo1UpdatedAt, ReasonRepoOutdatedPermissions)
q := sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 1, repo1UpdatedAt, ReasonRepoOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, finished_at, reason) VALUES(%d, %s, %s)`, 2, repo2UpdatedAt, ReasonRepoOutdatedPermissions)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 2, repo2UpdatedAt, ReasonRepoOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, finished_at, reason) VALUES(%d, %s, %s)`, 3, repo3UpdatedAt, ReasonRepoOutdatedPermissions)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, state, finished_at, reason) VALUES(%d, 'completed', %s, %s)`, 3, repo3UpdatedAt, ReasonRepoOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, state, reason) VALUES(%d, 'queued', %s)`, 4, ReasonRepoOutdatedPermissions)
executeQuery(t, ctx, s, q)
q = sqlf.Sprintf(`INSERT INTO permission_sync_jobs(repository_id, state, reason) VALUES(%d, 'processing', %s)`, 5, ReasonRepoOutdatedPermissions)
executeQuery(t, ctx, s, q)
t.Run("One result when limit is 1", func(t *testing.T) {
// Should only get private_repo_1 back, because limit is 1
// Repos with jobs queued or in progress are ignored
results, err := s.ReposIDsWithOldestPerms(ctx, 1, 0)
if err != nil {
t.Fatal(err)