Ticket 13545

Dear Felix,

I'm pretty sure the helpers plugin has always worked this way (I need to check others). slurmd has always told slurmctld its dynamic features via registration RPC.

I believe increasing SlurmdTimeout will increase the ping interval which in turn increases the amount of time in between registration messages, so that's a potential workaround.

You could also use CoreSpecCount/CpuSpecList to make sure that slurmd/slurmstepd never run on the same CPUs as the jobs. You would have to use the task/cgroup plugin, but I think you are. I think any scripts run by slurmd/slurmstepd (prolog/epilog, helpers, etc.) should be constrained by this cgroup as well, but I need to double check that.

Regarding the issue itself I have some ideas however, I will need some time to weigh the pros and cons and discuss this internally. I see this like an enhancement somehow as well, so I need to double check that as well.

I'll keep you posted,
Carlos.

> I'm pretty sure the helpers plugin has always worked this way (I need to check others). slurmd has always told slurmctld its dynamic features via registration RPC.

True, but node_features/helpers was added in 21.08 and before that the only existing plugin was for KNL. So perhaps we can call it an oversight from my part when designing the new node_features/helpers in https://bugs.schedmd.com/show_bug.cgi?id=9567, but I would qualify it as a minor performance bug.

> I believe increasing SlurmdTimeout will increase the ping interval which in turn increases the amount of time in between registration messages

Like increasing MAX_REG_FREQUENCY, this doesn't really eliminate the problem, unfortunately.

> You could also use CoreSpecCount/CpuSpecList to make sure that slurmd/slurmstepd never run on the same CPUs as the jobs. 

This is not enough to prevent possible performance interference to the user applications, so we can't use this approach.

Hi Felix,

I do see your point here, just give me some time to evaluate the best way to tackle the issue.

It might collide with "scontrol reconfig", or "scontrol update [...]" for avail/active features as well. So, if we decide to not issue the script every registration RPC, we need at least to issue the script if the features change.

Anyway, I think the idea of not executing the script every time is the right direction. What we need is to see how to accomplish this. My view of things is that somehow in the future we will have Bug 12288 implemented and it's best to address the issue here in a general way.

I think the reboot of nodes is well controlled now. So now we want to see how we can control when to issue the helper, maybe by comparing actual/new active/avail features when a potential modification is triggered, and issue the helper *only* there?

Everything I'm saying is just a conceptual explanation, and I need to go down the code to see if this can be done but, what do you think of?

Thanks,
Carlos.

Being explicit, looking at the source, "node_features_p_node_state" could be modified to check that if "avail_modes" are the same as last call don't trigger "list_for_each(helper_features, _foreach_helper_get_modes, &args)".

The way I see this possible, and thread safe, is to add a:

static char** prev_avail_modes = NULL;

as part of the local variables for "node_features_p_node_state". And place there the "avail_modes" for the call at the end to let the next call check the previous values.

I still need to check if this approach works, but it seems like a potential solution.

Ahh (I think) I see the point now.

1. It always uses _feature_get_state to directly query HW about the real active features. This way Slurm do not cache anything and if a feature is changed outside Slurm, Slurm will be aware of anyways.

2. But if we go and create some sort of cache for features and avoid usage of _feature_get_state, then we can end up with problems if 1. happens.

3. I think a possibility is to cache active features in _feature_set_state, disregard the 1st approach in Comment 9, and use _feature_get_state only if slurmd starts or scontrol reconfig/update is issued. And document appropriately that now features can't be modified outside Slurm.

It still remains to check how to detect scontrol reconfig/update, slurmd is pretty simple because the cache variables should be null at this point.

Yes, I suppose that one way of supporting "rebootless" node features changes today would be to modify the state of the node outside of Slurm, and then issue a "scontrol update" (or reconfigure). I think it's fine if all state changes must be under Slurm control instead, as long as we still leave the door open for rebootless node features changes to be implemented in a future version of Slurm.


Unrelated to node_features but related to the performance issue here, do you know if GRES autodetection will also be triggered again on periodic node re-registration? Having many calls to NVML for finding available GPUs could also impact user jobs.

> do you know if GRES autodetection will also be triggered again on periodic node re-registration?

AFAIU no, only when the config is generated for gpu.c or gres.c. When used in gres_gpu.c, it only queries node_flags to know the bits set.

Felix,

We're actively working in this improvement and now it's being reviewed.

I'll let you know once it's released.

Regards,
Carlos.

Got it, thanks for the update!

Felix,

Still working in this improvement. It's not stalled.

Regards,
Carlos.

Felix,

Happy to say fixes have been pushed ahead in master:

*   a298b1d46f (HEAD -> master, origin/master, origin/HEAD) Merge branch 'bug13545'
|\  
| * afa04f5608 (bug13545) Adding NEWS entry for the last 2 commits
| * 1140899487 slurmd - Refresh features cache if scontrol reconfig
| * 0da310d444 slurmd - Cache features at startup
|/

Going to mark the bug as resolved.

Cheers,
Carlos.

I confirm that it seems fixed on master, thank you!