[backend/filestate] Allow preview on locked stack #8642
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The httpstate backend allows previews to proceed even while an update is in progress. This is potentially problematic as the preview may be relative to a partial state of the stack, but ensures that previews will not be blocked on stacks that have long running updates (for example, to allow for concurrent PR jobs to preview changes). This behaviour has been consistent ~forever for the httpstate backend. In the filestate backend, we recently introduced locking, using a quite different (more coarse-grained) approach. As part of this implementation, preview was added to the list of operations that require an exclusive lock on the stack. For consistency, we should loosen this so that preview behaves the same relative to state locking in the filestate and httpstate backends. In the future, we may well want to tighten this up for both backends, with some additional user controls. Also, when the update plans feature lands shortly, that will provide some additional helpful guarantees that a previous preview was not accidentally relative to a partial state. Fixes #8609.
lukehoban
commented
Dec 28, 2021
| @@ -456,6 +457,25 @@ func TestLocalStateLocking(t *testing.T) { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| // Run 10 concurrent previews | |||
There was a problem hiding this comment.
Added test coverage here - but this test is skipped currently due to #7269. I verified locally that this test correctly fails before the change and succeeds after the change when un-skipped.
Codecov Report
@@ Coverage Diff @@
## master #8642 +/- ##
==========================================
- Coverage 58.80% 56.39% -2.41%
==========================================
Files 634 540 -94
Lines 97295 91931 -5364
Branches 1383 1383
==========================================
- Hits 57216 51847 -5369
- Misses 36823 36878 +55
+ Partials 3256 3206 -50
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
httpstatebackend allows previews to proceed even while an update is in progress. This is potentially problematic as the preview may be relative to a partial state of the stack, but ensures that previews will not be blocked on stacks that have long running updates (for example, to allow for concurrent PR jobs to preview changes). This behaviour has been consistent ~forever for thehttpstatebackend.In the
filestatebackend, we recently introduced locking, using a quite different (more coarse-grained) approach. As part of this implementation, preview was added to the list of operations that require an exclusive lock on the stack.For consistency, we should loosen this so that preview behaves the same relative to state locking in the
filestateandhttpstatebackends.In the future, we may well want to tighten this up for both backends, with some additional user controls. Also, when the update plans feature lands shortly, that will provide some additional helpful guarantees that a previous preview was not accidentally relative to a partial state.
Fixes #8609.