fix: parallel plan and apply also in a single workspace (rebased) #5264
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
plentydone
requested review from
jamengual,
lukemassa and
nitrocode
and removed request for
a team
github-actions
bot
added
go
plentydone
changed the title
|
I'm not sure what the etiquette is around signing off on this as the committer given that I just rebased someone else's work from their own MR. |
2 tasks
just sign it with yours |
We generate a list of all interesting directories, so we can target the locks to the affected directories instead of using a (too) global lock Signed-Off-By: Andrew Carter <[email protected]>
There is a race condition here where we test if we are current, and only then if we are not current we grab the lock. In the meantime, that information could be stale. Extend the lock to cover all operations, and unconditionally wait for the lock. We can't assume anything can be skipped if we have to wait for the lock. Signed-Off-By: Andrew Carter <[email protected]>
All Clone() calls that have signaled an interest in merging before another Clone() checks whether a merge is necessary can skip their own checks. This should reduce the thundering herd problem at the beginning of large paralell runs. Signed-Off-By: Andrew Carter <[email protected]>
Clone is now a NOP if the PR has not changed, and loses its second return value, the MergedAgain flag. MergeAgain must be called explicitly in the only location that cares about this flag, just before planning. This cleans up the code for Clone and re-merging a bit. Also regenerated mocks Signed-Off-By: Andrew Carter <[email protected]>
plentydone
force-pushed
the
parallel-plans
branch
from
671efda to
cde1fe5
Compare
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.
what
@finnag did all the real work here. I just rebased and regenerated mocks. I'm new to this codebase, but I've reviewed the code and it seems reasonable to me, and I've reviewed all the commits between October and today and I see no logical conflicts. If in fresh review any changes are needed, I'm happy to do so.
From the original PR description:
why
tests
I ran
make test-all fmt lintreferences
This is just #3670 rebased