Skip to content

Upgrade Helix to 1.0.4 in Pinot #8325

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
jackjlli merged 1 commit into from
Jul 21, 2022
Merged

Upgrade Helix to 1.0.4 in Pinot #8325

jackjlli merged 1 commit into from
Jul 21, 2022

Conversation

@jackjlli
Copy link
Member

@jackjlli jackjlli commented Mar 9, 2022 •
edited
Loading

Description

This PR upgrades Helix version from 0.9.8 to 1.0.4 in Pinot.

Related issue: #8306

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

  • Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

  • Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

  • New configuration options
  • Deprecation of configurations
  • Signature changes to public methods/interfaces
  • New plugins added or old plugins removed
  • Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

Documentation

@jackjlli jackjlli force-pushed the helix-1.0 branch 2 times, most recently from eb963cf to e0930b0 Compare March 9, 2022 06:51
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2022 •
edited
Loading

Codecov Report

Merging #8325 (af4c88b) into master (c006f59) will decrease coverage by 6.47%.
The diff coverage is 67.85%.

@@             Coverage Diff              @@
##             master    #8325      +/-   ##
============================================
- Coverage     70.09%   63.62%   -6.48%     
+ Complexity     4972     4720     -252     
============================================
  Files          1831     1784      -47     
  Lines         96428    94867    -1561     
  Branches      14413    14381      -32     
============================================
- Hits          67596    60358    -7238     
- Misses        24173    30187    +6014     
+ Partials       4659     4322     -337
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 66.93% <0.00%> (+0.06%) ⬆️
unittests2 15.32% <67.85%> (-0.13%) ⬇️

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

Impacted Files Coverage Δ
...ache/pinot/broker/broker/AccessControlFactory.java 73.33% <ø> (-6.67%) ⬇️
...broker/broker/ZkBasicAuthAccessControlFactory.java 0.00% <ø> (ø)
...e/pinot/broker/broker/helix/BaseBrokerStarter.java 71.83% <ø> (-7.99%) ⬇️
.../BrokerResourceOnlineOfflineStateModelFactory.java 41.86% <ø> (-13.96%) ⬇️
...quota/HelixExternalViewBasedQueryQuotaManager.java 62.78% <ø> (-6.28%) ⬇️
...che/pinot/broker/routing/BrokerRoutingManager.java 59.60% <ø> (-25.71%) ⬇️
...elector/SegmentLineageBasedSegmentPreSelector.java 100.00% <ø> (ø)
.../segmentpreselector/SegmentPreSelectorFactory.java 100.00% <ø> (ø)
...oker/routing/segmentpruner/EmptySegmentPruner.java 87.93% <ø> (ø)
...mentpruner/MultiPartitionColumnsSegmentPruner.java 73.91% <ø> (ø)
... and 522 more

@jackjlli jackjlli force-pushed the helix-1.0 branch 4 times, most recently from 26c052c to bf3f61e Compare March 11, 2022 01:32
@jackjlli jackjlli force-pushed the helix-1.0 branch 13 times, most recently from be39375 to c3e967a Compare March 25, 2022 00:25
@jackjlli jackjlli force-pushed the helix-1.0 branch 4 times, most recently from 9465977 to def58dc Compare March 26, 2022 00:54
@jackjlli jackjlli force-pushed the helix-1.0 branch 4 times, most recently from 926257a to 34cf23c Compare March 29, 2022 05:38
@jackjlli jackjlli force-pushed the helix-1.0 branch 2 times, most recently from fea5b65 to ae9a1dc Compare June 10, 2022 16:20
@jackjlli jackjlli changed the title [WIP] Upgrade Helix to 1.0.3 in Pinot (not ready yet) [WIP] Upgrade Helix to 1.0.4 in Pinot (not ready yet) Jun 10, 2022
@jackjlli jackjlli force-pushed the helix-1.0 branch 6 times, most recently from 2f7cc7d to aa1aca5 Compare June 23, 2022 17:31
@jackjlli jackjlli force-pushed the helix-1.0 branch 5 times, most recently from 19a2f55 to 04aeeee Compare June 30, 2022 18:21
@Jackie-Jiang Jackie-Jiang mentioned this pull request Jul 11, 2022
Closed
@jackjlli jackjlli force-pushed the helix-1.0 branch 2 times, most recently from 40ed79e to c45edbf Compare July 18, 2022 21:29
@jackjlli jackjlli changed the title [WIP] Upgrade Helix to 1.0.4 in Pinot (not ready yet) Upgrade Helix to 1.0.4 in Pinot Jul 19, 2022
@jackjlli jackjlli marked this pull request as ready for review July 19, 2022 22:08
@jackjlli
Copy link
Member Author

jackjlli commented Jul 19, 2022 •
edited
Loading

The end-to-end deployment with Helix 1.0.4 is done at LinkedIn. Here are the highlights from the new Helix release:

  • All the Pinot clusters at LinkedIn are up and running with Helix 1.0.4, there is no performance downgrade after deploying Helix 1.0+.
  • Zookeeper read/write access latency (P99th) dropped significantly from seconds to milliseconds, which is 1000x improvement.
  • Both helix controller pipeline queued time and process time (P99th) dropped by 25-50%.
  • There is no downtime during the Pinot deployment.

Thus, we have the conclusion that the new Helix 1.0.4 release is stable, we should be good to bump up the Helix version to 1.0.4 in master branch.

mcvsubbu
mcvsubbu approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@mcvsubbu mcvsubbu left a comment

Choose a reason for hiding this comment

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

With the kind of testing that this has gone through, I am fine merging this.

If there are small concerns (to re-organize code, etc.) I suggest we can take it up in another PR after we merge this one.

Please give the other committers a few days in case they have any comments.

Jackie-Jiang
Jackie-Jiang approved these changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

Thanks for taking time verifying the new Helix version! The effort is much appreciated!

@jackjlli jackjlli merged commit 989df64 into master Jul 21, 2022
@jackjlli jackjlli deleted the helix-1.0 branch July 21, 2022 20:16
@jackjlli jackjlli added release-notes Referenced by PRs that need attention when compiling the next release notes enhancement labels Jul 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcvsubbu mcvsubbu mcvsubbu approved these changes

@Jackie-Jiang Jackie-Jiang Jackie-Jiang approved these changes

+1 more reviewer

@qqu0127 qqu0127 qqu0127 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement 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.

5 participants

@jackjlli @codecov-commenter @mcvsubbu @Jackie-Jiang @qqu0127