-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Index Balanced Allocation Decider Add index routing filter handling #138741
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
base: main
Are you sure you want to change the base?
Index Balanced Allocation Decider Add index routing filter handling #138741
Conversation
|
Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination) |
| private List<RoutingNode> searchIier; | ||
|
|
||
| private void setup(Settings settings) { | ||
| private void setup(Settings clusterSettings, Supplier<Settings> indexSettings) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason why indexSettings is a Supplier is due to the need to defer retrieving DiscoveryNode reference until these variables are initialized.
String id = randomFrom(indexNodeOne.getId(), indexNodeTwo.getId(), searchNodeOne.getId(), searchNodeTwo.getId());
String hostName = randomFrom(
indexNodeOne.getHostName(),
indexNodeTwo.getHostName(),
searchNodeOne.getHostName(),
searchNodeTwo.getHostName()
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's OK though the filters and the decider behaviour do not depend on the filtered node IDs to match existing nodes.
ywangd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| private List<RoutingNode> searchIier; | ||
|
|
||
| private void setup(Settings settings) { | ||
| private void setup(Settings clusterSettings, Supplier<Settings> indexSettings) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's OK though the filters and the decider behaviour do not depend on the filtered node IDs to match existing nodes.
| final IndexMetadata indexMetadata = allocation.getClusterState().metadata().getProject(projectId).index(index); | ||
|
|
||
| if (hasIndexRoutingFilters(indexMetadata)) { | ||
| return allocation.decision(Decision.YES, NAME, "Decider is disabled."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we be a bit more elaborative in the explanation string
| return allocation.decision(Decision.YES, NAME, "Decider is disabled."); | |
| return allocation.decision(Decision.YES, NAME, "Decider is disabled for index level allocation filters."); |
?
Relates ES-12080
In previous #135875, Reviewer correctly pointed out the handling of Index Routing Filters is absent in the current decider.
This PR addresses this deficiency by recuse the decider in event of the presence of index routing filters.