Skip to content
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

Add query interruption flag check to broker groupby reduction #9499

Merged
merged 7 commits into from
Sep 30, 2022

Conversation

jasperjiaguo
Copy link
Contributor

@jasperjiaguo jasperjiaguo commented Sep 29, 2022

Check for interruption during broker reduction on groupby results. So that after the timeout the worker threads can be cleaned up timely.

Screen Shot 2022-09-29 at 9 51 43 PM

@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2022

Codecov Report

Merging #9499 (3ecdb3a) into master (d2619c1) will increase coverage by 3.73%.
The diff coverage is 80.00%.

@@             Coverage Diff              @@
##             master    #9499      +/-   ##
============================================
+ Coverage     63.55%   67.28%   +3.73%     
+ Complexity     5115     4999     -116     
============================================
  Files          1862     1421     -441     
  Lines         99733    74215   -25518     
  Branches      15212    11850    -3362     
============================================
- Hits          63387    49938   -13449     
+ Misses        31676    20657   -11019     
+ Partials       4670     3620    -1050     
Flag Coverage Δ
unittests1 67.28% <80.00%> (+0.19%) ⬆️
unittests2 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...not/core/query/reduce/GroupByDataTableReducer.java 63.80% <80.00%> (-0.26%) ⬇️
...ore/query/scheduler/resources/ResourceManager.java 84.00% <0.00%> (-12.00%) ⬇️
...e/pinot/query/runtime/operator/FilterOperator.java 60.00% <0.00%> (-4.00%) ⬇️
...gregation/function/StUnionAggregationFunction.java 73.52% <0.00%> (-2.95%) ⬇️
.../pinot/core/query/scheduler/PriorityScheduler.java 77.77% <0.00%> (-2.78%) ⬇️
...or/transform/function/IsNullTransformFunction.java 72.41% <0.00%> (-2.59%) ⬇️
...he/pinot/common/utils/config/TableConfigUtils.java 76.68% <0.00%> (-2.40%) ⬇️
...transform/function/IsNotNullTransformFunction.java 62.06% <0.00%> (-2.22%) ⬇️
...ders/forward/VarByteChunkMVForwardIndexReader.java 77.57% <0.00%> (-1.87%) ⬇️
...he/pinot/spi/utils/builder/TableConfigBuilder.java 82.23% <0.00%> (-1.10%) ⬇️
... and 483 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@siddharthteotia
Copy link
Contributor

let's try power of 2 batch size once and assess impact of modulo v/s bit shift

@siddharthteotia siddharthteotia merged commit b026d32 into apache:master Sep 30, 2022
61yao pushed a commit to 61yao/pinot that referenced this pull request Oct 3, 2022
…#9499)

* add query interruption flag check to broker groupby reduction

* add query interruption flag check to broker groupby reduction

* add query interruption flag check to broker groupby reduction

* add benchmark

* tiled loop

* add benchmark

* Trigger Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants