From db5361a096c4ea3870fe3cf91948c9cbe63897cf Mon Sep 17 00:00:00 2001 From: Jake Landis Date: Wed, 27 Sep 2023 13:04:19 -0500 Subject: [PATCH] adopt canSearchShard --- .../cluster/routing/IndexRoutingTable.java | 5 +- .../cluster/routing/ShardRouting.java | 6 ++ .../routing/IndexRoutingTableTests.java | 84 ++++++++++++++++--- .../support/SecurityIndexManager.java | 2 +- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java index def9b1be1df72..8fbdd3790e158 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/IndexRoutingTable.java @@ -8,6 +8,7 @@ package org.elasticsearch.cluster.routing; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.Diff; import org.elasticsearch.cluster.SimpleDiffable; import org.elasticsearch.cluster.metadata.IndexMetadata; @@ -234,12 +235,12 @@ public boolean allPrimaryShardsActive() { /** * @return true if an index is available to service search queries. */ - public boolean readyForSearch() { + public boolean readyForSearch(ClusterState clusterState) { for (IndexShardRoutingTable shardRoutingTable : this.shards) { boolean found = false; for (int idx = 0; idx < shardRoutingTable.size(); idx++) { ShardRouting shardRouting = shardRoutingTable.shard(idx); - if (shardRouting.active() && shardRouting.role().isSearchable()) { + if (shardRouting.active() && OperationRouting.canSearchShard(shardRouting, clusterState)) { found = true; break; } diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java index b0c79102fcd80..90bcc8a7270cb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/ShardRouting.java @@ -10,6 +10,7 @@ import org.elasticsearch.TransportVersion; import org.elasticsearch.TransportVersions; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.routing.RecoverySource.ExistingStoreRecoverySource; import org.elasticsearch.cluster.routing.RecoverySource.PeerRecoverySource; @@ -930,6 +931,11 @@ public boolean isPromotableToPrimary() { return role.isPromotableToPrimary(); } + /** + * Determine if role searchable. Consumers should prefer {@link OperationRouting#canSearchShard(ShardRouting, ClusterState)} to + * determine if a shard can be searched and {@link IndexRoutingTable#readyForSearch(ClusterState)} to determine if an index + * is ready to be searched. + */ public boolean isSearchable() { return role.isSearchable(); } diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTableTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTableTests.java index 383305180647f..788a8a235dfa5 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTableTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/IndexRoutingTableTests.java @@ -8,17 +8,34 @@ package org.elasticsearch.cluster.routing; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.UUIDs; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.Index; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.test.ESTestCase; +import org.mockito.Mockito; import java.util.List; +import static org.elasticsearch.index.IndexSettings.INDEX_FAST_REFRESH_SETTING; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class IndexRoutingTableTests extends ESTestCase { public void testReadyForSearch() { + innerReadyForSearch(false); + innerReadyForSearch(true); + } + + private void innerReadyForSearch(boolean fastRefresh) { Index index = new Index(randomIdentifier(), UUIDs.randomBase64UUID()); + ClusterState clusterState = mock(ClusterState.class, Mockito.RETURNS_DEEP_STUBS); + when(clusterState.metadata().index(any(Index.class)).getSettings()).thenReturn( + Settings.builder().put(INDEX_FAST_REFRESH_SETTING.getKey(), fastRefresh).build() + ); // 2 primaries that are search and index ShardId p1 = new ShardId(index, 0); IndexShardRoutingTable shardTable1 = new IndexShardRoutingTable( @@ -31,25 +48,35 @@ public void testReadyForSearch() { List.of(getShard(p2, true, ShardRoutingState.STARTED, ShardRouting.Role.DEFAULT)) ); IndexRoutingTable indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); - assertTrue(indexRoutingTable.readyForSearch()); + assertTrue(indexRoutingTable.readyForSearch(clusterState)); // 2 primaries that are index only + shardTable1 = new IndexShardRoutingTable(p1, List.of(getShard(p1, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY))); + shardTable2 = new IndexShardRoutingTable(p2, List.of(getShard(p2, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY))); + indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); + if (fastRefresh) { + assertTrue(indexRoutingTable.readyForSearch(clusterState)); + } else { + assertFalse(indexRoutingTable.readyForSearch(clusterState)); + } + + // 2 unassigned primaries that are index only shardTable1 = new IndexShardRoutingTable( p1, - List.of(getShard(p1, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY)) + List.of(getShard(p1, true, ShardRoutingState.UNASSIGNED, ShardRouting.Role.INDEX_ONLY)) ); shardTable2 = new IndexShardRoutingTable( p2, - List.of(getShard(p2, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY)) + List.of(getShard(p2, true, ShardRoutingState.UNASSIGNED, ShardRouting.Role.INDEX_ONLY)) ); indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); - assertFalse(indexRoutingTable.readyForSearch()); + assertFalse(indexRoutingTable.readyForSearch(clusterState)); // 2 primaries that are index only with replicas that are not all available shardTable1 = new IndexShardRoutingTable( p1, List.of( - getShard(p1, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p1, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) ) @@ -57,19 +84,46 @@ public void testReadyForSearch() { shardTable2 = new IndexShardRoutingTable( p2, List.of( - getShard(p2, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p2, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), getShard(p2, false, ShardRoutingState.UNASSIGNED, ShardRouting.Role.SEARCH_ONLY), getShard(p2, false, ShardRoutingState.UNASSIGNED, ShardRouting.Role.SEARCH_ONLY) ) ); indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); - assertFalse(indexRoutingTable.readyForSearch()); + if (fastRefresh) { + assertTrue(indexRoutingTable.readyForSearch(clusterState)); + } else { + assertFalse(indexRoutingTable.readyForSearch(clusterState)); + } // 2 primaries that are index only with some replicas that are all available shardTable1 = new IndexShardRoutingTable( p1, List.of( - getShard(p1, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p1, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), + getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), + getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) + ) + ); + shardTable2 = new IndexShardRoutingTable( + p2, + List.of( + getShard(p2, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), + getShard(p2, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), + getShard(p2, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) + ) + ); + indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); + assertTrue(indexRoutingTable.readyForSearch(clusterState)); + + // 2 unassigned primaries that are index only with some replicas that are all available + // TODO: Is this correct behavior for fastRefresh ? Fast refresh indices do not support replicas so this can not practically happen. + // However, if we add support we will want to ensure that readyForSearch/canSearchShard allows for searching replicas for + // indices with fast refresh for when the index shard is not available. + shardTable1 = new IndexShardRoutingTable( + p1, + List.of( + getShard(p1, true, ShardRoutingState.UNASSIGNED, ShardRouting.Role.INDEX_ONLY), getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) ) @@ -77,19 +131,23 @@ public void testReadyForSearch() { shardTable2 = new IndexShardRoutingTable( p2, List.of( - getShard(p2, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p2, true, ShardRoutingState.UNASSIGNED, ShardRouting.Role.INDEX_ONLY), getShard(p2, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), getShard(p2, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) ) ); indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); - assertTrue(indexRoutingTable.readyForSearch()); + if (fastRefresh) { + assertFalse(indexRoutingTable.readyForSearch(clusterState)); // this is wrong if ever support replicas for fast refreshes + } else { + assertTrue(indexRoutingTable.readyForSearch(clusterState)); + } // 2 primaries that are index only with at least 1 replica per primary that is available shardTable1 = new IndexShardRoutingTable( p1, List.of( - getShard(p1, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p1, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), getShard(p1, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY), getShard(p1, false, ShardRoutingState.UNASSIGNED, ShardRouting.Role.SEARCH_ONLY) ) @@ -97,13 +155,13 @@ public void testReadyForSearch() { shardTable2 = new IndexShardRoutingTable( p2, List.of( - getShard(p2, true, randomFrom(ShardRoutingState.values()), ShardRouting.Role.INDEX_ONLY), + getShard(p2, true, ShardRoutingState.STARTED, ShardRouting.Role.INDEX_ONLY), getShard(p2, false, ShardRoutingState.UNASSIGNED, ShardRouting.Role.SEARCH_ONLY), getShard(p2, false, ShardRoutingState.STARTED, ShardRouting.Role.SEARCH_ONLY) ) ); indexRoutingTable = new IndexRoutingTable(index, new IndexShardRoutingTable[] { shardTable1, shardTable2 }); - assertTrue(indexRoutingTable.readyForSearch()); + assertTrue(indexRoutingTable.readyForSearch(clusterState)); } private ShardRouting getShard(ShardId shardId, boolean isPrimary, ShardRoutingState state, ShardRouting.Role role) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java index 0edb1a04449e5..048dd2fd7a925 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/support/SecurityIndexManager.java @@ -242,7 +242,7 @@ private boolean checkIndexAvailable(ClusterState state) { return false; } final IndexRoutingTable routingTable = state.routingTable().index(metadata.getIndex()); - if (routingTable == null || routingTable.readyForSearch() == false) { + if (routingTable == null || routingTable.readyForSearch(state) == false) { logger.debug("Index [{}] is not yet active", aliasName); return false; } else {