document scheduling.location in R to allow separate storage of scheduling key#524
document scheduling.location in R to allow separate storage of scheduling key#524grondo wants to merge 2 commits into
scheduling.location in R to allow separate storage of scheduling key#524Conversation
Problem: The RFC 27 Alloc Success section lists only the OPTIONAL `annotations` key as an additional field of the SUCCESS response, but the implementation requires an `R` key carrying the allocated resource set (RFC 20) and the job manager treats its absence as an error. Add `R` as a REQUIRED key of the SUCCESS response and specify that the scheduler MAY omit the OPTIONAL `scheduling` key from it, since the job manager needs only the execution portion. Describe how `scheduling` rides the allocation protocol: the scheduler persists it out-of-line to `job.<jobid>.scheduling` (RFC 20) and the persistence layer reassembles the complete R for consumers that require it. Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
Problem: The RFC 20 `scheduling` member of an R object contains the Fluxion JGF graph, which is large but needed by only a few readers. Inlining it into every `job.<id>.R` and streaming it to every job shell causes excessive overlay network traffic at scale. Add a `scheduling.location` sub-key to RFC 20 that references a complete `scheduling` object stored out-of-line in the KVS. When `scheduling` is stored out-of-line, `location` SHALL be present and the referenced key SHALL contain a complete `scheduling` object. A component persisting R this way is required to reassemble the complete R for consumers that request it. Update RFC 16 to document `job.<jobid>.R` as the canonical key whose `scheduling` object MAY be stored out-of-line, and `job.<jobid>.scheduling` as the write-once key holding the complete object. Update RFC 28 to note that the `resources` object in the initial acquire response MAY reference an out-of-line `scheduling` object at `resource.scheduling`. Co-authored-by: Claude Sonnet 4 <noreply@anthropic.com>
|
Tick the box to add this pull request to the merge queue (same as
|
scheduling.location key in R to allow separate storage of scheduling keyscheduling.location in R to allow separate storage of scheduling key
garlick
left a comment
There was a problem hiding this comment.
I made a few suggestions but I don't feel terribly strongly about them.
I guess my main suggestion is that we make location a key name in the same directory as R rather than a path, and for now to require it to be set to scheduling. This would tighten up the spec, require fewer words to describe, and give consumers the option of fetching the keys in parallel rather than have to fetch R first to determine the location value.
| manager requires only the execution portion, the scheduler MAY omit the | ||
| OPTIONAL :data:`scheduling` key from *R* in this response; the job manager | ||
| SHALL NOT rely on :data:`scheduling` being present here. The scheduler SHALL |
There was a problem hiding this comment.
Suggestion: Just say the scheduler SHALL NOT include the scheduling key in this response rather than make baggage optional that the job manager is then ordered not to use. Flux-core owns the code that composes this response for fluxion (libschedutil) so we can make this change in one place and it won't break interoperability.
There was a problem hiding this comment.
This would be a larger change in behavior orthogonal to adding a scheduling.location key, and so perhaps would be appropriate for a separate PR and corresponding flux-core PR to make the change. However, if you think it should be folded into this PR, I'm fine with that, but that's why I held back from proposing SHALL NOT here.
| complete :data:`scheduling` object to ``job.<jobid>.scheduling`` and | ||
| reference it with a :data:`scheduling.location` key in ``job.<jobid>.R``. |
There was a problem hiding this comment.
Suggestion: just say that location is for now always set to scheduling (bare key, in same directory). That simplifies descriptions and lets code fetch both keys in parallel, ignoring scheduling if it's not found or not referenced, retaining the self-describing nature of "location" and leaving the door open for other locations if need be.
There was a problem hiding this comment.
location is for now always set to
scheduling(bare key, in same directory)
The KVS doesn't have a concept of relative paths, so it would be difficult in the future to tell if "scheduling" indicates the "scheduling" key in the top-level KVS directory or the same directory as R. Also if for some reason an R object is copied or moved to a different directory, then the location becomes stale. While these misgivings are highly unlikely, it does make me question if scheduling.location is the right solution if all we want to do is specify that the scheduling key SHALL be separately stored in a scheduling key in the same directory as R.
| in ``job.<jobid>.R``. It SHALL be written before ``job.<jobid>.R`` is | ||
| committed so that the referenced data is available as soon as the reference | ||
| is visible. | ||
|
|
There was a problem hiding this comment.
Suggestion: s/written before/committed before or at the same time as/
| (*string*, OPTIONAL) A KVS key path at which the :data:`scheduling` | ||
| object is stored out-of-line. When :data:`scheduling` is stored | ||
| out-of-line, :data:`location` SHALL be present and the referenced KVS | ||
| key SHALL contain a complete :data:`scheduling` object. The referenced | ||
| key is written once and never rewritten. Per-job the conventional key | ||
| is ``job.<id>.scheduling``; for the instance resource inventory the | ||
| conventional key is ``resource.scheduling``. |
There was a problem hiding this comment.
If suggestion to hardwire a value of scheduling is accepted, the two conventional key examples can be dropped.
"written once" is covered below (redundant here?)
There was a problem hiding this comment.
I think possibly hardwire of location to scheduling may require a more nuanced rewrite of these RFC changes. I was just trying to get the discussion started. I'll close this one and the next person with the energy can take a stab at it.
|
|
||
| A consumer that requires the complete :data:`scheduling` object SHALL | ||
| obtain it either by fetching the key named by :data:`location` itself, | ||
| or by requesting a complete *R* from a service that performs the | ||
| reassembly on its behalf. A service that provides *R* (such as a job | ||
| information service) MAY offer this reassembly on demand, controlled by | ||
| a request option. The default behavior of such a service is | ||
| implementation-defined and MAY change over time, so a requester that | ||
| requires a specific behavior SHOULD request it explicitly. |
There was a problem hiding this comment.
Suggestion: this interrupts the flow of the object definition a bit. Move to note box or combine with other expository paragraphs below?
| Flux instance hierarchy. When :data:`scheduling` is stored out-of-line via | ||
| :data:`location`, it rides these protocols by virtue of being retrievable | ||
| from the KVS and reassembled by the persistence layer. |
There was a problem hiding this comment.
This paragraph is now confusing as to whether scheduling "rides along" or is stored separately, and in fact the RFC 27 allocation protocol describes optionally storing scheduling separately in the KVS and maybe being included in the alloc RPC response, while the RFC 28 acquisition protocol presumably must continue to provide the fully composed R to the scheduler.
Since those protocols are both defined elsewhere, maybe this paragraph could be dropped?
There was a problem hiding this comment.
It rides along the protocol by virtue of being referenced by location. Agreed that this could be dropped though.
Based on @garlick's idea in #523, this alternate proposal specifies a new
scheduling.locationkey in R which, when present, denotes a separate storage location for the Rschedulingkey in the KVS. Components reading R directly from the KVS will therefore read only the base R by default and will need to separately fetch anyschedulingkey if required.A couple design decisions of note:
scheduling.locationkey was chosen rather than extending thescheduling.writeras suggested in propose redacted R stored in KVS asR_redacted#523 as a separation of concerns approach. This allowsscheduling.writerto appear in its original form in the separately storedschedulingobject. The object itself is self-contained and the writer key can be read from there.schedulingkey anyway, the scheduler is allowed to drop theschedulingkey in the response. Not sure if it would be useful to keep it when it only contains alocationkey.This proposal as-is would break backwards compatibility with any component that assumes the full R will be returned when R is fetched from the KVS, job-info service, etc. This includes the
resourceandsched-fluxion-resourcemodules in subinstances, which make use ofjob-info. For a seamless transition, one plan would be to introduce two new flags in thejob-infolookup protocol:assemble- if the key (e.g. R) is stored in parts, job-info returns the fully assembled resultno-assemble- if the key is stored in parts, job-info does not assemble the resultThen,
assembleis the default in the initial version of the implementation. This keeps backwards compatibility for subinstances of differing Flux versions. Components that do not need the scheduling key in R (like same-version job shells), add theno-assembleflag to thejob-info.lookupRPC. After awhile, we switch the default and all components keep working as is. Theno-assembleflag could eventually be retired.Components that fetch R directly from the KVS will always get the unassembled version. However, these are all same-version components and for the most part (except for resource module restart) want only the unassembled R so I don't think there's an compatibility issue there.