Skip to content
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.

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

Remove support for delaying state recovery pending master #53845

Merged
merged 17 commits into from
Feb 3, 2021

Conversation

ShawnLi1014
Copy link
Contributor

@ShawnLi1014 ShawnLi1014 commented Mar 20, 2020

The following settings are deprecated:

  • gateway.expected_nodes
  • gateway.expected_master_nodes
  • gateway.recover_after_nodes
  • gateway.recover_after_master_nodes
    This pull request removed these settings.

Closes #51806

@DaveCTurner DaveCTurner added :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >breaking v8.0.0 labels Mar 20, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Cluster Coordination)

@DaveCTurner DaveCTurner self-requested a review March 20, 2020 09:15
@DaveCTurner
Copy link
Contributor

@elasticmachine test this please

Copy link
Contributor

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

Thanks @ShawnLi1014. Before I dive into the code in detail, there's a couple of points for the docs:

  • would you document this change in docs/reference/migration/migrate_8_0/settings.asciidoc since it's a breaking change? Note the settings that were removed and name the settings that users should use instead.
  • the docs themselves deserve a bit more of an overhaul, since there's now only one expected_ setting and one recover_after_. I think we can explain what they do a bit more clearly now that we don't have to account for people using some combination of all the different settings.

You can see a preview of the docs here: http://elasticsearch_53845.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/modules-gateway.html which hopefully helps.

I think an example would be useful too. See the end of docs/reference/setup/important-settings/discovery-settings.asciidoc for an example of the syntax for a YAML example with callouts to explain each part. Would you add an example?

I'll help with the details of the docs at a later date if needed.

@DaveCTurner
Copy link
Contributor

Also note the check style failure from CI:

11:54:21 [ant:checkstyle] [ERROR] /dev/shm/elastic+elasticsearch+pull-request-1/server/src/test/java/org/elasticsearch/gateway/GatewayServiceTests.java:24:8: Unused import - org.elasticsearch.common.settings.Setting. [UnusedImports]

@DaveCTurner
Copy link
Contributor

@ShawnLi1014 how are you getting on with addressing the review comments?

@ShawnLi1014
Copy link
Contributor Author

@ShawnLi1014 how are you getting on with addressing the review comments?

Sry for the delay. I have committed the changes, please take a look. If there's anything need to be improved please let me know

@rjernst rjernst added the Team:Distributed Meta label for distributed team label May 4, 2020
@DaveCTurner DaveCTurner self-assigned this May 14, 2020
@DaveCTurner
Copy link
Contributor

@elasticmachine ok to test

@DaveCTurner
Copy link
Contributor

Thanks for your patience here @ShawnLi1014. I've brought this branch up to date with recent changes in master, but there are still some places where we mention gateway.recover_after_nodes in tests and these tests now fail. I've enabled CI on this PR; would you work through any failures it encounters please?

@ywelsch
Copy link
Contributor

ywelsch commented Jul 23, 2020

@ShawnLi1014 are you still interested in working on this?

DaveCTurner and others added 2 commits February 1, 2021 15:55
This commit removes the following deprecated settings in v8:

- `gateway.expected_nodes`
- `gateway.expected_master_nodes`
- `gateway.recover_after_nodes`
- `gateway.recover_after_master_nodes`

Co-authored-by: ShawnLi1014 <shawnli1014@gmail.com>
@DaveCTurner
Copy link
Contributor

I opened #68316 for the failure.

@elasticmachine please run elasticsearch-ci/1

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner merged commit b0d185b into elastic:master Feb 3, 2021
masseyke added a commit that referenced this pull request Sep 2, 2021
…ettings (#77042)

The ability to delay cluster state recovery once a majority of master eligible nodes has joined has
been removed in 8.0. This commit checks for settings that are related, and issues a deprecation
info API message about it.
Relates #42404 #53845
jrodewig added a commit that referenced this pull request Sep 16, 2021
We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77951)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77952)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77953)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77958)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77954)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77956)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77955)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
elasticsearchmachine pushed a commit that referenced this pull request Sep 16, 2021
…) (#77957)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
jrodewig added a commit that referenced this pull request Sep 16, 2021
…) (#77959)

We deprecated the following settings in 7.7 with PR #53646:

* `gateway.expected_nodes`
* `gateway.expected_master_nodes`
* `gateway.recover_after_nodes`
* `gateway.recover_after_master_nodes`

However, we didn't add a related item to the 7.7 deprecation docs. This adds the
missing item.

Relates to #53845.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker >breaking :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed Meta label for distributed team v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove support for delaying state recovery pending master nodes
6 participants