Make dirty runs dirtier #683
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
Previously, `Deploy(t, 1)` would deploy 1 HS in 1 isolated blueprint which could be reused by other tests who also call `Deploy(t, 1)`. This pattern extended to `Deploy(t, 2)`, which would deploy 2x HSes in 1 isolated blueprint. This however means we'd run more containers in parallel. What's worse: these containers would only be cleaned up at the end of the entire test package run: `defer deployment.Destroy(t)` would do nothing. This was Very Bad on GHA boxes as they have limited resources, causing synapse/workers to fail to complete.
devonh
approved these changes
Oct 19, 2023
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.
Previously,
Deploy(t, 1)would deploy 1 HS in 1 isolated blueprint which could be reused by other tests who also callDeploy(t, 1). This pattern extended toDeploy(t, 2), which would deploy 2x HSes in 1 isolated blueprint. This however means we'd run more containers in parallel. What's worse: these containers would only be cleaned up at the end of the entire test package run:defer deployment.Destroy(t)would do nothing.This was Very Bad on GHA boxes as they have limited resources, causing synapse/workers to fail to complete.
The cost of this aggressive reuse of dirty containers is slightly more ugliness internally in Complement, as we now entirely have separate code paths for handling dirty stuff. This can be refactored later down the line to demarcate the difference between this and blueprints.