Possible concurrency issues with resolve on a cluster

[amilsted]

I think I am seeing multiple processes trying to simultaneously resolve julia versions and dependencies, mirroring JuliaPy/CondaPkg.jl#40. Maybe locking is needed here too?

[cjdoris]

Yeah makes sense.

[amilsted]

Oh, by the way, I wonder if "pidfile" locking makes sense on a cluster? I think it probably doesn't, since there is no global list of processes.

[cjdoris]

I just took a look at the source:

https://github.com/vtjnash/Pidfile.jl/blob/67c3f23b25d4e43f1f5785ce77c2cac6952fefcf/src/Pidfile.jl#L128

Pidfile.jl uses both the PID and hostname, which presumably means it works with shared filesystems too.

[amilsted]

Ah, that's smart! Much better than https://github.com/mosquito/python-pidfile/blob/master/pidfile/pidfile.py

https://github.com/keiranmraine/pidlock sees to use hostname too.

[cjdoris]

Oh yeah of course this is a python package so need a python implementation of pidfiles.

[cjdoris]

pidlock looks good thanks

[amilsted]

Btw, is it really necessary to write out the meta file every time? Perhaps we could check for changes first?

[amilsted]

The situation is much better now that we fall back on hashing juliapkg.json files when there is an mtime mismatch and also don't write out the meta file if there have been no changes. Although there are still situations in which concurrency issues would occur (if multiple processes are simultaneously trying to do the initial setup of an env), perhaps it is now enough to recommend users always initialize on one process before e.g. spawning many cluster jobs.

[cjdoris]

Good to know thanks, I don't do any distributed programming myself. I'll leave this issue open as a reminder to do pidlocking though, it's still a good idea.

Recommending resolving first makes sense. Could put something into the readme and also the message that appears when waiting for the lock.

[willow-ahrens]

I was getting a similar error in CI when multiple pytest runners all tried to import juliapkg. We ended up with FileExistsError because multiple runners had deleted and then tried to copytree a julia installation into the conda environment. We fixed this using your recommended approach (call import juliapkg on one process first).

However, a pidfile fix would be more robust, and not require downstream packages to be careful about including packages which depend on juliapkg. I am willing to contribute a PR, and want to double check that there is the will to review. The approach would be:

1. use PidLock
2. lock the installer (everything in install_julia)
3. lock the resolver (everything in resolve)
4. keep the lock file in the environment directory (the same place that we would put install)
5. if the lock cannot be aquired after 1 second, prints a debug message that we are waiting on a julia install, and suggests that we might consider installing on one thread first.