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

Set expectations when upscaling a StatefulSet #1813

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Sep 27, 2019

We had a race condition where some master nodes could be created at
reconciliation #1, then reconciliation #2 would create another set of
master nodes, ignoring the ones from #1 that were created. As a result,
master nodes from #2 would start with the wrong zen1/zen2 settings.

To prevent that from happening, let's reuse our existing expectations
mechanism. When we apply an update, make sure we wait for that update to
appear in the StatefulSets cache.

This goes beyond simply updating replicas and generally slows down the
reconciliation if fields other than replicas were updated.
For the sake of simplicity, I think it's fine.

Fixes #1678.

Kudos to @pebrc and @barkbay for finding this bug in #1654 (comment).

We had a race condition where some master nodes could be created at
reconciliation elastic#1, then reconciliation elastic#2 would create another set of
master nodes, ignoring the ones from elastic#1 that were created. As a result,
master nodes from elastic#2 would start with the wrong zen1/zen2 settings.

To prevent that from happening, let's reuse our existing expectations
mechanism. When we apply an update, make sure we wait for that update to
appear in the StatefulSets cache.

This goes beyond simply updating replicas and generally slows down the
reconciliation if fields other than replicas were updated.
For the sake of simplicity, I think it's fine.
@sebgl sebgl added >bug Something isn't working v1.0.0-beta1 labels Sep 27, 2019
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

LGMT 👍

@sebgl sebgl merged commit 9ccd4b9 into elastic:master Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Expectations on updates sets to avoid cache inconsistencies
3 participants