Hi all,
I’m proposing removing the job attribute ‘pset’. Please go through the design document and let me know what you think:
https://pbspro.atlassian.net/wiki/spaces/PD/pages/1555726352/Remove+pset+job+attribute
Thanks,
Ravi
Hi!
Generally, we want all changes to PBS interfaces to be backward compatible. The goal should be 100% syntax compatible for anything used in job submission (e.g., via qsub or #PBS), status checking (e.g., qstat output), or known to be used by 3rd party/customer tools. For admin capabilities, being 100% syntax compatible is less important as long as there is another syntax that provides the same capability, though, best case is always 100%. In cases where we plan to make a change, the process should be to first deprecate the syntax, then (a few releases/years later), remove it. (Deprecated means we label/document it as being deprecated and to be removed in a future release, but it is otherwise unchanged.)
To me, pset is an externally supported interface that might be used by tools, so simply removing it would be … bad.
An alternative suggestion would be to:
(1) Mark it deprecated in the next release, but leave it in. Also add an option to PBS that can be used to turn it off (but set the default to “on”).
(2) Next release, change the option default to “off”
(3) At some future release after that, remove it and the option to turn it on.
(Sorry for the long note.)
The pset attribute was never really intended for external customers. It was meant to pass information to the mom for Bluegene support. We’ve long since dropped support for the Bluegene. This is just a left over attribute. It never hurt anything, so I never removed it. The multi-server team has identified that the scheduler -> server alter jobs as a slowdown. Since pset is now no longer needed for any PBS functionality, it was identified as something to remove.
Since the multi-server code isn’t checking in until after the 2020.1 release, how about we deprecate it in the 2020.1 release and then just remove it.
Bhroam
Thanks for the feedback guys.
@billnitzberg what do you think of Bhroam’s suggestion of marking it as deprecated now and removing it the release after?
My suggestion was to deprecate it in the upcoming release, so that’s fine.
If “pset” never ended up in any man pages or other documentation, then I have less concern, and it can probably be removed in a later release. It’s still a risk, but less so. In general, we should err towards making decisions on backward compatibility based on actual use today (not the intended use from a past design).
Thx!
It is unfortunately in the pbs_job_attributes man page. It just has a one sentence description of what it does. It’s probably there because Anne tries to be complete with things like this.
I still don’t have a problem with deprecating it for one release and then removing it the next. We can’t put any sort of message in the product since it is just a variable used for output. The customer doesn’t interact with the pset variable with PBS in any way. It is just visible in qstat -f.
The pset job attribute is listed in the man page.
OK, we need to be more careful as this is a documented, external interface. Again, deprecating it is fine (and noting it in the release notes as being deprecated, and removing it from the man page). We should revisit whether/when it can be removed from the interface based on feedback after it’s been deprecated for a while (~1 year at least).
Ok, in that case we’d have to introduce a short lived attribute which will need to be passed to the scheduler to tell it to not send pset updates to server. I’ll make sure to not mark it a stable interface, otherwise this will be an endless loop 
Thanks for explaining the right way to do this.
I’d rather just let it be set, but remove mention of it from our documentation. I would hate to add something that we’d just rip out later for such an inconsequential attribute.
Bhroam
I hate the idea of adding a new attribute for it as well, but we do need to do something. It’s not even marked NOSAVM, so every alterjob for it will cause a db write. How about passing it with the pbs_runjob call itself? Maybe make it a part of the ‘location’ string?
i take it back, that might be too much work for something that we will deprecate and remove anyways …
@bhroam and @billnitzberg I had a discussion with @subhasisb and he had a suggestion: let’s deprecate pset for now, but instead of adding a new attribute which tells sched not to set it, it can instead check whether PBS is running in multi-server mode or not. If it is, then sched will not send pset updates. We will be adding a new pbs.conf option PBS_MAX_SERVER_INSTANCES for multi-server, maybe sched can use that, if it’s > 1 then it won’t send out pset updates. How does this sound?
I’m not sure we need that. Our releases are usually 1 year apart, so if we deprecate it in 2020.1, we should be able to remove it in 2021.1. The multi-server code will not check into 2020.1, so we should be fine.
Bhroam
I like that solution — it allows for backward compatibility, and also allows the new behavior when it provides a benefit.
Alright, thanks guys, so as and when multi-server is checked in, we can modify sched to not send pset updates if multi-server is configured, or just remove it if multi-server doesn’t get checked in until 2021.
Now, i need to figure out how to deprecate pset, do we just need to update the user documentation since there’s no behavioral change associated with it? And remove it from the man pages?
Please read the latest on the page (https://pbspro.atlassian.net/wiki/spaces/PD/pages/1555726352/Deprecate+pset+job+attribute) and let me know if it looks okay. Thanks.
Hi all,
An update on this, since pset has been deprecated for a release, it should be ok to remove it. I’ve updated the doc to reflect that: PBS Pro Confluence
and am going to remove it soon. Please let me know if this sounds ok. Thanks!