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

Make TransportAddVotingConfigExclusionsAction retryable #98386

Conversation

DaveCTurner
Copy link
Contributor

The docs for this API say the following:

If the API fails, you can safely retry it. Only a successful response
guarantees that the node has been removed from the voting
configuration and will not be reinstated.

Unfortunately this isn't true today: if the request adds no exclusions
then we do not wait before responding. This commit makes the API wait
until all exclusions are really applied.

The docs for this API say the following:

> If the API fails, you can safely retry it. Only a successful response
> guarantees that the node has been removed from the voting
> configuration and will not be reinstated.

Unfortunately this isn't true today: if the request adds no exclusions
then we do not wait before responding. This commit makes the API wait
until all exclusions are really applied.
@DaveCTurner DaveCTurner added >bug :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v8.10.0 v7.17.13 labels Aug 11, 2023
@DaveCTurner
Copy link
Contributor Author

Probably going to be a bit of a pain to backport this, but seems worth the effort.

@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Aug 11, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

This test was a duplicate of
`testExcludeByNodeNameSucceedsEvenIfAllExclusionsAlreadyAdded` before
this change, but is now meaningfully different.
Copy link
Contributor

@henningandersen henningandersen 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 1fef466 into elastic:main Aug 14, 2023
@DaveCTurner DaveCTurner deleted the 2023/08/11/TransportAddVotingConfigExclusionsAction-idempotent branch August 14, 2023 06:13
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 17, 2023
The docs for this API say the following:

> If the API fails, you can safely retry it. Only a successful response
> guarantees that the node has been removed from the voting
> configuration and will not be reinstated.

Unfortunately this isn't true today: if the request adds no exclusions
then we do not wait before responding. This commit makes the API wait
until all exclusions are really applied.

Backport of elastic#98386, plus the test changes from elastic#98146 and elastic#98356.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Aug 17, 2023
This test is today asserting that the _initial_ state has all the
expected exclusions, but it should be checking the _final_ state.

Relates elastic#98386
elasticsearchmachine pushed a commit that referenced this pull request Aug 17, 2023
The docs for this API say the following:

> If the API fails, you can safely retry it. Only a successful response
> guarantees that the node has been removed from the voting
> configuration and will not be reinstated.

Unfortunately this isn't true today: if the request adds no exclusions
then we do not wait before responding. This commit makes the API wait
until all exclusions are really applied.

Backport of #98386, plus the test changes from #98146 and #98356.
elasticsearchmachine pushed a commit that referenced this pull request Aug 18, 2023
This test is today asserting that the _initial_ state has all the
expected exclusions, but it should be checking the _final_ state.

Relates #98386
csoulios pushed a commit to csoulios/elasticsearch that referenced this pull request Aug 18, 2023
The docs for this API say the following:

> If the API fails, you can safely retry it. Only a successful response
> guarantees that the node has been removed from the voting
> configuration and will not be reinstated.

Unfortunately this isn't true today: if the request adds no exclusions
then we do not wait before responding. This commit makes the API wait
until all exclusions are really applied.
csoulios pushed a commit to csoulios/elasticsearch that referenced this pull request Aug 18, 2023
…c#98570)

This test is today asserting that the _initial_ state has all the
expected exclusions, but it should be checking the _final_ state.

Relates elastic#98386
@DaveCTurner DaveCTurner restored the 2023/08/11/TransportAddVotingConfigExclusionsAction-idempotent branch June 17, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport pending >bug :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v7.17.13 v8.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants