lock timeouts #13262
lock timeouts #13262
Conversation
Since moving to the new backends, all states (except InmemState) are Lockers. Add the methods to the State interface to remove a heap of assertion checks.
All states are lockers, so get rid of extra asertions.
LockWithContext will retry a lock until the context expires or is cancelled. This will let us implement a `-lock-timeout` flag, and make use of existing contexts when applicable.
- Have the ui Lock helper use state.LockWithContext. - Rename the message package to clistate, since that's how it's imported everywhere. - Use a more idiomatic placement of the Context in the LockWithContext args.
Add fields required to create an appropriate context for all calls to clistate.Lock. Add missing checks for Meta.stateLock, where we would attempt to lock, even if locking should be skipped.
shouldn't be listed as a common command
Add the -lock-timeout flag to the appropriate commands. Add the -lock flag to `init` and `import` which were missing it. Set both stateLock and stateLockTimeout in Meta.flagsSet, and remove the extra references for clarity.
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
ac13d14
to
5eca913
Compare
Backoff the Lock calls exponentially, to a reasonable limit.
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
51a0e5f
to
83ab684
Compare
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
83ab684
to
3d60485
Compare
|
yeah, I usually prefer to create hooks to cause time-related tests to step through the possible states without Sleep (and I've been known to habitually refactor Sleeps out of exiting tests 😉 ). I could pretty easily drop the sleep from the in-package test (which I think I'll do now), but agree adding the hooks for external testing doesn't seem worth it, especially since that is using an external process to lock the file as well. |
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
23668c2
to
0855e29
Compare
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
0855e29
to
af2e289
Compare
jbardin
force-pushed
the
jbardin/lock-timeouts
branch
from
397ec87
to
7cfb515
Compare
|
Just tacked on the doc changes here too |
|
The docs look good to me too! 😀 |
|
@jbardin If you run the |
|
@brikis98, the The locks by design are intended to fail fast, protecting the state files from concurrent mutations, and the lock implementations themselves don't provide any blocking behavior. This flag was primarily added for the case where automated systems may want to retry the operation automatically if a lock was already taken, and we often rely on flags over configuration for automated systems. This is not meant to solve the problem for highly concurrent systems, since the locks themselves can't block, and therefor can't provide a deterministic serialization of waiting clients. Not providing the flag can't cause any additional errors, it just reports the lock status to the user on the first attempt. |
|
Got it, thanks. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Adds a
-lock-timeoutparameter, which will retry a state lock for at least the duration of the timeout.The actual blocking retry loop is provided by the
state.LockWithContextfunction. State implementations themselves still don't need to be able to handle blocking on locks or timeouts.Having a way to retry a lock, up until a timeout, would allow terraform automation processes to run concurrently without immediately failing.
Some additional cleanup:
force-unlockcommand to "plumbing"-lockoption toimportandinit-lock=false.