Skip to content

Conversation

@Jackie-Jiang
Copy link
Contributor

Currently in the combine operator all the segments are pre-assigned to the threads. If one thread finishes early, it won't be able to pick up more segments.
This PR makes the threads pick up segments once the previous one is finished

@siddharthteotia
Copy link
Contributor

Good one.
cc @jasperjiaguo

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2022

Codecov Report

Merging #9387 (81e25f3) into master (45cc5dd) will decrease coverage by 34.67%.
The diff coverage is 83.33%.

@@              Coverage Diff              @@
##             master    #9387       +/-   ##
=============================================
- Coverage     69.75%   35.08%   -34.68%     
+ Complexity     5095      185     -4910     
=============================================
  Files          1885     1885               
  Lines        100406   100411        +5     
  Branches      15277    15277               
=============================================
- Hits          70042    35227    -34815     
- Misses        25414    62178    +36764     
+ Partials       4950     3006     -1944     
Flag Coverage Δ
integration1 26.12% <83.33%> (-0.07%) ⬇️
integration2 ?
unittests1 ?
unittests2 15.35% <0.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
...reaming/StreamingSelectionOnlyCombineOperator.java 0.00% <0.00%> (-66.00%) ⬇️
...not/core/operator/combine/BaseCombineOperator.java 79.01% <100.00%> (-7.24%) ⬇️
...perator/combine/GroupByOrderByCombineOperator.java 65.65% <100.00%> (-17.00%) ⬇️
...nMaxValueBasedSelectionOrderByCombineOperator.java 71.21% <100.00%> (-1.31%) ⬇️
...in/java/org/apache/pinot/spi/utils/BytesUtils.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/user/RoleType.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1090 more

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

@jasperjiaguo
Copy link
Contributor

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants