Skip to content

Conversation

@joegallo
Copy link
Contributor

Part of #76147, follow up to #79210 and #79275

#79210 added this new setting, and #79275 made it so the value defaults to true. This PR goes one further, and makes it so that the setting is deprecated and ignored -- we force a tier preference, and there's no setting to disable that. Mostly it's adjusting the tests to account for that, there were some tests where we were still explicitly setting the value to false in order to test 'what if' scenarios.

By having MetadataCreateIndexService ignore the
ENFORCE_DEFAULT_TIER_PREFERENCE setting (and, by extension, acting as
if it's always true).
It's just always true, these tests need to learn to live with that.
The tests that checked what happens when we don't enforce a tier
preference don't need to exist anymore. Likewise, since we always
enforce we can rename the tests that check what happens when we do.
We'll be removing it one day, and it's only allowed to be true anyway,
so there's no point in having it.
Rather, we can strip it from the settings and rely on the default
@joegallo joegallo added >breaking :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 labels Oct 25, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 25, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@joegallo
Copy link
Contributor Author

Note: this is the alternative PR I mentioned in #79723 (comment)

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.


public void testEnforceDefaultTierPreferenceSetting() {
Settings settings = Settings.builder()
.put(DataTier.ENFORCE_DEFAULT_TIER_PREFERENCE_SETTING.getKey(), true)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this could be randomBoolean() in this version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch, ed12c39

Having the setting set at all is what's deprecated, it doesn't matter
what the value actually is.
@joegallo
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-2

@joegallo joegallo mentioned this pull request Oct 26, 2021
7 tasks
@joegallo joegallo merged commit 9d27c74 into elastic:master Oct 26, 2021
@joegallo joegallo deleted the always-enforce-default-tier-preference-take-2 branch October 26, 2021 15:09
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this pull request Oct 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>breaking :Data Management/ILM+SLM Index and Snapshot lifecycle management Team:Data Management Meta label for data/management team v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants