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

ES-Operator doesn't separate system indices and ordinary users indices #177

Merged
merged 3 commits into from
Dec 9, 2021

Conversation

otrosien
Copy link
Member

@otrosien otrosien commented May 14, 2021

One-line summary

Exclude ES-Operator from managing system indices. This can be controlled per EDS. To avoid this being a breaking change, the default setting is opt-in.

Issue : #161

Description

Added an optional setting to the EDS (spec.excludeSystemIndices). If set to true, the ES-Operator will filter out indices starting with "." (indication of an internal Elasticsearch index) when calculating shard-per-node ratio and scaling index replica counts.

It will still check all shards even ones for system indices when draining to make sure we don't terminte a node with data on it.

Types of Changes

  • New feature (non-breaking change which adds functionality)
  • Configuration change

Tasks

Open question: we introducing a double-negation property, which I'm not a big fan of. What do others think of that?

Review

List of tasks the reviewer must do to review the PR

  • Tests
  • Documentation (TODO)

Deployment Notes

Regarding manifest versioning, this should be fine, we're extending the resource with an optional property.

@otrosien otrosien requested review from mikkeloscar and removed request for mikkeloscar May 14, 2021 08:30
@otrosien
Copy link
Member Author

@s-vkropotko FYI

@s-vkropotko
Copy link

s-vkropotko commented May 14, 2021

Hey!
I can suggest to rename it to avoid to be a double-negative property:
excludeSystemIndices=false -> manageSystemIndices=true/operateSystemIndices=true?
WDYT?

@otrosien
Copy link
Member Author

otrosien commented May 14, 2021

Hey!
I can suggest to rename it to avoid to be a double-negative property:
excludeSystemIndices=false -> manageSystemIndices=true/operateSystemIndices=true?
WDYT?

I was thinking about it but then do we introduce a new default to not manageSystemIndices (as default non-specified value becomes false)?

Also, these system indices are a PITA also for a different reason: They break our "shards-to-node ratio" calculation even if we ignore them - you could easily get into a situation of non-ideal shard distribution (e.g. 2 production shards on one node, 2 system shards on the other node) if system indices get colocated with user indices - so in our cluster we always try to move them away onto a dedicated (or otherwise non-critical) EDS.

operator/es_client.go Outdated Show resolved Hide resolved
Signed-off-by: Oliver <[email protected]>
Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
@mikkeloscar
Copy link
Collaborator

👍

1 similar comment
@otrosien
Copy link
Member Author

otrosien commented Dec 9, 2021

👍

@otrosien otrosien merged commit 85f0872 into master Dec 9, 2021
@otrosien otrosien deleted the exclude-system-indices branch December 9, 2021 11:14
otrosien added a commit that referenced this pull request Feb 7, 2022
Introduced in #177

Signed-off-by: Oliver Trosien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants