Skip to content

Alloc. awareness should ignore nodes without attr#69374

Merged
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2021-02-22-allocation-awareness-ignore-nodes-without-attribute
Feb 23, 2021
Merged

Alloc. awareness should ignore nodes without attr#69374
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2021-02-22-allocation-awareness-ignore-nodes-without-attribute

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

Today we count null (i.e. missing) as a valid attribute value in
allocation awareness, even though allocation awareness forbids the
allocation of shards to such a node. Prior to #69334 this didn't matter,
a data node without allocation attributes was pointless.

However, #69334 means we now can allocate shards to such a node: for
instance, there is no need for nodes holding only enrich indices to have
allocation attributes. Therefore we should stop counting null as one
of the attribute values.

Today we count `null` (i.e. missing) as a valid attribute value in
allocation awareness, even though allocation awareness forbids the
allocation of shards to such a node. Prior to elastic#69334 this didn't matter,
a data node without allocation attributes was pointless.

However, elastic#69334 means we now can allocate shards to such a node: for
instance, there is no need for nodes holding only enrich indices to have
allocation attributes. Therefore we should stop counting `null` as one
of the attribute values.
@DaveCTurner DaveCTurner added >non-issue :Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) v8.0.0 v7.13.0 labels Feb 22, 2021
@DaveCTurner DaveCTurner requested a review from ywelsch February 22, 2021 18:18
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Feb 22, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

Copy link
Copy Markdown
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

I find it a bit difficult to understand the implications of this PR. will it break clusters that have cluster.routing.allocation.awareness.attributes defined but where the nodes are missing the attributes?

.add(newNode("X-0", emptyMap()))
).build(), strategy);

assertThat(clusterState.getRoutingNodes().shardsWithState(UNASSIGNED), empty());
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.

where do we assert that X-0 is ignored, i.e. does not hold a shard copy?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Added in d7216bb

@DaveCTurner
Copy link
Copy Markdown
Member Author

Today if you have a cluster containing a data node without all the awareness attributes defined then those nodes get no shards at all.

// the node the shard exists on must be associated with an awareness attribute
if (node.node().getAttributes().containsKey(awarenessAttribute) == false) {
return debug ? debugNoMissingAttribute(awarenessAttribute, awarenessAttributes) : Decision.NO;
}

It's hard to imagine that there are any clusters in this state today, they would have these empty data nodes and would almost always have unassigned shards too.

The effect of this change is to reduce the effective number of attribute values in such a cluster, which relaxes the upper bound on how many shards can be allocated to each value, which shouldn't break anything.

Copy link
Copy Markdown
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

ok, I see, thank you for the explanation. LGTM

@DaveCTurner DaveCTurner merged commit 2e5625d into elastic:master Feb 23, 2021
@DaveCTurner DaveCTurner deleted the 2021-02-22-allocation-awareness-ignore-nodes-without-attribute branch February 23, 2021 17:29
DaveCTurner added a commit that referenced this pull request Feb 23, 2021
Today we count `null` (i.e. missing) as a valid attribute value in
allocation awareness, even though allocation awareness forbids the
allocation of shards to such a node. Prior to #69334 this didn't matter,
a data node without allocation attributes was pointless.

However, #69334 means we now can allocate shards to such a node: for
instance, there is no need for nodes holding only enrich indices to have
allocation attributes. Therefore we should stop counting `null` as one
of the attribute values.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Feb 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >non-issue Team:Distributed Meta label for distributed team. v7.13.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants