Cache: reduce sleep time

[matthiasdiener]

CC: #503.

[inducer]

That won't work reliably. / unconditionally has a floating point result, and comparing floating point numbers to zero is unreliable.

[matthiasdiener]

That won't work reliably. / unconditionally has a floating point result, and comparing floating point numbers to zero is unreliable.

4d13b82 should fix this

[yxliang01]

Actually, what about don't do polling at all, i.e. no sleep. There's a lot more efficient system calls available like inotify and seemingly good quality python package watchdog (never use myself though) to do this cross-platform.

[inducer]

Thanks for the suggestion. IMO, anytime there is contention for this lock, you're running your program poorly. (I.e. this should always be avoidable, e.g. by setting a private cache folder.) As such, I'm not very inclined to optimize that case.

[matthiasdiener]

Thanks for the suggestion. IMO, anytime there is contention for this lock, you're running your program poorly. (I.e. this should always be avoidable, e.g. by setting a private cache folder.) As such, I'm not very inclined to optimize that case.

A bit of background on this PR: We keep seeing this timeout a lot when running multiple MPI ranks on a single node, which is a difficult situation to set private cache folders (especially when running interactively).

[inducer]

What's the motivation for this change? It seems really short. If you were running this (by mistake, say) on 1000 nodes sharing a cache directory, I think the resulting thrashing would not be a good thing.

[matthiasdiener]

The specific value was taken from https://github.com/tox-dev/py-filelock/blob/a6c8fabc4192fa7a4ae19b1875ee842ec5eb4f61/src/filelock/_api.py#L113. I can't really predict what the result of this with thousands of nodes would be, but we already keep on hitting the 1 second timeout when running with 2 ranks on a single node, which I guess indicates that the existing polling time would cause minutes-long waiting times when running with thousands of ranks.

[inducer]

I can see that argument. Could you add a comment justifying this choice of time-out value? (making reference to the single-node interactive "test" use case)

[matthiasdiener]

Added in 387011e