diff --git a/server/src/main/java/org/opensearch/action/get/TransportGetAction.java b/server/src/main/java/org/opensearch/action/get/TransportGetAction.java index 0c444732fb12b..00a795c86356f 100644 --- a/server/src/main/java/org/opensearch/action/get/TransportGetAction.java +++ b/server/src/main/java/org/opensearch/action/get/TransportGetAction.java @@ -37,6 +37,7 @@ import org.opensearch.action.support.single.shard.TransportSingleShardAction; import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; +import org.opensearch.cluster.metadata.Metadata; import org.opensearch.cluster.routing.Preference; import org.opensearch.cluster.routing.ShardIterator; import org.opensearch.cluster.service.ClusterService; @@ -92,8 +93,8 @@ protected boolean resolveIndex(GetRequest request) { /** * Returns true if GET request should be routed to primary shards, else false. */ - protected static boolean shouldForcePrimaryRouting(ClusterState state, boolean realtime, String preference, String indexName) { - return state.isSegmentReplicationEnabled(indexName) && realtime && preference == null; + protected static boolean shouldForcePrimaryRouting(Metadata metadata, boolean realtime, String preference, String indexName) { + return metadata.isSegmentReplicationEnabled(indexName) && realtime && preference == null; } @Override @@ -101,7 +102,12 @@ protected ShardIterator shards(ClusterState state, InternalRequest request) { final String preference; // route realtime GET requests when segment replication is enabled to primary shards, // iff there are no other preferences/routings enabled for routing to a specific shard - if (shouldForcePrimaryRouting(state, request.request().realtime, request.request().preference(), request.concreteIndex())) { + if (shouldForcePrimaryRouting( + state.getMetadata(), + request.request().realtime, + request.request().preference(), + request.concreteIndex() + )) { preference = Preference.PRIMARY.type(); } else { preference = request.request().preference(); diff --git a/server/src/main/java/org/opensearch/action/get/TransportMultiGetAction.java b/server/src/main/java/org/opensearch/action/get/TransportMultiGetAction.java index a1a74208dc725..5d58d09d7a320 100644 --- a/server/src/main/java/org/opensearch/action/get/TransportMultiGetAction.java +++ b/server/src/main/java/org/opensearch/action/get/TransportMultiGetAction.java @@ -112,7 +112,7 @@ protected void doExecute(Task task, final MultiGetRequest request, final ActionL MultiGetShardRequest shardRequest = shardRequests.get(shardId); if (shardRequest == null) { - if (shouldForcePrimaryRouting(clusterState, request.realtime, request.preference, concreteSingleIndex)) { + if (shouldForcePrimaryRouting(clusterState.getMetadata(), request.realtime, request.preference, concreteSingleIndex)) { request.preference(Preference.PRIMARY.type()); } shardRequest = new MultiGetShardRequest(request, shardId.getIndexName(), shardId.getId()); diff --git a/server/src/main/java/org/opensearch/cluster/ClusterState.java b/server/src/main/java/org/opensearch/cluster/ClusterState.java index 2fd58d3db4975..1b87a60c2ccf5 100644 --- a/server/src/main/java/org/opensearch/cluster/ClusterState.java +++ b/server/src/main/java/org/opensearch/cluster/ClusterState.java @@ -61,7 +61,6 @@ import org.opensearch.core.xcontent.ToXContentFragment; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.discovery.Discovery; -import org.opensearch.indices.replication.common.ReplicationType; import java.io.IOException; import java.util.Collections; @@ -410,21 +409,6 @@ public boolean supersedes(ClusterState other) { } - /** - * Utility to identify whether input index belongs to SEGMENT replication in established cluster state. - * - * @param indexName Index name - * @return true if index belong SEGMENT replication, false otherwise - */ - public boolean isSegmentReplicationEnabled(String indexName) { - return Optional.ofNullable(this.getMetadata().index(indexName)) - .map( - indexMetadata -> ReplicationType.parseString(indexMetadata.getSettings().get(IndexMetadata.SETTING_REPLICATION_TYPE)) - .equals(ReplicationType.SEGMENT) - ) - .orElse(false); - } - /** * Metrics for cluster state. * diff --git a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java index 13a27f76a181c..bd6fec9c90d5c 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/Metadata.java @@ -66,6 +66,7 @@ import org.opensearch.core.xcontent.XContentParser; import org.opensearch.gateway.MetadataStateFormat; import org.opensearch.index.IndexNotFoundException; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.plugins.MapperPlugin; import java.io.IOException; @@ -107,6 +108,21 @@ public class Metadata implements Iterable, Diffable, To public static final String UNKNOWN_CLUSTER_UUID = Strings.UNKNOWN_UUID_VALUE; public static final Pattern NUMBER_PATTERN = Pattern.compile("[0-9]+$"); + /** + * Utility to identify whether input index uses SEGMENT replication strategy in established cluster state metadata. + * + * @param indexName Index name + * @return true if index uses SEGMENT replication, false otherwise + */ + public boolean isSegmentReplicationEnabled(String indexName) { + return Optional.ofNullable(index(indexName)) + .map( + indexMetadata -> ReplicationType.parseString(indexMetadata.getSettings().get(IndexMetadata.SETTING_REPLICATION_TYPE)) + .equals(ReplicationType.SEGMENT) + ) + .orElse(false); + } + /** * Context of the XContent. * diff --git a/server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java b/server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java index f78977492a168..320e3ddf7a59c 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java +++ b/server/src/main/java/org/opensearch/cluster/routing/RoutingNodes.java @@ -66,8 +66,6 @@ import java.util.stream.Collectors; import java.util.stream.Stream; -import static org.opensearch.action.get.TransportGetAction.isSegmentReplicationEnabled; - /** * {@link RoutingNodes} represents a copy the routing information contained in the {@link ClusterState cluster state}. * It can be either initialized as mutable or immutable (see {@link #RoutingNodes(ClusterState, boolean)}), allowing @@ -84,7 +82,7 @@ * @opensearch.internal */ public class RoutingNodes implements Iterable { - private final ClusterState clusterState; + private final Metadata metadata; private final Map nodesToShards = new HashMap<>(); @@ -110,7 +108,7 @@ public RoutingNodes(ClusterState clusterState) { } public RoutingNodes(ClusterState clusterState, boolean readOnly) { - this.clusterState = clusterState; + this.metadata = clusterState.getMetadata(); this.readOnly = readOnly; final RoutingTable routingTable = clusterState.routingTable(); this.nodesPerAttributeNames = Collections.synchronizedMap(new HashMap<>()); @@ -387,7 +385,7 @@ public ShardRouting activeReplicaBasedOnReplicationStrategy(ShardId shardId) { Stream candidateShards = assignedShards(shardId).stream() .filter(shr -> !shr.primary() && shr.active()) .filter(shr -> node(shr.currentNodeId()) != null); - if (isSegmentReplicationEnabled(clusterState, shardId.getIndexName())) { + if (metadata.isSegmentReplicationEnabled(shardId.getIndexName())) { return candidateShards.min( Comparator.comparing( shr -> node(shr.currentNodeId()).node(), diff --git a/server/src/test/java/org/opensearch/action/get/TransportGetActionTests.java b/server/src/test/java/org/opensearch/action/get/TransportGetActionTests.java index 2eca49fb3032f..9565e219d1a78 100644 --- a/server/src/test/java/org/opensearch/action/get/TransportGetActionTests.java +++ b/server/src/test/java/org/opensearch/action/get/TransportGetActionTests.java @@ -67,24 +67,24 @@ private static ClusterState clusterState(ReplicationType replicationType) { public void testShouldForcePrimaryRouting() { - ClusterState clusterState = clusterState(ReplicationType.SEGMENT); + Metadata metadata = clusterState(ReplicationType.SEGMENT).getMetadata(); // should return false since preference is set for request - assertFalse(TransportGetAction.shouldForcePrimaryRouting(clusterState, true, Preference.REPLICA.type(), "index1")); + assertFalse(TransportGetAction.shouldForcePrimaryRouting(metadata, true, Preference.REPLICA.type(), "index1")); // should return false since request is not realtime - assertFalse(TransportGetAction.shouldForcePrimaryRouting(clusterState, false, null, "index1")); + assertFalse(TransportGetAction.shouldForcePrimaryRouting(metadata, false, null, "index1")); // should return true since segment replication is enabled - assertTrue(TransportGetAction.shouldForcePrimaryRouting(clusterState, true, null, "index1")); + assertTrue(TransportGetAction.shouldForcePrimaryRouting(metadata, true, null, "index1")); // should return false since index doesn't exist - assertFalse(TransportGetAction.shouldForcePrimaryRouting(clusterState, true, null, "index3")); + assertFalse(TransportGetAction.shouldForcePrimaryRouting(metadata, true, null, "index3")); - clusterState = clusterState(ReplicationType.DOCUMENT); + metadata = clusterState(ReplicationType.DOCUMENT).getMetadata(); // should fail since document replication enabled - assertFalse(TransportGetAction.shouldForcePrimaryRouting(clusterState, true, null, "index1")); + assertFalse(TransportGetAction.shouldForcePrimaryRouting(metadata, true, null, "index1")); } diff --git a/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java b/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java index c4fb3271ae3ce..457bdac1809ef 100644 --- a/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java +++ b/server/src/test/java/org/opensearch/cluster/ClusterStateTests.java @@ -57,7 +57,6 @@ import org.opensearch.core.rest.RestStatus; import org.opensearch.core.xcontent.ToXContent; import org.opensearch.core.xcontent.XContentBuilder; -import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.test.TestCustomMetadata; @@ -117,39 +116,6 @@ public void testSupersedes() { ); } - public void testIsSegmentReplicationEnabled() { - final String indexName = "test"; - ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build(); - Settings.Builder builder = settings(Version.CURRENT).put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT); - IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexName) - .settings(builder) - .numberOfShards(1) - .numberOfReplicas(1); - Metadata.Builder metadataBuilder = Metadata.builder().put(indexMetadataBuilder); - RoutingTable.Builder routingTableBuilder = RoutingTable.builder().addAsNew(indexMetadataBuilder.build()); - clusterState = ClusterState.builder(clusterState) - .metadata(metadataBuilder.build()) - .routingTable(routingTableBuilder.build()) - .build(); - assertTrue(clusterState.isSegmentReplicationEnabled(indexName)); - } - - public void testIsSegmentReplicationDisabled() { - final String indexName = "test"; - ClusterState clusterState = ClusterState.builder(CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build(); - IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexName) - .settings(settings(Version.CURRENT)) - .numberOfShards(1) - .numberOfReplicas(1); - Metadata.Builder metadataBuilder = Metadata.builder().put(indexMetadataBuilder); - RoutingTable.Builder routingTableBuilder = RoutingTable.builder().addAsNew(indexMetadataBuilder.build()); - clusterState = ClusterState.builder(clusterState) - .metadata(metadataBuilder.build()) - .routingTable(routingTableBuilder.build()) - .build(); - assertFalse(clusterState.isSegmentReplicationEnabled(indexName)); - } - public void testBuilderRejectsNullCustom() { final ClusterState.Builder builder = ClusterState.builder(ClusterName.DEFAULT); final String key = randomAlphaOfLength(10); diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java index fff39d14e9702..40eefa6cdbf03 100644 --- a/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java +++ b/server/src/test/java/org/opensearch/cluster/metadata/MetadataTests.java @@ -52,6 +52,7 @@ import org.opensearch.core.index.Index; import org.opensearch.core.xcontent.XContentBuilder; import org.opensearch.core.xcontent.XContentParser; +import org.opensearch.indices.replication.common.ReplicationType; import org.opensearch.plugins.MapperPlugin; import org.opensearch.test.OpenSearchTestCase; @@ -1425,6 +1426,29 @@ public void testMetadataBuildInvocations() { compareMetadata(previousMetadata, builtMetadata, false, true, true); } + public void testIsSegmentReplicationEnabled() { + final String indexName = "test"; + Settings.Builder builder = settings(Version.CURRENT).put(IndexMetadata.SETTING_REPLICATION_TYPE, ReplicationType.SEGMENT); + IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexName) + .settings(builder) + .numberOfShards(1) + .numberOfReplicas(1); + Metadata.Builder metadataBuilder = Metadata.builder().put(indexMetadataBuilder); + Metadata metadata = metadataBuilder.build(); + assertTrue(metadata.isSegmentReplicationEnabled(indexName)); + } + + public void testIsSegmentReplicationDisabled() { + final String indexName = "test"; + IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexName) + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(1); + Metadata.Builder metadataBuilder = Metadata.builder().put(indexMetadataBuilder); + Metadata metadata = metadataBuilder.build(); + assertFalse(metadata.isSegmentReplicationEnabled(indexName)); + } + public static Metadata randomMetadata() { Metadata.Builder md = Metadata.builder() .put(buildIndexMetadata("index", "alias", randomBoolean() ? null : randomBoolean()).build(), randomBoolean())