Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Hello @casassg ,
Frankly, the original piece of code looks like terribly over complicated solution for an easy problem.
I'm hesitant to add more to it.

Could you please explain your motivation for this fix, so we could prioritize it (move forward with your fix or invest into better solution) ?

At a glance, this cache is "append only" :

• records never deleted;
• records never change (all rewrites are to the same value), due to immutable nature of instance template.

So access to this cache shouldn't involve any synchronizations. One way would be to stop using shelve and have directory where each file is dedicated to one cached instance template.
The content of file is either JSON (preferable) or pickled (makes for easier change).
To avoid "partial writes/reads" the addition of records could be done through (write_tmp + mv)

@mr0re1 agree w your assessment that this piece of code is more complex than needed, however it breaks allowing for partition updates without node reconstruction.

Issue (I think) is that in the initial run this shelve cache is created as root (the chown_slurm does basically nothing as the file its chowing does not exist). This means that on subsequent runs of the slurm management service which checks for changes in config files, when the update happens it can't write the cache. This leads to cache being essentially only the one from the initial run with partition changes which should be possible to update not updating at all.

to repro:

• Deploy slurm w partition p1 with nodes a1-[0-1]
• Change p1 to add new nodes (or something which should be an accepted change). Deploy change to cluster.
• See controller logs for the above error.

regarding and have directory where each file is dedicated to one cached instance template agree as well. But tried to do the minimal change that fixed the above issue

@casassg , I've missed linked issue, thank you for reporting it!
This is serious issue, I'm inclined to take in your solution.

/gcbrun

great! lmk when this gets released so I can pull it from my side. I think this requires a full controller recreation (so that startup scripts are pulled)