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

Fix Serialization version check in Shutdown Status #81342

Conversation

gwbrown
Copy link
Contributor

@gwbrown gwbrown commented Dec 3, 2021

The version used to determine whether we should serialize the
allocation decisions as part of the Shutdown Status response
was not updated after #78727 was merged. This causes very
confusing errors in mixed 8.x/7.16.x clusters.

Note: The version constant was updated in the backport to 7.16,
but not in 8.x. This is good, as at time of writing, 7.16 has
shipped, but 8.0 has not.

The version used to determine whether we should serialize the
allocation decisions as part of the Shutdown Status response
was not updated after elastic#78727 was merged. This causes very
confusing errors in mixed 8.x/7.16.x clusters.

Note: The version constant was updated in the backport to 7.16,
but not in 8.x. This is good, as at time of writing, 7.16 has
shipped, but 8.0 has not.
@gwbrown gwbrown added >non-issue v8.0.0 :Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown v8.1.0 labels Dec 3, 2021
@gwbrown gwbrown requested review from dakrone and pgomulka December 3, 2021 22:42
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 3, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM

@gwbrown
Copy link
Contributor Author

gwbrown commented Dec 3, 2021

@pgomulka I ran the upgrade test several times locally with this patch and didn't see any failures - this is a great example of why we need those tests! Let me know if this doesn't solve the issues you're seeing.

@gwbrown gwbrown added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Dec 3, 2021
@elasticsearchmachine elasticsearchmachine merged commit 774bc11 into elastic:master Dec 3, 2021
gwbrown added a commit to gwbrown/elasticsearch that referenced this pull request Dec 3, 2021
The version used to determine whether we should serialize the allocation
decisions as part of the Shutdown Status response was not updated after
elastic#78727 was merged. This causes very confusing errors in mixed 8.x/7.16.x
clusters. Note: The version constant was updated in the backport to
7.16, but not in 8.x. This is good, as at time of writing, 7.16 has
shipped, but 8.0 has not.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.0

elasticsearchmachine pushed a commit that referenced this pull request Dec 4, 2021
The version used to determine whether we should serialize the allocation
decisions as part of the Shutdown Status response was not updated after
#78727 was merged. This causes very confusing errors in mixed 8.x/7.16.x
clusters. Note: The version constant was updated in the backport to
7.16, but not in 8.x. This is good, as at time of writing, 7.16 has
shipped, but 8.0 has not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Node Lifecycle Node startup, bootstrapping, and shutdown >non-issue Team:Core/Infra Meta label for core/infra team v8.0.0-rc1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants