Skip to content

Fix cluster health when closing #61709

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Sep 19, 2020
Merged

Fix cluster health when closing #61709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
809c2d6
Fix cluster health when closing
henningandersen Aug 31, 2020
5687146
Merge remote-tracking branch 'origin/master' into fix_cluster_health_…
henningandersen Sep 8, 2020
cf46543
Better way to provoke wait in test case
henningandersen Sep 8, 2020
1d9c966
Merge remote-tracking branch 'origin/master' into fix_cluster_health_…
henningandersen Sep 9, 2020
b89d999
Merge remote-tracking branch 'origin/master' into fix_cluster_health_…
henningandersen Sep 18, 2020
89cc97f
Add comment
henningandersen Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion server/src/internalClusterTest/java/org/elasticsearch/cluster/ClusterHealthIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import org.elasticsearch.action.ActionFuture;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse;
import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
Expand Down Expand Up @@ -309,14 +310,29 @@ public void clusterStateProcessed(String source, ClusterState oldState, ClusterS

public void testHealthOnMasterFailover() throws Exception {
final String node = internalCluster().startDataOnlyNode();
boolean withIndex = randomBoolean();
if (withIndex) {
// Create index with many shards to provoke the health request to wait (for green) while master is being shut down.
// Notice that this is set to 0 after the test completed starting a number of health requests and master restarts.
// This ensures that the cluster is yellow when the health request is made, making the health request wait on the observer,
// triggering a call to observer.onClusterServiceClose when master is shutdown.
createIndex("test", Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, randomIntBetween(0, 10)).build());
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused. How can the cluster ever be green with that many replicas? Should this be number_of_shards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ensures that the cluster is yellow when the health request is made, making the health request wait on the observer, triggering the call to onClusterServiceClose when master is shutdown.

The number of replicas is cleared to 0 after having fired all the async restarts and done the master restarts. That ensures that all the requests responds with green status.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah ok, can you add a comment to that effect

}
final List<ActionFuture<ClusterHealthResponse>> responseFutures = new ArrayList<>();
// Run a few health requests concurrent to master fail-overs against a data-node to make sure master failover is handled
// without exceptions
for (int i = 0; i < 20; ++i) {
responseFutures.add(client(node).admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID)
.setWaitForGreenStatus().execute());
.setWaitForGreenStatus().setMasterNodeTimeout(TimeValue.timeValueMinutes(1)).execute());
internalCluster().restartNode(internalCluster().getMasterName(), InternalTestCluster.EMPTY_CALLBACK);
}
if (withIndex) {
assertAcked(
client().admin().indices()
.updateSettings(new UpdateSettingsRequest("test")
.settings(Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0))).get()
);
}
for (ActionFuture<ClusterHealthResponse> responseFuture : responseFutures) {
assertSame(responseFuture.get().getStatus(), ClusterHealthStatus.GREEN);
}
Expand Down
3 changes: 2 additions & 1 deletion ...main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.CollectionUtils;
import org.elasticsearch.index.IndexNotFoundException;
import org.elasticsearch.node.NodeClosedException;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;
Expand Down Expand Up @@ -204,7 +205,7 @@ public void onNewClusterState(ClusterState newState) {

@Override
public void onClusterServiceClose() {
listener.onFailure(new IllegalStateException("ClusterService was close during health call"));
listener.onFailure(new NodeClosedException(clusterService.localNode()));
}

@Override
Expand Down