syncjobs: use rcache.FIFOList

[bobheadxi]

Migrates syncjobs to use the new rcache.FIFOList - this allows us to avoid scanning all Redis keys to find relevant records.

The experimental setting authz.syncJobsRecordsTTL has been changed to authz.syncJobsRecordsLimit - records are no longer retained based on age, but based on this size cap. The setting is clearly marked as experimental and is not advertised, so I think this is fine to change.

For simplicity, a hard cap of 500 is placed on reads, so that the reader does not need to implement conf watching.

Addresses https://github.com/sourcegraph/sourcegraph/pull/44387/files#r1080897926

Test plan

Test coverage, and a quick manual test via sg start and some dev-private config

[keegancsmith]

nice! Looking forward to approving this :)

For simplicity, a hard cap of 500 is placed on reads, so that the reader does not need to implement conf watching.

elegant :D

Given most uses of FIFOList have this pattern I wonder if the constructor should take a getter for MaxSize. Would simplify code in a few places.

[sourcegraph-bot]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 02c985e...85fc2b3.

Notify

File(s)

@unknwon

enterprise/cmd/frontend/internal/authz/resolvers/permissions_sync_jobs.go enterprise/cmd/frontend/internal/authz/resolvers/resolver.go enterprise/cmd/frontend/internal/authz/resolvers/resolver_test.go enterprise/internal/authz/syncjobs/mocks_test.go enterprise/internal/authz/syncjobs/records_reader.go enterprise/internal/authz/syncjobs/records_reader_test.go enterprise/internal/authz/syncjobs/records_store.go enterprise/internal/authz/syncjobs/records_store_test.go

[bobheadxi]

Was considering how this will tie in with https://github.com/sourcegraph/sourcegraph/issues/46078, since I just noticed that work has begun in earnest on DB-backed permissions sync - this query and cache was created to make it possible to validate that every authz provider configured has started permissions syncing successfully as part of prep work for https://github.com/sourcegraph/customer/issues/1525, so that we can offer guarantees of successful migration.

Looking at the newly introduced backgroundJobs query, and the format of the perms sync jobs, I think we can eventually get away with dropping this functionality entirely and just using backgroundJobs, at the cost of losing some granularity on per-provider statuses, but maybe it doesn't really matter that much.

It looks like the DB-backed sync might still be a few releases away, so I ended up deciding it's worthwhile to get this updated for now and deprecate/remove it later - the query is marked as experimental so I don't believe there's much issue in getting rid of it eventually.

cc @mrnugget @sashaostrikov

[keegancsmith]

Thanks!!

[keegancsmith]

I'm unsure how FIFOList behaves with a negative size. It likely needs to be updated (will be a tiny update) to special case that. Alternatively here if < 0 you make the number 0

[keegancsmith]

I did the former here https://github.com/sourcegraph/sourcegraph/pull/46702

I might land this for ya since I am now basing some other work of this PR.

[bobheadxi]

Thank you!

[mrnugget]

Was considering how this will tie in with https://github.com/sourcegraph/sourcegraph/issues/46078 [...]

@bobheadxi Work is in progress and @thenamankumar and @sashaostrikov are making great progress. It's not "a few releases" away, I'd say. But Naman can probably say more to when he expects to remove your Redis-based solution.

[0xnmn]

@bobheadxi, we are targeting to release DB backed perms syncer, thoroughly tested along with a new permissions sync job API by the end of Feb.

March release will definitely have it, and if things go well, we can release it in the Feb release itself.

[bobheadxi]

Happy to work with y'all on its removal - feel free to cc me on anything!