Provide a warning about Slurm-related envar

[rhc54]

Instead of silently ignoring the Slurm envar controlling internal PRRTE cmd line arguments used by srun to start the PRRTE daemons, let's output a warning message and error out if the envar is found.

[rhc54]

@jsquyres I basically took the message we had previously developed and massaged it a bit. Not entirely happy with it, but would welcome your contribution!

[jsquyres]

Just a thought: should we:

1. Put the lengthy explanation in https://docs.openpmix.org/en/latest/
2. Have the show_help message be short, but cite the specific page on https://docs.openpmix.org/en/latest/ with the full / lengthy explanation

?

[rhc54]

Yeah, I think that would be better - otherwise, it's a little overwhelming. Besides, we know that anyone running this on the latest Slurm is going to hit it. Something to consider: do we need an override option? Otherwise, I'm not sure how someone on these latest Slurm environs is going to be able to run - they cannot get Slurm to stop putting it in the environ.

[jsquyres]

Per our discussion on this PR, how about this for the message? (this was typed directly in github; please word wrap as appropriate)

PRTE detected the following MCA parameter in the environment:

  Parameter name:  %s
  Paramater value: %s

Unfortunately, SLURM will silently overwrite this value with its own value.  

To ensure that your MCA parameter value is used (and not the value that SLURM will silently set), 
you have two options:

1. Set this MCA parameter on the command line (vs. setting it in the environment).
2. Set this MCA parameter in a config file (vs. setting it in the environment).

Please see the following URL for a more complete description of the issue:

  ...some URL under https://docs.openpmix.org/en/latest/...

Note that this is slightly different %s expansion than the current message.

[rhc54]

Couple of issues here. First, the value we can see is the final envar value - i.e., if Slurm is going to overwrite it, they already have done so. We have no idea if the user attempted to set an envar for that param, nor what that value might have been.

Second, we still need to address the problem this PR creates - it will block any execution under Slurm 23.11 and beyond. I do think that (if we are going to do this) we need some flag the user can set to indicate that we should ignore the envar value (i.e., unsetenv it) and continue executing.

[jsquyres]

it will block any execution under Slurm 23.11 and beyond.

Oh, gotcha -- no, I don't think we want that. I thought we talked about adding (abstraction-breaking, but probably still necessary) code in the tools codes directly -- before prte_init() and friends...?

That's why I was thinking we could show the value and have good confidence that it's the user's value -- not the SLURM-set value.

Am I thinking incorrectly?

[rhc54]

Yeah, I'm afraid you're off - Slurm sets their envar before invoking mpirun (they just throw it into every allocation, just in case it is needed), so we only see the end result of what they do. If the user provided an envar, Slurm will have already overwritten it before invoking mpirun - so we can't know about it.

I guess that's only true for batch runs, though, as otherwise the user would have the chance to overwrite it (probably without realizing they did). In that case, our srun launch would fail due to the changed cmd line. Very awkward situation. Probably should just focus on the non-interactive use-case as that is the one where the problem would appear (i.e., envars getting overwritten).

If we detect an envar and error out, then there is no way the user can execute mpirun under Slurm 23.11 or above because Slurm will always have set their new envar. I guess we could detect and parse to see if other things beyond their addition are present - IIRC, they did release a fix to try and avoid overwriting the user's settings (not sure about that, but maybe). But that starts making this rather complicated.

So it seems to me we have a few non-ideal choices:

1. just silently unset the envar
2. warn and error out, but provide another envar that disables that behavior so people can execute. However, you can pretty much guarantee Slurm will just start setting that envar to alleviate user annoyance, so I'm not sure it will actually accomplish anything
3. check the envar to see if it includes the Slurm value and edit it out since we are dealing with the new cmd line internally. If the only thing we find is the Slurm value, then just ignore it. Otherwise, just silently let the rest of the value continue being processed as we will add the required new cmd line option before we launch.
4. or...??

Option 3 is beginning to sound like the best to me, but I'm open 🤷‍♂️

[jsquyres]

Slurm sets their envar before invoking mpirun (they just throw it into every allocation, just in case it is needed)

Ah, I guess I was thinking something different. Are you saying that this will happen:

shell$ salloc -N 1
shell$ echo $PRTE_MCA_plm_slurm_args
...will show the SLURM-set value here...

(and similar if I srun or sbatch a script that executes echo $PRTE_MCA_plm_slurm_args)

And if the above is correct, is the script below enough to defeat the SLURM silent env var writing?

shell$ cat myscript.sh
export PRTE_MCA_plm_slurm_args=foo
mpirun a.out
shell$ sbatch -N 2 ./myscript.sh

Or does SLURM set it in every allocation and also set it after mpirun and before prte (i.e., in the remote environment before it fork/exec's prte)? I.e., in all possible cases, SLURM sets this env variable, and it's just 100% not possible for the user to overwrite it.

If we detect an envar and error out, then there is no way the user can execute mpirun under Slurm 23.11 or above because Slurm will always have set their new envar.

Yeah, that's clearly bad -- we don't want that.

Per your specific options:

1. Just silently unset: we probably don't want to compound bad behavior with more bad behavior.
2. warn and error: I agree that this is not what we want.
3. check the env var: this becomes an arms race, doesn't it? I.e., would a future release of slurm change the env var value so that our code doesn't find the magic value and therefore not think it's the SLURM-set value?

Hmm. I guess I need to think about this more...

[rhc54]

Are you saying that this will happen:

shell$ salloc -N 1
shell$ echo $PRTE_MCA_plm_slurm_args
...will show the SLURM-set value here...

Pretty sure this is true

Or does SLURM set it in every allocation and also set it after mpirun and before prte (i.e., in the remote

Pretty sure this is not true, but I have no way of testing it.

I.e., in all possible cases, SLURM sets this env variable, and it's just 100% not possible for the user to overwrite it.

I don't believe this is true, but I honestly am not sure. There fix was aimed at mpirun users, but IIUC the envar was set at time-of-allocation. I don't see any way to set it between the mpirun and fork/exec of prterun as Slurm isn't involved there. So it would seem like users could override it if they chose, so long as they realized they need to include the new cmd line option.

The issue, in my mind, is that the envar is overriding what users put in their system and user-level default param files because the envar gets processed after those files. It isn't a question of someone setting an envar that doesn't work - it's a problem of putting values into default param files and unexplainingly finding them ignored. The user didn't put anything in the environment - their set values just "disappear" somehow.

Not sure what to do with it. Still a puzzlement on right path.

[jsquyres]

...but IIUC the envar was set at time-of-allocation. I don't see any way to set it between the mpirun and fork/exec of prterun as Slurm isn't involved there.

Gotcha.

But even at the mpirun level (which we do have control of in Open MPI, obviously), how can we tell the difference between the user setting this env var and SLURM itself setting this env var?

It isn't a question of someone setting an envar that doesn't work - it's a problem of putting values into default param files and unexplainingly finding them ignored.

Oh, so I totally misunderstood this, then. Ok.

There is an MCA API that finds the source of an MCA param value -- but IIRC, it just checks for the value that will be used (which, if the value is in both the param file and the environment, it'll report that the value is in the environment).

Sigh. What a pickle. I'm honestly not sure what the right answer is here. The only way I can think to do this is to have a horrible abstraction break of putting a check for these specific variables when reading param files, and if we're in the SLURM job and the corresponding getenv() returns non-NULL, emit the warning (this is arguably quite a bit worse abstraction break than checking for these env vars in main() or the like).

The goal is simple: warn users that their value is silently being ignored. How to detect that is the problem. ☹️

[rhc54]

Yeah, the problem is that it is impossible to tell who set the envar.

Only thing I can think of to try is to check for the envar and then parse the value if it is found. We know what Slurm is setting - so if we see that value, we could silently ignore it. If we see any other value, then we probably can safely assume that the user provided it, and let the MCA system handle it as usual.

Would take a little checking to ensure we correctly identify the Slurm value. I can give that a try.

[rhc54]

Afraid I cannot find a new enough version of Slurm I can put in my container - and as reported elsewhere, I'm having zero success getting Slurm to run in the containers. I give up. Let's just silently ignore it?