Skip to content

[Multi-stage] Optimize mailbox info in query plan #12382

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.

Sign up for GitHub

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

Jump to bottom
Merged
Jackie-Jiang merged 2 commits into from
Feb 8, 2024
Merged

[Multi-stage] Optimize mailbox info in query plan #12382

Jackie-Jiang merged 2 commits into from
Feb 8, 2024

Conversation

@Jackie-Jiang
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang commented Feb 8, 2024 •
edited
Loading

Extended optimization over #12370
Further reduce the footprint of mailbox info by:

  • Only keep the worker id instead of the full virtual server address
  • Group multiple workers on the same physical server together
  • Share serialized mailbox infos

Backward Incompatible

This PR changes the proto object structure, which will cause backward incompatibility when broker and server are running different version. Since we are not maintaining backward compatibility for multi-stage engine as of now, user should upgrade both brokers and servers, then start querying.

@Jackie-Jiang Jackie-Jiang added enhancement release-notes Referenced by PRs that need attention when compiling the next release notes backward-incompat Referenced by PRs that introduce or fix backward compat issues multi-stage Related to the multi-stage query engine labels Feb 8, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2024 •
edited
Loading

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (8434158) 61.74% compared to head (88a1f04) 61.69%.

Files Patch % Lines
...ery/planner/physical/MailboxAssignmentVisitor.java 79.16% 9 Missing and 1 partial ⚠️
...va/org/apache/pinot/query/runtime/QueryRunner.java 76.47% 4 Missing ⚠️
...apache/pinot/query/service/server/QueryServer.java 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #12382      +/-   ##
============================================
- Coverage     61.74%   61.69%   -0.05%     
+ Complexity     1142      207     -935     
============================================
  Files          2425     2428       +3     
  Lines        132715   132737      +22     
  Branches      20532    20540       +8     
============================================
- Hits          81942    81892      -50     
- Misses        44759    44829      +70     
- Partials       6014     6016       +2
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (ø)
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 61.65% <91.75%> (-0.01%) ⬇️
java-21 61.58% <91.75%> (-0.05%) ⬇️
skip-bytebuffers-false 61.67% <91.75%> (-0.06%) ⬇️
skip-bytebuffers-true 61.55% <91.75%> (-0.01%) ⬇️
temurin 61.69% <91.75%> (-0.05%) ⬇️
unittests 61.69% <91.75%> (-0.05%) ⬇️
unittests1 46.85% <91.75%> (-0.02%) ⬇️
unittests2 27.72% <0.00%> (-0.04%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jackie-Jiang Jackie-Jiang force-pushed the optimize_plan_mailbox branch from 05c1d15 to 88a1f04 Compare February 8, 2024 07:28
@kishoreg
Copy link
Member

kishoreg commented Feb 8, 2024

Is there a way to maintain backwards compatibility?

@Jackie-Jiang
Copy link
Contributor Author

Jackie-Jiang commented Feb 8, 2024

@kishoreg Yes, but probably not worth it. We can add new fields and let servers handle both old and new fields. It will take 2 releases to roll out any change.
For multi-stage engine, with current development speed, maintaining backward compatibility takes too much overhead, thus we have clear called out that we won't maintain backward compatibility during upgrade (nodes running different versions).

@Jackie-Jiang Jackie-Jiang merged commit 2d41b38 into apache:master Feb 8, 2024
@Jackie-Jiang Jackie-Jiang deleted the optimize_plan_mailbox branch February 8, 2024 23:32
suyashpatel98 pushed a commit to suyashpatel98/pinot that referenced this pull request Feb 28, 2024
@Jackie-Jiang @suyashpatel98
[Multi-stage] Optimize mailbox info in query plan (apache#12382)
fbf12ff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xiangfu0 xiangfu0 xiangfu0 approved these changes

@walterddr walterddr Awaiting requested review from walterddr

Assignees

No one assigned

Labels

backward-incompat Referenced by PRs that introduce or fix backward compat issues enhancement multi-stage Related to the multi-stage query engine release-notes Referenced by PRs that need attention when compiling the next release notes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Jackie-Jiang @codecov-commenter @kishoreg @xiangfu0