Repository navigation
Optimize task dispatch queries and bound listing payloads - #1260
Merged
Merged
Conversation
get_available_build_task() selected every eligible task FOR UPDATE and kept the first one in Python, so a single poll locked the whole set and ran its seven eager loaders over all of it. A second build node polling at the same time blocked on those locks until the first transaction ended, which capped dispatch on lock contention rather than on work. The build query now takes LIMIT 1 with FOR UPDATE OF build_tasks SKIP LOCKED, so it locks and loads exactly the task it hands out and a concurrent poller moves on to the next free one. The test task batch already had LIMIT 10 and only gains SKIP LOCKED, so two schedulers take disjoint batches instead of serializing. get_available_sign_task() read idle tasks without any lock and then updated the first one unconditionally, so two sign nodes polling at once could claim the same task. It now uses the same ORDER BY id, LIMIT 1, FOR UPDATE SKIP LOCKED shape, which makes the claim exclusive. sign_tasks.status has no index, so the lookup is still a sequential scan; that is left for a follow-up. The new tests drive two independent connections, since row locks are not observable from one session, and bound every call with a timeout so a regression that drops SKIP LOCKED fails instead of hanging. All of them fail against the previous queries. Related to AlmaLinux/build-system#553
GET /errata/query/ is unauthenticated, and without a pageNumber it returned every matching record with packages, albs_packages, build_artifacts and build_tasks eager-loaded for each of them. The frontend's "advisory ID already taken" check calls it exactly that way and only reads total_records. pageNumber now defaults to the first page. The response is an ErrataListResponse either way, so only the length of `records` changes, and total_records is still counted over the full filter. GET /errata/all/ keeps calling the CRUD with page=None for the compact OVAL projection. get_products() loaded every column of every build attached to a product, mock_options JSONB included, although ProductBuild only serializes the id. Product.builds is now loaded id-only. The number of rows is still unbounded; replacing `builds` with a count needs the frontend to stop reading product.builds.length first. GET /releases/ keeps its unpaginated contract: omitting pageNumber still returns the bare list of releases, which external API consumers rely on. In both list_errata_records() and get_releases(), a page below 1 is now clamped to the first page. Page 0 fell into the unpaginated branch with a different response shape, and negative values built a negative OFFSET that Postgres rejects. Page sizes are named constants. Related to AlmaLinux/build-system#553
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.
Summary
Part of AlmaLinux/build-system#553: two slow paths fixed. The task dispatch queries locked and loaded far more rows than they hand out, and two listing endpoints return more data than their callers read.
Claim build, test and sign tasks as queue pops
get_available_build_task():LIMIT 1+FOR UPDATE OF build_tasks SKIP LOCKED. Before, one poll locked every eligible task and ran its 7 eager loaders over all of them, and concurrent build nodes blocked on each other's locks.get_available_test_tasks(): addsSKIP LOCKED(it already hadLIMIT 10), so concurrent schedulers take disjoint batches.get_available_sign_task(): before, it read idle tasks without a lock and then ran an unconditional UPDATE, so two sign nodes could claim the same task. It now uses the same locked queue-pop shape.Bound errata and product listing payloads
GET /errata/query/:pageNumberdefaults to 1. The response shape is unchanged (ErrataListResponse) andtotal_recordsis still counted over the full filter, which is what the "advisory ID taken" check inCreateErrata.vuereads./errata/all/(OVAL) is unaffected.get_products():Product.buildsis loaded id-only.ProductBuildonly serializesid, so it no longer loadsmock_optionsJSONB for every build.GET /releases/: contract unchanged. OmittingpageNumberstill returns the bare list.pageNumber< 1 is clamped to page 1. Before, 0 fell into the unpaginated branch and negative values built a negative OFFSET.Testing
tests/test_crud/test_dispatch_locking.pyuses two independent DB connections with timeouts. All 5 tests fail onmasterand pass on this branch.test_errata.py,test_products.py,test_releases.py.test_releases.py,test_errata.py,test_products.py,test_dispatch_locking.py,test_crud/test_test.py: 38 passed against Postgres 13.EXPLAIN ANALYZEon tables shaped like production (2M build tasks / 400k sign tasks). The build dispatch query usesidx_build_tasks_status_arch_tsand does not walk the primary key underORDER BY id LIMIT 1.Follow-ups
sign_tasks.statushas no index, so the sign dispatch lookup is still a sequential scan. A partial index on idle tasks would fix that.GET /products/still returns every build id per product. Replacing it with a count needsProductFeed.vueto stop readingproduct.builds.lengthfirst.🤖 Generated with Claude Code