From 10333e2f05c9673a85fb0a2f1e3e763f51629349 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 15 Mar 2016 11:39:45 +0100 Subject: [PATCH] IndicesStore checks for `allocated elsewhere` for every shard not allocated on the local node On each clusterstate update we check on the local node if we can delete some shards content. For this we linearly walk all shards and check if they are allocated and started on another node and if we can delete them locally. if we can delete them locally we go and ask other nodes if we can delete them and then if the shared IS active elsewhere issue a state update task to delete it. Yet, there is a bug in IndicesService#canDeleteShardContent which returns `true` even if that shards datapath doesn't exist on the node which causes tons of unnecessary node to node communciation and as many state update task to be issued. This can have large impact on the cluster state processing speed. **NOTE:** This only happens for shards that have at least one shard allocated on the node ie. if an `IndexService` exists. Closes #17106 --- .../org/elasticsearch/indices/IndicesService.java | 13 +++++++++---- .../elasticsearch/indices/IndicesServiceTests.java | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index 06eb71724c8..b0e1bbdbd2b 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -624,12 +624,17 @@ public class IndicesService extends AbstractLifecycleComponent i assert shardId.getIndex().equals(indexSettings.getIndex()); final IndexService indexService = indexService(shardId.getIndex()); if (indexSettings.isOnSharedFilesystem() == false) { - if (indexService != null && nodeEnv.hasNodeFile()) { - return indexService.hasShard(shardId.id()) == false; - } else if (nodeEnv.hasNodeFile()) { - if (indexSettings.hasCustomDataPath()) { + if (nodeEnv.hasNodeFile()) { + final boolean isAllocated = indexService != null && indexService.hasShard(shardId.id()); + if (isAllocated) { + return false; // we are allocated - can't delete the shard + } else if (indexSettings.hasCustomDataPath()) { + // lets see if it's on a custom path (return false if the shared doesn't exist) + // we don't need to delete anything that is not there return Files.exists(nodeEnv.resolveCustomLocation(indexSettings, shardId)); } else { + // lets see if it's path is available (return false if the shared doesn't exist) + // we don't need to delete anything that is not there return FileSystemUtils.exists(nodeEnv.availableShardPaths(shardId)); } } diff --git a/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java b/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java index 57a7f34e4b7..336d5a84a8d 100644 --- a/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java +++ b/core/src/test/java/org/elasticsearch/indices/IndicesServiceTests.java @@ -81,6 +81,8 @@ public class IndicesServiceTests extends ESSingleNodeTestCase { assertFalse("shard is allocated", indicesService.canDeleteShardContent(shardId, test.getIndexSettings())); test.removeShard(0, "boom"); assertTrue("shard is removed", indicesService.canDeleteShardContent(shardId, test.getIndexSettings())); + ShardId notAllocated = new ShardId(test.index(), 100); + assertFalse("shard that was never on this node should NOT be deletable", indicesService.canDeleteShardContent(notAllocated, test.getIndexSettings())); } public void testDeleteIndexStore() throws Exception {