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

RFC21/27: should priorities be 0-LONGLONGMAX-1? #282

Open
chu11 opened this issue Jan 4, 2021 · 4 comments
Open

RFC21/27: should priorities be 0-LONGLONGMAX-1? #282

chu11 opened this issue Jan 4, 2021 · 4 comments

Comments

@chu11
Copy link
Member

chu11 commented Jan 4, 2021

Per discussion in flux-framework/flux-core#3428

The current priority range is 0 - 4294967295 (i.e. 0 to UINT32 MAX).

Some JSON libraries such as jansson may not support encoding of unsigned integers, only signed integers. So having a range of 0 to UINT32 MAX can be inconvenient at times.

Consider extending the range from 0 to LONGLONG_MAX (i.e. INT64 MAX or 2^62-1), so that 64 bit signed ints can be used everywhere for convenience.

@grondo
Copy link
Contributor

grondo commented Jan 4, 2021

I'll just note that there's no reason the range has to match the type. We could still specify 0-UINT32_MAX (or 0-2^48as you suggested) for the range, but useuint64_t` to store the value. This would require no change to the RFCs since they only specify the range and do not enforce the use of a type to store the priority.

@chu11
Copy link
Member Author

chu11 commented Jan 4, 2021

summarizing comments from here & flux-framework/flux-core#3428

keep things as they are now, defining priority in the range 0 - 2^32-1, there can be implementation ambiguity. We could:

  • define priority with same range, but define storage is larger (lets say int64_t) - pro: can remove some implementation inconsistencies, storage is pre-defined in case of need for expansion in future, -1 can be used for special purposes

  • expand range (anywhere from 2^32 to 2^63-1) - no need to define storage requirement, implementation ambiguity is likely removed

  • do both of the above, int64_t w/ max range defined.

@chu11
Copy link
Member Author

chu11 commented Jan 4, 2021

if I had to vote on the 3 above, I'd say lets go with the second option. Don't like the idea of defining a storage requirement, and we don't have a need for -1 for the time being.

@garlick
Copy link
Member

garlick commented Jan 4, 2021

I no longer have an opinion on this one. Status quo is fine by me, and as @grondo points out, independent of representation.

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

3 participants