Revert #2161 #2187
Merged
Revert #2161 #2187
Conversation
This file contains hidden or 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
Kobzol
changed the title
Member
Author
|
For future reference: the bug can be reproduced with the |
Mark-Simulacrum
approved these changes
Jul 6, 2025
Member
Author
|
I just realized, if we revert this, and don't land #2188, then we'll be potentially skipping more test cases (because the old logic would just skip everything once). But it should always be at most one benchmark (crate) skipped, so hopefully that's still not so terrible. |
Member
Author
|
Landing as a hotfix, it should help us make progress on the collector in the face of S3 upload failures. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This reverts #2161. I noticed that the recomputation logic has some issue on production, and it needlessly reruns stuff when a benchmark was restarted. Also the computation of step durations on the status page suffers when a benchmark is restarted, because we now restart steps, which is something the status page wasn't really prepared for.
The recomputation bug could be fixed, but now I think that we should merge this PR only after we have the new system, to avoid the problems with the step durations.