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

Do not call the voting config exclusions API at every single reconciliation #2642

Merged
merged 5 commits into from
Mar 2, 2020

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Mar 2, 2020

This PR sets up an annotation containing the last applied value for
voting config exclusions. The annotation can either be:

  • nil: it has not been set yet
  • empty: the last setting applied was to reset voting config exclusions
  • not empty: it contains the last applied voting config exclusions

Before making the ES API call to either add nodes to voting config exclusions
or reset its value, we check the value of the annotation.

Users can remove the annotation at anytime, it will be set again after
the next API call.
Note we ignore cases where users would manipulate voting config
exclusions behind our back here.

Fixes #2605.

sebgl added 3 commits March 2, 2020 11:54
This commit sets up an annotation containing the last applied value for
voting config exclusions. The annotation can either be:
- nil: it has not been set yet
- empty: the last setting applied was to reset voting config exclusions
- not empty: it contains the last applied voting config exclusions

Before making the ES API call, which check the value of that annotation.
Users can remove the annotation at anytime, it will be set again after
the next API call.
Note we ignore cases where users would manipulate voting config
exclusions behind our back here.
@sebgl sebgl added >enhancement Enhancement of existing functionality v1.1.0 labels Mar 2, 2020
copy(sliceCopy, excludedNodes)
sort.Strings(sliceCopy)
return strings.Join(sliceCopy, ",")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to sort a copy of the slice instead of mutating the slice itself. That's an extra allocation but I would not expect such a side effect to occur in that function.

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

@sebgl sebgl merged commit 4ee5ae5 into elastic:master Mar 2, 2020
@anyasabo anyasabo changed the title Don't call the voting config exclusions API at every single reconciliation Do not call the voting config exclusions API at every single reconciliation Mar 31, 2020
@pebrc pebrc added >non-issue and removed >enhancement Enhancement of existing functionality labels Apr 21, 2020
@pebrc
Copy link
Collaborator

pebrc commented Apr 21, 2020

reverted in #2880 (thus >non-issue)

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

Successfully merging this pull request may close these issues.

Don't reset voting config exclusions at every single reconciliation
2 participants