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 min node version before state recovery #86482

Conversation

DaveCTurner
Copy link
Contributor

We use the minimum node version in the cluster state to make decisions
about backwards compatibility (e.g. to choose newer actions in the REST
layer only if all nodes will support it). Once the cluster is fully
formed we reject attempts by older nodes to join the cluster so that the
minimum node version only ever increases, which makes
backwards-compatibility decisions safe.

However, it's possible that the REST layer will make decisions about
backwards compatibility before the cluster is fully formed. In this
state, older nodes may still join the cluster and may therefore see
actions that they do not understand.

With this commit we report no nodes to the REST layer until the cluster
is fully-formed, and change the minimum node version in an empty cluster
to be the minimum compatible version. This means the REST layer will
operate in a maximally-compatible mode until the cluster is formed.

Relates #86405

We use the minimum node version in the cluster state to make decisions
about backwards compatibility (e.g. to choose newer actions in the REST
layer only if all nodes will support it). Once the cluster is fully
formed we reject attempts by older nodes to join the cluster so that the
minimum node version only ever increases, which makes
backwards-compatibility decisions safe.

However, it's possible that the REST layer will make decisions about
backwards compatibility before the cluster is fully formed. In this
state, older nodes may still join the cluster and may therefore see
actions that they do not understand.

With this commit we report no nodes to the REST layer until the cluster
is fully-formed, and change the minimum node version in an empty cluster
to be the minimum compatible version. This means the REST layer will
operate in a maximally-compatible mode until the cluster is formed.

Relates elastic#86405
@DaveCTurner DaveCTurner added >bug :Core/Infra/REST API REST infrastructure and utilities v8.3.0 labels May 5, 2022
@DaveCTurner DaveCTurner requested a review from grcevski May 5, 2022 17:22
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label May 5, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @DaveCTurner, I've created a changelog YAML for you.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

I like the idea here! However, how would the nodes passed to the initRestHandlers ever get updated once the cluster state has stabilized on the new ES version? This happens only once when starting up the node.

@rjernst
Copy link
Member

rjernst commented May 5, 2022

Oh I see, we pass a supplier around everywhere, so once the cluster state gets updated locally, the next call to any rest handler using it will see the new nodes.

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM. Might be nice to have a test checking a rest handler sees the newly reflected nodes, but it's fine as is too.

@DaveCTurner
Copy link
Contributor Author

++ yes it's a Supplier<DiscoveryNodes>, we get it afresh each time. Bit tricky to test but I think the one added in 8c4e1ed should do the job.

Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@DaveCTurner DaveCTurner merged commit 80ce6e6 into elastic:master May 6, 2022
@DaveCTurner
Copy link
Contributor Author

Thanks both :)

@DaveCTurner DaveCTurner deleted the 2022-05-05-min-node-version-before-recovery branch May 6, 2022 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Core/Infra/REST API REST infrastructure and utilities Team:Core/Infra Meta label for core/infra team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants