Repository navigation
feat(rod): read jobs and allocations from one read-only snapshot - #1598
Draft
janzak-dev wants to merge 3 commits into
Draft
janzak-dev wants to merge 3 commits into
janzak-dev wants to merge 3 commits into
Conversation
This was referenced Nov 6, 2026
There was a problem hiding this comment.
Code Review
This pull request refactors the storage layer to ensure that active work unit fetching and resource aggregation are executed within a single, consistent read-only snapshot. It introduces the WithReadOnlyWorkUnitSnapshot method to the WorkUnitStore interface and implements it for both PostgreSQL and Spanner backends, replacing the separate AggregateActiveResourcesByAuthor method. Corresponding updates have been made to the data provider server, mock implementations, unit tests, and documentation. I have no feedback to provide as there are no review comments.
janzak-dev
force-pushed
the
feat/rod-user-alloc-backend-snapshot
branch
from
November 6, 2026 13:10
af6d17b to
6f28728
Compare
janzak-dev
force-pushed
the
feat/rod-user-alloc-backend-snapshot
branch
from
November 6, 2026 13:55
6f28728 to
b3804ed
Compare
janzak-dev
force-pushed
the
feat/rod-user-alloc-backend-snapshot
branch
from
November 6, 2026 14:36
b3804ed to
53276a3
Compare
Add WorkUnitStore.WithReadOnlyWorkUnitSnapshot and the WorkUnitSnapshotRepository it hands to callers, implemented with a Spanner ReadOnlyTransaction and a Postgres REPEATABLE READ READ ONLY transaction. GetRunningAndPendingJobs now reads the jobs page and the per-author aggregation from the same snapshot, so user_allocations and the jobs page always describe the same queue state. FetchActiveWorkUnitsByQueues delegates to the snapshot, and the standalone repository-level aggregation is folded into it. Bug: 582253355 TAG=agy CONV=4ed50232-a6f2-4c76-aa6f-ef1c3833a550
…ository The Postgres snapshot now hands fn the existing TxRepository (via WithTx) instead of a second transaction-bound wrapper. The Spanner and Postgres snapshot tests share storagetest.ReadActiveResourcesSnapshot and its verify helpers instead of duplicated snapshot plumbing, and drop guard assertions already covered by FetchActiveWorkUnitsByQueues, which runs through the same snapshot. Bug: 582253355 TAG=agy CONV=4ed50232-a6f2-4c76-aa6f-ef1c3833a550
- WorkUnitFetcher only needs WithReadOnlyWorkUnitSnapshot now that the handler reads the jobs page through the snapshot repository. - Unexport readActiveResourcesSnapshot, which only the storagetest verify helpers use, and drop a compile-time assertion the snapshot call site already enforces. Bug: 582253355 TAG=agy CONV=4ed50232-a6f2-4c76-aa6f-ef1c3833a550
janzak-dev
force-pushed
the
feat/rod-user-alloc-backend-snapshot
branch
from
November 6, 2026 15:17
53276a3 to
6ca8a84
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stacked PR (part of #1515): per-user "dominant resource" allocation on
/resources. Every layer has a production caller and tests. Unfinished backend pieces return empty data, never placeholder values.Backend: endpoint and RPC presenting correct information
user_allocations+ handler mapping from the per-author storage contract. The handler is the producer; storage returns no rows yet, so the field is empty.Frontend: UI presents correct data
4. #1599 Per-user allocation bar on
/resources(the consumer ofuserAllocations).Summary
GetRunningAndPendingJobsnow reads the jobs page and the per-author aggregation from one read-only snapshot, souser_allocationsand the jobs page always describe the same queue state. After this PR the RPC returns correct, consistent data.Motivation & Context
The final backend layer. In the previous PR the two reads use separate transactions, so a job changing state between them could make the allocations disagree with the jobs page.
Key Changes
internal/storage/storage.go:WorkUnitSnapshotRepository(FetchActiveWorkUnitsByQueuesandAggregateActiveResourcesByAuthor) plusWorkUnitStore.WithReadOnlyWorkUnitSnapshot. The repository-level aggregation moves into the snapshot repository.ReadOnlyTransaction-bound snapshot repository.FetchActiveWorkUnitsByQueuesdelegates to it; existing comments are kept.pgdb.RunInReadOnlyTx(REPEATABLE READ READ ONLY) handsfnthe existingTxRepository(viaWithTx), which gains the two read methods;FetchActiveWorkUnitsByQueuesdelegates to it.jobs.go:fetchJobsAndAllocationsruns both reads insideWithReadOnlyWorkUnitSnapshot.WorkUnitFetchernarrows toWithReadOnlyWorkUnitSnapshot, andMockStoragepasses itself as the snapshot repository.Assumptions
None — requirements were fully specified (caller-managed read-only transaction per
internal/storage/AGENTS.md).Verification & Testing
storagetest.VerifyActiveResourcesSnapshot: the first jobs page (2 of 6 active work units) and the aggregation come from the same snapshot. ExistingFetchActiveWorkUnitsByQueuespagination and ordering tests still pass, and a handler test checks there is exactly one snapshot per request.bazel test //internal/storage/... //cmd/rod_data_provider/...: all pass../.githooks/pre-commitpasses.user_allocationsequals the per-user sum of requested units over all pages of jobs. For example, a 14-job CPU queue returnedcpu 0 running + 1002 pendingandmemory 0 + 7 GiB, and withpage_size=5every page returned the same totals.Related Issues / Bugs
Part of #1515
TAG=agy
CONV=4ed50232-a6f2-4c76-aa6f-ef1c3833a550