Fix panic due to error in HasWork #2456
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 panic was happening in the `Init` but it was due to the fact that HasWork wrongfully returned True when it should've returned false.
mstoykov
force-pushed
the
fixPanicsWithExecutionSegments
branch
from
211b462
to
029f6f5
Compare
|
While this is fixing the particular case that we know of, there are other places where I have trouble remembering why we decided to leave both given that segment.Scale has known problems with splitting work 🤔 |
|
I can't find any discussion about the topic :( but here is a PR (based on this one) that removes the Segment.Scale and not test fail so 🤷 Maybe it's good idea to have only one implementation ... |
Most executors use that already and while ExecutionSegment.Scale gives the same result in a lot of cases - it doesn't in some. This potentially can lead to small (1 VU/iteration) discrepancies but more importantly panics. Especially in cases where the differences are when one returns a 0 and the other a 1 for a given value.
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 panic was happening in the
Initbut it was due to the fact thatHasWork wrongfully returned True when it should've returned false.
Changelog entry: