Add setting to enforce a default TIER_PREFERENCE#79210
Merged
joegallo merged 14 commits intoelastic:masterfrom Oct 15, 2021
Merged
Add setting to enforce a default TIER_PREFERENCE#79210joegallo merged 14 commits intoelastic:masterfrom
joegallo merged 14 commits intoelastic:masterfrom
Conversation
It's not wired up to anything yet, though
This precise implementation is really janky, but once we force the setting to always be true, we should be able to simplify it quite a bit.
Collaborator
|
Pinging @elastic/es-data-management (Team:Data Management) |
henningandersen
approved these changes
Oct 15, 2021
server/src/main/java/org/elasticsearch/cluster/routing/allocation/DataTier.java
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/cluster/metadata/MetadataCreateIndexServiceTests.java
Show resolved
Hide resolved
...est/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDeciderIT.java
Outdated
Show resolved
Hide resolved
| enforceDefaultTierPreference(true); | ||
|
|
||
| Template t = new Template(Settings.builder() | ||
| .putNull(DataTier.TIER_PREFERENCE) |
Contributor
There was a problem hiding this comment.
Can we randomly pick between this and setting:
.put(IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_PREFIX + ".box", "warm")
No problem if this is problematic, I do think we cover this somewhat with the unit tests.
Contributor
Author
There was a problem hiding this comment.
I'm adding a reminder onto #76147 so that this isn't forgotten, but I want to unblock this PR and keep making progress for now.
My code here was mistaken -- there was a point where I didn't have the new logic conditional on whether it was a resize/shrink/clone/split, so I did need to change this test then, but indeed it's no longer necessary.
The test reuses the same 'request' object from stanza to stanza, so we have to carefully empty it out -- otherwise, if we're randomly using the template settings here the request settings from the previous stanza would leak through.
7 tasks
This was referenced Oct 15, 2021
joegallo
added a commit
that referenced
this pull request
Oct 15, 2021
This was referenced Oct 25, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #76147
Adds a new cluster setting (
cluster.routing.allocation.enforce_default_tier_preference), defaulting tofalse, that (if set totrue) will inject aindex.routing.allocation.include._tier_preferencewhen indices are created. This is in addition to the other places where we have conditional logic that sometimes injects a default value, and it serves as a final backstop -- if nothing else sets the value, then this will. Minor caveat: it doesn't apply in the case of resizes (shrink/clone/split).In a subsequent PR, this will be set to
trueon 8.0, but the tests work regardless of what the default is (those that require a particular value control it explicitly).Still todo (but don't let that stop you from reviewing):- [ ] Add docs for this new setting.edit: I'm going to add the docs once this has settled better -- I've got four in flight PRs on this right now, and I don't want to do the the bigger aspects of the docs changes pitter patter across the lot of them.
/cc @dakrone FYI (and you're welcome to take a look!), but I'm explicitly asking for review from @henningandersen.