Skip to content

Conversation

@Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Aug 31, 2023

  • Make multi-stage leaf operator use the streaming operator from single-stage engine to avoid memory issue and also improve efficiency.
  • Applied back pressure to the streaming operator
  • Allow streaming only for selection without order-by
  • Fix the execution stats override issue

Documentation

Added new query option maxStreamingPendingBlocks to control the pending blocks for streaming (back pressure, 100 by default)

Incompatible

The following interface changed:

  • BaseResultsBlock
  • PlanMaker
  • QueryExecutor

@Jackie-Jiang Jackie-Jiang added enhancement documentation incompatible Indicate PR that introduces backward incompatibility bugfix multi-stage Related to the multi-stage query engine labels Aug 31, 2023
@Jackie-Jiang Jackie-Jiang requested a review from xiangfu0 August 31, 2023 08:04
@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Merging #11472 (f536386) into master (3858787) will decrease coverage by 0.04%.
Report is 1 commits behind head on master.
The diff coverage is 81.18%.

@@             Coverage Diff              @@
##             master   #11472      +/-   ##
============================================
- Coverage     62.95%   62.91%   -0.04%     
- Complexity     1098     1106       +8     
============================================
  Files          2319     2320       +1     
  Lines        124404   124476      +72     
  Branches      18996    19004       +8     
============================================
- Hits          78316    78313       -3     
- Misses        40514    40568      +54     
- Partials       5574     5595      +21     
Flag Coverage Δ
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 14.42% <1.39%> (-48.50%) ⬇️
java-17 62.90% <81.18%> (+0.11%) ⬆️
java-20 14.41% <1.39%> (-48.37%) ⬇️
temurin 62.91% <81.18%> (-0.04%) ⬇️
unittests 62.90% <81.18%> (-0.04%) ⬇️
unittests1 67.36% <80.91%> (-0.07%) ⬇️
unittests2 14.45% <1.39%> (-0.01%) ⬇️

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

Files Changed Coverage Δ
...core/operator/blocks/results/BaseResultsBlock.java 93.87% <ø> (ø)
...operator/blocks/results/ExceptionResultsBlock.java 50.00% <0.00%> (-4.55%) ⬇️
.../operator/blocks/results/MetadataResultsBlock.java 33.33% <0.00%> (+33.33%) ⬆️
...not/core/operator/combine/BaseCombineOperator.java 91.11% <ø> (-0.20%) ⬇️
...tor/streaming/StreamingGroupByCombineOperator.java 0.00% <0.00%> (ø)
...pache/pinot/core/query/executor/QueryExecutor.java 100.00% <ø> (+87.50%) ⬆️
...che/pinot/core/transport/grpc/GrpcQueryServer.java 0.00% <0.00%> (ø)
.../core/transport/grpc/GrpcResultsBlockStreamer.java 0.00% <0.00%> (ø)
...va/org/apache/pinot/spi/utils/CommonConstants.java 21.11% <ø> (ø)
...e/pinot/common/utils/config/QueryOptionsUtils.java 68.75% <50.00%> (-0.61%) ⬇️
... and 39 more

... and 28 files with indirect coverage changes

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

@Jackie-Jiang Jackie-Jiang force-pushed the multi_stage_stream branch 3 times, most recently from 544838f to 4aa19e5 Compare August 31, 2023 22:19
@Jackie-Jiang Jackie-Jiang marked this pull request as ready for review September 1, 2023 02:00
@Jackie-Jiang Jackie-Jiang force-pushed the multi_stage_stream branch 9 times, most recently from 8145adb to 3ea2af3 Compare September 1, 2023 08:46
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to put queryContext into BaseResultsBlock with a constructor and use super here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, but it doesn't apply to metadata and exception block, so I kept it only in data blocks

@Jackie-Jiang Jackie-Jiang merged commit d8d1e92 into apache:master Sep 1, 2023
@Jackie-Jiang Jackie-Jiang deleted the multi_stage_stream branch September 1, 2023 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix documentation enhancement incompatible Indicate PR that introduces backward incompatibility multi-stage Related to the multi-stage query engine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants