-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Fail allocation of new primaries in empty cluster #43284
Changes from 2 commits
33029bb
8dd7523
bd1edbf
aace656
a593514
aacbc98
82c1e28
88ecfb8
40d410d
3c57592
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -29,6 +29,7 @@ | |||||
import org.elasticsearch.cluster.routing.RoutingNodes; | ||||||
import org.elasticsearch.cluster.routing.ShardRouting; | ||||||
import org.elasticsearch.cluster.routing.ShardRoutingState; | ||||||
import org.elasticsearch.cluster.routing.UnassignedInfo; | ||||||
import org.elasticsearch.cluster.routing.UnassignedInfo.AllocationStatus; | ||||||
import org.elasticsearch.cluster.routing.allocation.AllocateUnassignedDecision; | ||||||
import org.elasticsearch.cluster.routing.allocation.AllocationDecision; | ||||||
|
@@ -115,7 +116,8 @@ private void setThreshold(float threshold) { | |||||
@Override | ||||||
public void allocate(RoutingAllocation allocation) { | ||||||
if (allocation.routingNodes().size() == 0) { | ||||||
/* with no nodes this is pointless */ | ||||||
// If no data node then set AllocationStatus to DECIDERS_NO | ||||||
setAllocationStatus(allocation); | ||||||
return; | ||||||
} | ||||||
final Balancer balancer = new Balancer(logger, allocation, weightFunction, threshold); | ||||||
|
@@ -141,6 +143,35 @@ public ShardAllocationDecision decideShardAllocation(final ShardRouting shard, f | |||||
return new ShardAllocationDecision(allocateUnassignedDecision, moveDecision); | ||||||
} | ||||||
|
||||||
/** | ||||||
* This method is called when there are no data nodes in the cluster. | ||||||
* | ||||||
* Newly created unassigned primary shards, with no prior allocation attempts | ||||||
* are classified as yellow instead of red by cluster health. | ||||||
* This function explicitly sets their allocation status to DECIDERS_NO, to | ||||||
* indicate red indices with unassigned shards. | ||||||
*/ | ||||||
private void setAllocationStatus(RoutingAllocation allocation){ | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
RoutingNodes routingNodes = allocation.routingNodes(); | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
if (unassignedShards.isEmpty()) { | ||||||
return; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this check is necessary? |
||||||
} | ||||||
RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
while (unassignedIterator.hasNext()) { | ||||||
ShardRouting shard = unassignedIterator.next(); | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
UnassignedInfo shardInfo = shard.unassignedInfo(); | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (shard.primary() && shardInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { | ||||||
UnassignedInfo newInfo = new UnassignedInfo(shardInfo.getReason(), shardInfo.getMessage(), shardInfo.getFailure(), | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
shardInfo.getNumFailedAllocations(), shardInfo.getUnassignedTimeInNanos(), | ||||||
shardInfo.getUnassignedTimeInMillis(), shardInfo.isDelayed(), | ||||||
AllocationStatus.DECIDERS_NO); | ||||||
unassignedIterator.updateUnassigned(newInfo, shard.recoverySource(), allocation.changes()); | ||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
|
||||||
/** | ||||||
* Returns the currently configured delta threshold | ||||||
*/ | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,153 @@ | ||||||
/* | ||||||
* Licensed to Elasticsearch under one or more contributor | ||||||
* license agreements. See the NOTICE file distributed with | ||||||
* this work for additional information regarding copyright | ||||||
* ownership. Elasticsearch licenses this file to you under | ||||||
* the Apache License, Version 2.0 (the "License"); you may | ||||||
* not use this file except in compliance with the License. | ||||||
* You may obtain a copy of the License at | ||||||
* | ||||||
* http://www.apache.org/licenses/LICENSE-2.0 | ||||||
* | ||||||
* Unless required by applicable law or agreed to in writing, | ||||||
* software distributed under the License is distributed on an | ||||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||
* KIND, either express or implied. See the License for the | ||||||
* specific language governing permissions and limitations | ||||||
* under the License. | ||||||
*/ | ||||||
package org.elasticsearch.cluster.health; | ||||||
|
||||||
import org.elasticsearch.Version; | ||||||
import org.elasticsearch.cluster.ClusterName; | ||||||
import org.elasticsearch.cluster.ClusterState; | ||||||
import org.elasticsearch.cluster.ESAllocationTestCase; | ||||||
import org.elasticsearch.cluster.metadata.IndexMetaData; | ||||||
import org.elasticsearch.cluster.metadata.MetaData; | ||||||
import org.elasticsearch.cluster.node.DiscoveryNode; | ||||||
import org.elasticsearch.cluster.node.DiscoveryNodes; | ||||||
import org.elasticsearch.cluster.routing.RoutingNodes; | ||||||
import org.elasticsearch.cluster.routing.RoutingTable; | ||||||
import org.elasticsearch.cluster.routing.ShardRouting; | ||||||
import org.elasticsearch.cluster.routing.UnassignedInfo; | ||||||
import org.elasticsearch.common.util.set.Sets; | ||||||
|
||||||
import java.util.Collections; | ||||||
|
||||||
import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; | ||||||
import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; | ||||||
|
||||||
/** | ||||||
* The test case checks for the scenario when there is no data node in the cluster and only | ||||||
* master is active. At this moment when index creation is tried, the cluster health status should | ||||||
* change to RED | ||||||
*/ | ||||||
public class NoDataNodesHealthTests extends ESAllocationTestCase { | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
/** | ||||||
* This method specifically creates a cluster with no data nodes | ||||||
* and a single master node | ||||||
*/ | ||||||
private ClusterState setUpClusterWithNoDataNodes() { | ||||||
|
||||||
DiscoveryNodes node = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))).build(); | ||||||
MetaData metaData = MetaData.builder() | ||||||
.put(IndexMetaData.builder("TestIndex") | ||||||
.settings(settings(Version.CURRENT)) | ||||||
.numberOfShards(randomIntBetween(1, 3)) | ||||||
.numberOfReplicas(randomIntBetween(0, 2))) | ||||||
.build(); | ||||||
RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); | ||||||
ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) | ||||||
.nodes(node) | ||||||
.metaData(metaData) | ||||||
.routingTable(routingTable) | ||||||
.build(); | ||||||
MockAllocationService service = createAllocationService(); | ||||||
state = service.reroute(state, "reroute"); | ||||||
return state; | ||||||
} | ||||||
|
||||||
public void testClusterHealthWithNoDataNodes() { | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
ClusterState state = setUpClusterWithNoDataNodes(); | ||||||
int dataNodes = state.nodes().getDataNodes().size(); | ||||||
int masterNodes = state.nodes().getMasterNodes().size(); | ||||||
assertTrue(dataNodes == 0); | ||||||
assertTrue(masterNodes > 0); | ||||||
ClusterHealthStatus clusterHealthStatus = new ClusterStateHealth(state).getStatus(); | ||||||
assertEquals(ClusterHealthStatus.RED, clusterHealthStatus); | ||||||
} | ||||||
|
||||||
/** | ||||||
* The method test for scenario where we have a cluster with indices and data nodes | ||||||
* and then all data nodes gets terminated now new index is created then | ||||||
* all indices and cluster health should be red, but last allocation attempts of new index shards v/s | ||||||
* old index shards should be different. | ||||||
*/ | ||||||
public void testAllocationStatusForTerminatedNodes() { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great stuff. As far as I can tell, we don't have any other tests that assert things about cluster health in an I think it'd be simpler to work with a single index per test, rather than dealing with the two indices here. Also it'd be good to test yellow health as well as the red and green cases. How about the following flow instead?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. I think this flow will be better and I will remove below assertions and make test case focus more on only Cluster health. |
||||||
//creates one master and two data nodes | ||||||
DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The master node isn't really necessary here but written like this it looks like it's important to this test. If you wanted, you could reasonably add a random number (possibly zero) non-data nodes (master nodes, coordinating-only nodes, etc) to document that the non-data nodes aren't important. |
||||||
.add(newNode("node_d1", Collections.singleton(DiscoveryNode.Role.DATA))) | ||||||
.add(newNode("node_d2", Collections.singleton(DiscoveryNode.Role.DATA))); | ||||||
MetaData metaData = MetaData.builder() | ||||||
.put(IndexMetaData.builder("TestIndex") | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
.settings(settings(Version.CURRENT)) | ||||||
.numberOfShards(2) | ||||||
.numberOfReplicas(0)) | ||||||
.build(); | ||||||
RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); | ||||||
ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
.nodes(nodeBuilder.build()) | ||||||
.metaData(metaData) | ||||||
.routingTable(routingTable) | ||||||
.build(); | ||||||
MockAllocationService allocationService = createAllocationService(); | ||||||
//perform allocation of TestIndex | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
state = allocationService.reroute(state, "Test_allocation"); | ||||||
state = allocationService.applyStartedShards(state, state.getRoutingNodes().shardsWithState(INITIALIZING)); | ||||||
IndexMetaData.Builder idxMetaBuilder = IndexMetaData.builder(state.metaData().index("TestIndex")); | ||||||
for (final ShardRouting shards : state.getRoutingTable().index("TestIndex").shardsWithState(STARTED)) { | ||||||
idxMetaBuilder.putInSyncAllocationIds(shards.getId(), Sets.newHashSet(shards.allocationId().getId())); | ||||||
} | ||||||
state = ClusterState.builder(state).metaData(MetaData.builder(state.metaData()).put(idxMetaBuilder)).build(); | ||||||
//asserting the cluster is in green after TestIndex Creation | ||||||
assertEquals(ClusterHealthStatus.GREEN, new ClusterStateHealth(state).getStatus()); | ||||||
//Terminating data nodes | ||||||
state = ClusterState.builder(state) | ||||||
.nodes(DiscoveryNodes.builder(state.getNodes()) | ||||||
.remove("node_d1").remove("node_d2").build()) | ||||||
.build(); | ||||||
//Removing dead nodes from cluster with a cluster reroute | ||||||
state = allocationService.deassociateDeadNodes(state, true, "Test_allocation"); | ||||||
//asserting that a cluster state goes Red after data nodes goes terminated | ||||||
assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); | ||||||
//Creating NewTestIndex meta deta | ||||||
metaData = MetaData.builder(state.metaData()) | ||||||
.put(IndexMetaData.builder("NewTestIndex") | ||||||
.settings(settings(Version.CURRENT)) | ||||||
.numberOfShards(2) | ||||||
.numberOfReplicas(0)) | ||||||
.build(); | ||||||
//changed cluster state | ||||||
state = ClusterState.builder(state) | ||||||
.metaData(metaData) | ||||||
.routingTable(RoutingTable.builder(state.getRoutingTable()).addAsNew(metaData.index("NewTestIndex")).build()).build(); | ||||||
//allocation after newly created index | ||||||
state = allocationService.reroute(state, "no data nodes"); | ||||||
assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); | ||||||
RoutingNodes routingNodes = state.getRoutingNodes(); | ||||||
RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); | ||||||
assertFalse(unassignedShards.isEmpty()); | ||||||
RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); | ||||||
while (unassignedIterator.hasNext()) { | ||||||
ShardRouting shard = unassignedIterator.next(); | ||||||
UnassignedInfo shardInfo = shard.unassignedInfo(); | ||||||
/* asserting that the TestIndex shards have different AllocationStatus than DECIDERS_NO | ||||||
and NewTestIndex status is DECIDERS_NO */ | ||||||
if (shard.getIndexName().equals("TestIndex")) { | ||||||
assertNotEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); | ||||||
Gaurav614 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} else if (shard.getIndexName().equals("NewTestIndex")) { | ||||||
assertEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); | ||||||
} | ||||||
} | ||||||
} | ||||||
} |
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.
No need for this comment if the method is better-named.
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.
Will resolve in next revision. Please review other changes