Skip to content

[bitnami/elasticsearch] Use 3 master nodes by default#6587

Merged
rafariossaa merged 4 commits into
bitnami:masterfrom
yariksheptykin:patch-1
Jun 15, 2021
Merged

[bitnami/elasticsearch] Use 3 master nodes by default#6587
rafariossaa merged 4 commits into
bitnami:masterfrom
yariksheptykin:patch-1

Conversation

@yariksheptykin

Copy link
Copy Markdown
Contributor

Description of the change

Currently, when using default settings, we install 2 master nodes. This PR suggests using 3 nodes by default to address "split brain" situation by default. "Split brain" occurs when a cluster elects more than one master due to network partitioning. See https://www.elastic.co/guide/en/elasticsearch/reference/7.x/modules-discovery-quorums.html#modules-discovery-quorums

Elasticsearch documentation suggests using an even number of master nodes to address this issue https://www.elastic.co/guide/en/elasticsearch/reference/7.x/modules-discovery-voting.html#_even_numbers_of_master_eligible_nodes. The next reasonable odd number for the default settings would be 3.

Benefits

Address "split brain" issue by default.

Possible drawbacks

  • Cluster availability is delayed until the 3 nodes form a quorum.
  • More resources are required upfront for default setup.

@github-actions

github-actions Bot commented Jun 7, 2021

Copy link
Copy Markdown

Great PR! Please pay attention to the following items before merging:

Files matching bitnami/*/values.yaml:

  • Is the PR adding a new container? Please reviewer, add it to the models (internal process)
  • Is the PR adding a new parameter? Please, ensure it’s documented in the README.md

This is an automatically generated QA checklist based on modified files

@yariksheptykin yariksheptykin marked this pull request as ready for review June 7, 2021 10:06
@yariksheptykin yariksheptykin changed the title [bitnami/elasticsearch] Default master replicas to 3 [bitnami/elasticsearch] Use 3 master replicas by default Jun 7, 2021
@yariksheptykin yariksheptykin changed the title [bitnami/elasticsearch] Use 3 master replicas by default [bitnami/elasticsearch] Use 3 master nodes by default Jun 7, 2021
@rafariossaa

Copy link
Copy Markdown
Contributor

Hi,
I am testing this PR in our internal CI/CD.

@yariksheptykin

Copy link
Copy Markdown
Contributor Author

Hi, @rafariossaa . Did your test uncover any issues with the PR? Can I be of any help to test this PR?

I could contribute a helm test so we could atomate some parts of testing. Something similart to this https://github.com/elastic/helm-charts/blob/master/elasticsearch/templates/test/test-elasticsearch-health.yaml maybe?

@rafariossaa

Copy link
Copy Markdown
Contributor

Hi, @yariksheptykin
Sorry for the delay, let me ping you back today on this.

@rafariossaa

Copy link
Copy Markdown
Contributor

Hi,
A new version of the chart was released, do you mind bumping the chart version in your PR ?

@yariksheptykin

Copy link
Copy Markdown
Contributor Author

No problem @rafariossaa . I rebased the patch onto the latest upstream master.

The version was set to 15.3.1. I chose 15.4.0 as it adds a new feature to my opinion.

However one might argue that changing default values in values.yaml changes the API of the chart for its consumers. In this case we should set the version to 16.x.

@rafariossaa

Copy link
Copy Markdown
Contributor

Hi,
15.4.0 is ok for this kind of changes.
Thanks !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants