Skip to content

Conversation

@jtibshirani
Copy link
Contributor

@jtibshirani jtibshirani commented Apr 7, 2021

When _tier_preference isn't set, getting it returns an empty string "". This
PR updates the _tier metadata field to detect this case correctly.

Relates to #68135.

When `_tier_preference` isn't set, getting it returns an empty string "". This
PR updates the _tier metadata field to detect this case correctly.
@jtibshirani jtibshirani added >non-issue :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.13.0 labels Apr 7, 2021
@elasticmachine elasticmachine added the Team:Search Meta label for search team label Apr 7, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (Team:Search)

}
String tierPreference = DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(context.getIndexSettings().getSettings());
if (tierPreference == null) {
if (Strings.hasText(tierPreference) == false) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check is a little unusual, but it matches how we test for the setting in other places like DataTierAllocationDecider.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM, I left a minor suggestion around reusing the existing test helper but leave it to you if you want to change that or not.

);
}

private SearchExecutionContext createContextWithoutSetting() {
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe we could re-use the other createContext() method by adding an argument (e.g. a bool flag) to either return a context with- or without the setting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I'll stick with this approach as I find it clearest right now. We can always refactor if these methods grow larger and there's more duplication!

@jtibshirani jtibshirani merged commit 5bcd1f3 into elastic:master Apr 8, 2021
@jtibshirani jtibshirani deleted the empty-tier-field branch April 8, 2021 17:04
jtibshirani added a commit that referenced this pull request Apr 8, 2021
When `_tier_preference` isn't set, getting it returns an empty string "". This
PR updates the _tier metadata field to detect this case correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search/Search Search-related issues that do not fall into other categories Team:Search Meta label for search team v7.13.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants