apache / pinot

Apache Pinot - A realtime distributed OLAP datastore
https://pinot.apache.org/
Apache License 2.0
5.2k stars 1.21k forks source link

Add PodDisruptionBudgets to the Pinot Helm chart #13153

Closed andimiller closed 1 week ago

andimiller commented 2 weeks ago

I've set these by default at allowing 50% of the servers/brokers/controllers to be unavailable, and exposed these settings in the configuration so the user can change them.

These prevent outages when the underlying kuberenetes cluster undergoes an upgrade.

You might configure these like so in your helm yaml settings:

broker:
  replicaCount: 2
  pdb:
    enabled: true
    minAvailable: 50% # we could also set this to 1
    maxUnavailable: "" # can turn these off with empty string
controller:
  replicaCount: 2
  pdb:
    enabled: true
    minAvailable: 1 # we could also set this to 50%
    maxUnavailable: "" # can turn these off with empty string
server:
  replicaCount: 12
  pdb:
    enabled: true
    maxUnavailable: 1
    # minAvailable: 11 # these would be equivalent
codecov-commenter commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 62.25%. Comparing base (59551e4) to head (b2dfe14). Report is 470 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #13153 +/- ## ============================================ + Coverage 61.75% 62.25% +0.50% + Complexity 207 198 -9 ============================================ Files 2436 2529 +93 Lines 133233 138215 +4982 Branches 20636 21386 +750 ============================================ + Hits 82274 86048 +3774 - Misses 44911 45762 +851 - Partials 6048 6405 +357 ``` | [Flag](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | |---|---|---| | [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (-0.01%)` | :arrow_down: | | [integration](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (-0.01%)` | :arrow_down: | | [integration1](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `<0.01% <ø> (-0.01%)` | :arrow_down: | | [integration2](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | | [java-11](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.19% <ø> (+0.48%)` | :arrow_up: | | [java-21](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.14% <ø> (+0.51%)` | :arrow_up: | | [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.22% <ø> (+0.47%)` | :arrow_up: | | [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.12% <ø> (+34.39%)` | :arrow_up: | | [temurin](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.25% <ø> (+0.50%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.25% <ø> (+0.50%)` | :arrow_up: | | [unittests1](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.75% <ø> (-0.14%)` | :arrow_down: | | [unittests2](https://app.codecov.io/gh/apache/pinot/pull/13153/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `27.92% <ø> (+0.18%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

abhioncbr commented 2 weeks ago

Thanks for the contribution.

In general, one comment from K8s before v1.21 support, and to give other users better control over the PDB, can we have a boolean property say enabled to make the PDB optional?

andimiller commented 2 weeks ago

Thanks for the contribution.

In general, one comment from K8s before v1.21 support, and to give other users better control over the PDB, can we have a boolean property say enabled to make the PDB optional?

Sure, done, and since the default replica counts of everything in values.yaml is 1 I've defaulted it to off

Jackie-Jiang commented 2 weeks ago

cc @xiangfu0 @zhtaoxiang

gortiz commented 1 week ago

FYI, the zookeeper chart from bitnami (the one we depend on) also has this ability.

andimiller commented 1 week ago

FYI, the zookeeper chart from bitnami (the one we depend on) also has this ability.

I'll adjust this to work with the same config settings as the zookeeper chart then, if people are already familiar with that one

andimiller commented 1 week ago

FYI, the zookeeper chart from bitnami (the one we depend on) also has this ability.

I've adjusted this to be closer how to the zookeeper chart works, with pdb as the config name, and exposing both minAvailable and maxUnavailable