Skip to content

[DOCS] Clarify cluster health status during rolling upgrade #40757

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
merged 3 commits into from
Mar 31, 2020
Merged

[DOCS] Clarify cluster health status during rolling upgrade #40757

merged 3 commits into from
Mar 31, 2020

Conversation

@GlenRSmith
Copy link
Contributor

@GlenRSmith GlenRSmith commented Apr 2, 2019

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS that we support?
  • If you are submitting this code for a class then read our policy for that.

@colings86 colings86 added :Core/Infra/Core Core issues without another label >docs General docs changes labels Apr 4, 2019
@elasticmachine
Copy link
Collaborator

elasticmachine commented Apr 4, 2019

Pinging @elastic/es-core-infra

jrodewig
jrodewig reviewed Dec 30, 2019
View reviewed changes
docs/reference/upgrade/rolling_upgrade.asciidoc Outdated
Comment on lines 123 to 124
Copy link
Contributor

@jrodewig jrodewig Dec 30, 2019

Choose a reason for hiding this comment

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

While technically accurate, the phrasing seems to imply that removing a node is a normal part of a rolling upgrade. To my knowledge, that's not the case.

Can you explain a bit more @GlenRSmith?

@jrodewig
Copy link
Contributor

jrodewig commented Dec 30, 2019

Apologies for the delay in reviewing this @GlenRSmith. I left a question for you.

After your response, I'll determine what the next steps are. Thanks!

@GlenRSmith
Copy link
Contributor Author

GlenRSmith commented Feb 19, 2020

@jrodewig Fixed. Sorry for the delay I think I don't have my notifications well-configured.

@jrodewig jrodewig added the :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) label Mar 31, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Mar 31, 2020

Pinging @elastic/es-distributed (:Distributed/Allocation)

@jrodewig jrodewig removed :Core/Infra/Core Core issues without another label feedback_needed labels Mar 31, 2020
DaveCTurner
DaveCTurner reviewed Mar 31, 2020
View reviewed changes
docs/reference/upgrade/rolling_upgrade.asciidoc Outdated
Copy link
Contributor

@DaveCTurner DaveCTurner Mar 31, 2020

Choose a reason for hiding this comment

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

I think we could drop any mention of the starting state here. It's true that it might start as red, and also that it might go straight from red to green without ever being yellow, but none of that is especially important here.

Suggested change
Wait for the `status` column to switch from `yellow` to `green`. Once the
Wait for the `status` column to switch to `green`. Once the

Copy link
Contributor

@DaveCTurner DaveCTurner Mar 31, 2020

Choose a reason for hiding this comment

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

TBC I think we should do this instead of adding the extra sentence suggested in this PR.

Copy link
Contributor

@jrodewig jrodewig Mar 31, 2020

Choose a reason for hiding this comment

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

I think that's a great idea. Implemented with 366586b.

jrodewig
jrodewig reviewed Mar 31, 2020
View reviewed changes
@jrodewig jrodewig changed the title Mention possibility of temporarily red cluster [DOCS] Clarify cluster health status during rolling upgrade Mar 31, 2020
@jrodewig jrodewig requested a review from DaveCTurner March 31, 2020 17:10
DaveCTurner
DaveCTurner approved these changes Mar 31, 2020
View reviewed changes
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.

LGTM

@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine test this please

@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine update branch

@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine test this please

@jrodewig jrodewig changed the base branch from 6.7 to 6.8 March 31, 2020 17:41
@jrodewig jrodewig changed the base branch from 6.8 to 6.7 March 31, 2020 17:41
@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine update branch

@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine test this please

@jrodewig jrodewig changed the base branch from 6.7 to 6.8 March 31, 2020 17:49
@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

@elasticmachine test this please

@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

Had to rebase this branch to get the CI to pass. Will merge and backport.

Thanks again for raising this @GlenRSmith.

@jrodewig jrodewig merged commit 1c83576 into elastic:6.8 Mar 31, 2020
jrodewig added a commit that referenced this pull request Mar 31, 2020
@GlenRSmith @jrodewig
[DOCS] Clarify cluster health status during rolling upgrade (#40757)
b7f62aa 
Remove mention of the `yellow` and `red` starting
health status from the rolling upgrade docs.

Instead, we should emphasize that users wait 
for the node to recover with a health status of
`green` rather than the starting status.

Co-authored-by: James Rodewig <[email protected]>
jrodewig added a commit that referenced this pull request Mar 31, 2020
@GlenRSmith @jrodewig
[DOCS] Clarify cluster health status during rolling upgrade (#40757)
309886a 
Remove mention of the `yellow` and `red` starting
health status from the rolling upgrade docs.

Instead, we should emphasize that users wait 
for the node to recover with a health status of
`green` rather than the starting status.

Co-authored-by: James Rodewig <[email protected]>
jrodewig added a commit that referenced this pull request Mar 31, 2020
@GlenRSmith @jrodewig
[DOCS] Clarify cluster health status during rolling upgrade (#40757)
0d4a001 
Remove mention of the `yellow` and `red` starting
health status from the rolling upgrade docs.

Instead, we should emphasize that users wait 
for the node to recover with a health status of
`green` rather than the starting status.

Co-authored-by: James Rodewig <[email protected]>
jrodewig added a commit that referenced this pull request Mar 31, 2020
@GlenRSmith @jrodewig
[DOCS] Clarify cluster health status during rolling upgrade (#40757)
1eb0047 
Remove mention of the `yellow` and `red` starting
health status from the rolling upgrade docs.

Instead, we should emphasize that users wait 
for the node to recover with a health status of
`green` rather than the starting status.

Co-authored-by: James Rodewig <[email protected]>
@jrodewig
Copy link
Contributor

jrodewig commented Mar 31, 2020

Backport commits

master 1eb0047
7.x 0d4a001
7.7 309886a
7.6 b7f62aa
6.8 1c83576

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

Reviewers

@DaveCTurner DaveCTurner DaveCTurner approved these changes

+1 more reviewer

@jrodewig jrodewig jrodewig left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

:Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >docs General docs changes v7.6.3 v7.7.1 v7.8.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@GlenRSmith @elasticmachine @jrodewig @DaveCTurner @colings86 @jakelandis