Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC20: Need queue key under scheduling. #240

Open
dongahn opened this issue May 1, 2020 · 6 comments
Open

RFC20: Need queue key under scheduling. #240

dongahn opened this issue May 1, 2020 · 6 comments

Comments

@dongahn
Copy link
Member

dongahn commented May 1, 2020

As discussed at flux-framework/flux-sched#640 (comment) (item #5), we need the queue name of the job added to R. This way, on qmanager reload, its schedutil hello callback can use it to find the corresponding queue to insert the job (for queue state reconstruction.)

I propose to add this to the level under scheduling and make it optional. If the key doesn't exist, the job should be inserted into the default queue.

@dongahn
Copy link
Member Author

dongahn commented Jun 5, 2020

It feels like introducing an optional attributes key under scheduling would be a bit more future proof: e.g., attributes.system.queue = batch

@grondo
Copy link
Contributor

grondo commented Jun 5, 2020

Wasn't the entire scheduling key under purview of the current scheduler and thus perhaps none of it belongs in an RFC?

@dongahn
Copy link
Member Author

dongahn commented Jun 5, 2020

Wasn't the entire scheduling key under purview of the current scheduler and thus perhaps none of it belongs in an RFC?

@grondo: Yes, but as RFC20 is written:

The scheduling key allows RFC4-compliant schedulers to serialize any subset of graph resource data into its value and later deserialize this value with no data loss. The scheduling key contains a dictionary with a single key: graph. Other keys are reserved for future extensions.

So I can't have another key under scheduling and comply with RFC20. I know this whole thing will change with the new canonical resource set, but I at least need to relax it to complete PR flux-framework/flux-sched#663

@grondo
Copy link
Contributor

grondo commented Jun 5, 2020

Oh, ok. So it really only meant for a graph scheduler (probably should have just been a graph key). I agree we need a place for schedulers to place any opaque data, and attributes seems like a good choice for parity with jobspec. I wonder if attributes should be at the top level as in jobspec though. Then scheduler-specific data is under attributes.scheduler.

I don't think users can modify R so I'm not sure we need a delineation between system and user attributes, though maybe we want to keep it just in case?

@dongahn
Copy link
Member Author

dongahn commented Jun 5, 2020

I wonder if attributes should be at the top level as in jobspec though. Then scheduler-specific data is under attributes.scheduler.

Oh. I like that idea. I will change PR #248. This would position us better towards RV2 as well: All we need may be to remove the scheduling key once and for all and hoist up the graph key, or similar to the top level.

I don't think users can modify R so I'm not sure we need a delineation between system and user attributes, though maybe we want to keep it just in case?

The only reason I introduce system is to have a parallel struct with the job spec. Will attributes.system.scheduler.queue work for you? I also may also later change the key to stick the queue name in jobspec: attributes.system.scheduler.queue (it is currently attributes.system.queue). This ways, folks can find info in a bit more consistent manner.

@grondo
Copy link
Contributor

grondo commented Jun 5, 2020

Will attributes.system.scheduler.queue work for you?

Works for me. I don't think one level extra of depth in the JSON spec makes much difference, and gives us future flexibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants