return effective queue config in job-manager.queue-list and use it in flux resource - #7772
Open
grondo wants to merge 10 commits into
Open
return effective queue config in job-manager.queue-list and use it in flux resource#7772grondo wants to merge 10 commits into
job-manager.queue-list and use it in flux resource#7772grondo wants to merge 10 commits into
Conversation
Problem: The multi-line comments on the 'requires' and 'parent' fields of struct queue are indented one space past the opening slash, so the comments do not properly align. Align the continuation stars with the opening comment. Assisted-by: Claude:Opus-4.8
Problem: The queue-list RPC returns queue names in arbitrary hash order, so tools cannot present queues reproducibly, and there is no ordered structure to build a richer response on. Maintain a zlistx of named queues alongside the hash, re-sequenced to config declaration order at the end of each reconfigure, and encode the name list from it. The list holds borrowed pointers with per-queue handles for O(1) removal, and is structured so future runtime queues can be appended after the static queues. Assisted-by: Claude:Opus-4.8
Problem: The new config-declaration ordering of the queue name list is not exercised by the unit tests, so a regression in the reorder logic would go unnoticed. Add test_list_order() covering initial declaration order and reloads that reorder, add, remove, combine all three, drop to anon mode, and re-enter named mode. Assisted-by: Claude:Opus-4.8
Problem: Components that need a queue's effective configuration re-read
the [queues] table and re-derive inherited fields client-side, so the
job-manager is not the authority on queue configuration.
Add a "conf" object to the queue-list response carrying each queue's
effective configuration ({name, requires, parent}) in declaration
order, so consumers can stop re-deriving it. A virtual queue's requires
is inherited from its parent. The conf object is an object, not a bare
array, leaving room for future global fields.
Assisted-by: Claude:Opus-4.8
Problem: The new conf object in the queue-list response, including its ordering and virtual-queue requires inheritance, has no test coverage. Add a queues_list_response() unit test asserting the conf shape, declaration order, and effective requires/parent, and extend the t0034-queuelist Python test to check the conf object over the wire, including a reconfigure that re-sequences queues. Assisted-by: Claude:Opus-4.8
Problem: The queue-list response will be fetched by many components but changes rarely, so rebuilding it on every request is wasted work. Cache the assembled response in the queues object, rebuilding it lazily and invalidating it from notify() on any mutation. queues_list_response() now returns a borrowed reference owned by the cache. Since notify() is the single choke point for every mutation, no change can bypass invalidation. The existing test_list_response() decref'd the return value, which is now owned by the cache, so those decrefs are removed here to keep the test correct. Assisted-by: Claude:Opus-4.8
Problem: The response caching added to queues_list_response() has no test asserting the cache is reused when unchanged and dropped on a mutation. Assert that a second call with no intervening mutation returns the same object, and that a queue mutation causes a fresh object to be built. Assisted-by: Claude:Opus-4.8
Problem: Clients that read the [queues] config to derive a queue's effective requires re-implement RFC 33 virtual-queue parent inheritance, and there is no shared way to produce the job-manager.queue-list "conf" object from a raw config. Add queue_conf_from_config(), which builds the "conf" object from a raw broker config in declaration order, resolving a virtual queue's requires from its parent. It backs the fallback path for older job-managers that predate the "conf" object and the hidden --config-file/--from-stdin test options. Assisted-by: Claude:Opus-4.8
Problem: flux-resource reads the [queues] config directly and re-derives each queue's effective requires, duplicating the RFC 33 virtual-queue parent inheritance that the job-manager already performs, so it is not the authority on queue configuration. Take the effective queue config from the job-manager.queue-list "conf" object instead. An older job-manager without "conf" and the hidden --config-file/--from-stdin test options fall back to deriving it locally via queue_conf_from_config(). This removes the client-side queue_effective_entry() resolver. Assisted-by: Claude:Opus-4.8
Problem: The new queue_conf_from_config() helper and its use as the authoritative queue config source for flux-resource have no test coverage, and nothing guards the helper against drifting from the job-manager's "conf" encoding. Add t0049-queue-conf.py unit tests for the helper (declaration order, effective requires, virtual-queue inheritance, and fail-closed on an unconfigured parent), and add a t0034 case asserting the helper reproduces the live queue-list "conf" object exactly. Assisted-by: Claude:Opus-4.8
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7772 +/- ##
==========================================
- Coverage 83.86% 83.84% -0.02%
==========================================
Files 599 599
Lines 102321 102414 +93
==========================================
+ Hits 85808 85874 +66
- Misses 16513 16540 +27
🚀 New features to boost your workflow:
|
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
1 participant
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.
This PR updates the
job-manager.queue-listRPC to include the effective queue configuration in a newconfobject, as a first step in making the job-manager the authoritative source for current queue configuration. Adding queue policy to the response is deferred to keep the PR reviewable, so only one consumer -flux-resource.py- is updated to useconf, which drops its need to read the[queues]table directly.The updated response includes a new
confobject next to the existingqueuesobject. Theconfobject currently contains a singlequeueskey - an array of queue configuration objects, e.g.:{"queues": [{"name:s", "requires?s", "parent?s"}, ...}.Other details of interest:
requiresis copied from its parent, so clients stop re-deriving inherited fields from[queues]. Policy inheritance will follow.confpreserves config declaration order, matching the ordering clients got when reading[queues]directly.flux resourcefalls back to reading[queues]from config directly if it is talking to an older job-manager that doesn't supply theconfobject in its response.