Skip to content

Always enforce default tier preference (ENFORCE_DEFAULT_TIER_PREFERENCE is ignored)#79751

Merged
joegallo merged 14 commits intoelastic:masterfrom
joegallo:always-enforce-default-tier-preference-take-2
Oct 26, 2021
Merged

Always enforce default tier preference (ENFORCE_DEFAULT_TIER_PREFERENCE is ignored)#79751
joegallo merged 14 commits intoelastic:masterfrom
joegallo:always-enforce-default-tier-preference-take-2

Conversation

@joegallo
Copy link
Copy Markdown
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 DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. v8.0.0 labels Oct 25, 2021
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Oct 25, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@joegallo
Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants