-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Execute testDynamicFiltering*
exclusively
#18850
Execute testDynamicFiltering*
exclusively
#18850
Conversation
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.
% comment
PARTITIONED, | ||
isAggregationPushedDown); | ||
executeExclusively(() -> { | ||
MaterializedResultWithQueryId resultWithQueryId = getDistributedQueryRunner() |
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.
please extract methods instead of long lambdas. The diff will be more human friendly.
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.
For extracted methods I'll add Flaky
suffix for existing method names
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.
Please don't. Flaky
"means" non deterministic. Whereas we know how it behaves and it is deterministic. I think the better name would be Unsafe
which is common term in regards to concurrency. However here I think a code comment would be simply the best.
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.
Method signatures are the same as of original methods, so I've used Unsafe
suffix.
This should mitigate flakiness. Relates to f3c41a4
c77a9b8
to
c886e9e
Compare
This should mitigate flakiness.
Relates to f3c41a4
#18323
#18499
Description
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: