Locking during download and install

[yermulnik]

Would it make sense to implement locking for TF archive and signature/checksum files during download and for destination TF file during install to avoid possible race conditions for when tfswitch is run for the same TF version be several different means (e.g. within two different CI pipelines inside the same CI worker)?

@warrensbox @MatrixCrawler @crablab @MatthewJohn @jukie

[kvendingoldo]

If you will do implementation for this, start from unit tests. We made the similar functionality some time ago for tenv in scope of tofuutils/tenv#116 and found interesting differences between os.O_WRONLY|os.O_CREATE and os.O_CREATE|os.O_EXCL.

[yermulnik]

Thank you for the heads-up.
Indeed recently added locking in tenv pushed me to think whether we want the same in tfswitch.

[warrensbox]

Thanks!

[warrensbox]

@yermulnik lets plan this for a future release

[MatthewJohn]

Given that we write to quite a few files, i.e. checksum, RECENT, public keys, the binary and the temporary files when installing a new version, I wonder if it's worth implementing a dedicated lock file (maybe a PID file on linux). This could be created/locked when performing any "writeable" action (i.e. selecting a version, rather dry-run).
If we lock individual files, I assume we'd want to lock them before we begin the download the content, because if two processes download the same file and then try to write them, if they only lock at the writing stage it wastes bandwidth downloading the same file twice.

This does make me ask a larger question though, since tfswitch's main function is to set a symlink pointing to a version, if it's being run twice simultaneously, either:

• Two different versions are being downloaded at the same time, which is interesting because one will immediately overwrite the other's symlink
• The same version is being installed, which adding a global lock could well help, because the entire functionality could(/would) be paused until the first version is installed and the second process wouldn't get "caught up" half-way through the process (just because one process was attempting to write to a file)

If we did go down this method, though, we'd need to: lock the file immediately, have a timeout waiting for the lock and we'd need to re-evalulate very carefully to ensure the lock file is removed in any failure cases - this might means moving away from Fatals (everywhere!)?

[yermulnik]

I wonder if it's worth implementing a dedicated lock file (maybe a PID file on linux). This could be created/locked when performing any "writeable" action (i.e. selecting a version, rather dry-run).

This sounds meaningful. Global though meaningful.
If we decide to go with global lock, I think it is worth of putting lock based on bin value so that people on a shared server don't step on each other's toes when using something like bin = "$HOME/bin/terraform".

ensure the lock file is removed in any failure cases - this might means moving away from Fatals (everywhere!)?

Or have a cleanup function defer'red at the start of each writable function?
(does defer gets triggered on Fatal in Golang? it probably should 🤔)

My option is sort of a dirty hack though. Though might require less effort and re-factoring 🤞🏻

[MatthewJohn]

I guess it depends on the result of (does defer gets triggered on Fatal in Golang? it probably should 🤔) :D That will likely might a large difference :)

If we decide to go with global lock, I think it is worth of putting lock based on bin value so that people on a shared server don't step on each other's toes when using something like bin = "$HOME/bin/terraform".

👍

[yermulnik]

I guess it depends on the result of (does defer gets triggered on Fatal in Golang? it probably should 🤔) :D That will likely might a large difference :)

It doesn't 😢
https://pkg.go.dev/os#Exit

The program terminates immediately; deferred functions are not run.