-
Notifications
You must be signed in to change notification settings - Fork 552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Assorted improvements to self-test #8695
Merged
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
- For when a node is up, but contains no cached results
- Reversing parallelism & duration members
- Fixes a bug where the netcheck benchmark would run in both directions, i.e. given two nodes 0 & 1, the benchmark would run twice, with one machine as the client the other as a server, then vice versa. - The actual desired behavior of this method is to have it generate pairs of node ids where no two pairs occur more then once.
- Test that the network_test_plan method for self-test produces unique pairs of node identifiers.
r-vasquez
reviewed
Feb 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rpk changes look good to me, however, I'll wait for the core review 😄 Thanks!
dotnwat
reviewed
Feb 7, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c++ bits lgtm
- Hang occured because the code waits for all outstanding work to leave the gate before stop() is called on any jobs - The solution is to call close but don't wait on it. Then return the future returned by close.
- The benchmarks were catching soft errors such as `benchmark_aborted` (only called on stop()) and logging at error, making ducktape tests fail when things were working as expected. Modifying these exceptions to print at debug level. - There is already a catch all exception handler in self_test_backend.cc that logs at warn. This can catch the hard errors and log at error. No need to have the tests intercept and rethrow. - Also handle possible gate_closed_exceptions so they don't percolate as hard errors.
graphcareful
force-pushed
the
self-test-nits
branch
from
February 7, 2023 22:19
31bf096
to
2831339
Compare
dotnwat
approved these changes
Feb 7, 2023
r-vasquez
approved these changes
Feb 7, 2023
andrwng
approved these changes
Feb 7, 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.
This PR fixes some small bugs and adds some quality of life improvements to redpanda
self-test
such as:Backports Required
Release Notes
Bug Fixes