From 91a80aca8bf58379d88db4c34ea8cde4e0889495 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 16 Jan 2023 18:18:42 +0530 Subject: [PATCH 1/3] Disallow preference search with strict weighted shard routing Signed-off-by: Anshu Agarwal --- .../search/SearchWeightedRoutingIT.java | 64 +++++++++++++++++++ .../cluster/routing/OperationRouting.java | 19 ++++++ ...ferenceBasedSearchNotAllowedException.java | 37 +++++++++++ .../common/settings/ClusterSettings.java | 1 + 4 files changed, 121 insertions(+) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/PreferenceBasedSearchNotAllowedException.java diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java index b0afbc6983c95..0d5e256ef1596 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java @@ -20,6 +20,7 @@ import org.opensearch.action.search.SearchResponse; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.node.DiscoveryNodes; +import org.opensearch.cluster.routing.PreferenceBasedSearchNotAllowedException; import org.opensearch.cluster.routing.WeightedRouting; import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider; import org.opensearch.common.collect.ImmutableOpenMap; @@ -27,6 +28,7 @@ import org.opensearch.index.query.QueryBuilders; import org.opensearch.index.search.stats.SearchStats; import org.opensearch.plugins.Plugin; +import org.opensearch.rest.RestStatus; import org.opensearch.search.aggregations.Aggregations; import org.opensearch.search.aggregations.bucket.terms.Terms; import org.opensearch.snapshots.mockstore.MockRepository; @@ -796,4 +798,66 @@ public void testMultiGetWithNetworkDisruption_FailOpenDisabled() throws Exceptio ); } + /** + * Assert that preference based search is not allowed with strict weighted shard routing + * @throws Exception throws exception + */ + public void testStrictWeightedRouting() throws Exception { + + Settings commonSettings = Settings.builder() + .put("cluster.routing.allocation.awareness.attributes", "zone") + .put("cluster.routing.allocation.awareness.force.zone.values", "a,b,c") + .put("cluster.routing.weighted.fail_open", true) + .put("cluster.routing.weighted.strict", true) + .build(); + + int nodeCountPerAZ = 1; + Map> nodeMap = setupCluster(nodeCountPerAZ, commonSettings); + + int numShards = 10; + int numReplicas = 1; + setUpIndexing(numShards, numReplicas); + + logger.info("--> setting shard routing weights for weighted round robin"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); + setShardRoutingWeights(weights); + + assertThrows( + PreferenceBasedSearchNotAllowedException.class, + () -> internalCluster().client(nodeMap.get("b").get(0)).prepareSearch().setSize(0).setPreference("_only_local").get() + ); + + } + + /** + * Assert that preference based search works with non-strict weighted shard routing + * @throws Exception + */ + public void testPreferenceSearchWithWeightedRouting() throws Exception { + Settings commonSettings = Settings.builder() + .put("cluster.routing.allocation.awareness.attributes", "zone") + .put("cluster.routing.allocation.awareness.force.zone.values", "a,b,c") + .put("cluster.routing.weighted.fail_open", true) + .put("cluster.routing.weighted.strict", false) + .build(); + + int nodeCountPerAZ = 1; + Map> nodeMap = setupCluster(nodeCountPerAZ, commonSettings); + + int numShards = 10; + int numReplicas = 1; + setUpIndexing(numShards, numReplicas); + + logger.info("--> setting shard routing weights for weighted round robin"); + Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); + setShardRoutingWeights(weights); + + SearchResponse searchResponse = internalCluster().client(nodeMap.get("b").get(0)) + .prepareSearch() + .setSize(0) + .setPreference("_only_local") + .get(); + assertEquals(RestStatus.OK.getStatus(), searchResponse.status().getStatus()); + } + } diff --git a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java index d7df1a2c2181b..cb20e223c9e20 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java +++ b/server/src/main/java/org/opensearch/cluster/routing/OperationRouting.java @@ -90,11 +90,19 @@ public class OperationRouting { Setting.Property.Dynamic, Setting.Property.NodeScope ); + + public static final Setting STRICT_WEIGHTED_SHARD_ROUTING_ENABLED = Setting.boolSetting( + "cluster.routing.weighted.strict", + false, + Setting.Property.Dynamic, + Setting.Property.NodeScope + ); private volatile List awarenessAttributes; private volatile boolean useAdaptiveReplicaSelection; private volatile boolean ignoreAwarenessAttr; private volatile double weightedRoutingDefaultWeight; private volatile boolean isFailOpenEnabled; + private volatile boolean isStrictWeightedShardRouting; public OperationRouting(Settings settings, ClusterSettings clusterSettings) { // whether to ignore awareness attributes when routing requests @@ -107,10 +115,12 @@ public OperationRouting(Settings settings, ClusterSettings clusterSettings) { this.useAdaptiveReplicaSelection = USE_ADAPTIVE_REPLICA_SELECTION_SETTING.get(settings); this.weightedRoutingDefaultWeight = WEIGHTED_ROUTING_DEFAULT_WEIGHT.get(settings); this.isFailOpenEnabled = WEIGHTED_ROUTING_FAILOPEN_ENABLED.get(settings); + this.isStrictWeightedShardRouting = STRICT_WEIGHTED_SHARD_ROUTING_ENABLED.get(settings); clusterSettings.addSettingsUpdateConsumer(USE_ADAPTIVE_REPLICA_SELECTION_SETTING, this::setUseAdaptiveReplicaSelection); clusterSettings.addSettingsUpdateConsumer(IGNORE_AWARENESS_ATTRIBUTES_SETTING, this::setIgnoreAwarenessAttributes); clusterSettings.addSettingsUpdateConsumer(WEIGHTED_ROUTING_DEFAULT_WEIGHT, this::setWeightedRoutingDefaultWeight); clusterSettings.addSettingsUpdateConsumer(WEIGHTED_ROUTING_FAILOPEN_ENABLED, this::setFailOpenEnabled); + clusterSettings.addSettingsUpdateConsumer(STRICT_WEIGHTED_SHARD_ROUTING_ENABLED, this::setStrictWeightedShardRouting); } void setUseAdaptiveReplicaSelection(boolean useAdaptiveReplicaSelection) { @@ -129,6 +139,10 @@ void setFailOpenEnabled(boolean isFailOpenEnabled) { this.isFailOpenEnabled = isFailOpenEnabled; } + void setStrictWeightedShardRouting(boolean strictWeightedShardRouting) { + this.isStrictWeightedShardRouting = strictWeightedShardRouting; + } + public boolean isIgnoreAwarenessAttr() { return ignoreAwarenessAttr; } @@ -267,6 +281,11 @@ private ShardIterator preferenceActiveShardIterator( if (preference == null || preference.isEmpty()) { return shardRoutings(indexShard, nodes, collectorService, nodeCounts, weightedRoutingMetadata); } + if (weightedRoutingMetadata != null && weightedRoutingMetadata.getWeightedRouting().isSet() && isStrictWeightedShardRouting) { + throw new PreferenceBasedSearchNotAllowedException( + "Preference based routing not allowed with strict weighted shard routing setting" + ); + } if (preference.charAt(0) == '_') { Preference preferenceType = Preference.parse(preference); if (preferenceType == Preference.SHARDS) { diff --git a/server/src/main/java/org/opensearch/cluster/routing/PreferenceBasedSearchNotAllowedException.java b/server/src/main/java/org/opensearch/cluster/routing/PreferenceBasedSearchNotAllowedException.java new file mode 100644 index 0000000000000..2cdaeee7e90f2 --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/PreferenceBasedSearchNotAllowedException.java @@ -0,0 +1,37 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + */ + +package org.opensearch.cluster.routing; + +import org.opensearch.OpenSearchException; +import org.opensearch.common.io.stream.StreamInput; +import org.opensearch.rest.RestStatus; + +import java.io.IOException; + +/** + * Thrown to disallow preference based search with strict weighted shard routing. See {@link WeightedRoutingService} + * * for more details. + * + * @opensearch.internal + */ +public class PreferenceBasedSearchNotAllowedException extends OpenSearchException { + + public PreferenceBasedSearchNotAllowedException(StreamInput in) throws IOException { + super(in); + } + + public PreferenceBasedSearchNotAllowedException(String msg, Object... args) { + super(msg, args); + } + + @Override + public RestStatus status() { + return RestStatus.FORBIDDEN; + } +} diff --git a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java index 55e88d2d09d8c..5549b4e3f26b7 100644 --- a/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/ClusterSettings.java @@ -541,6 +541,7 @@ public void apply(Settings value, Settings current, Settings previous) { OperationRouting.IGNORE_AWARENESS_ATTRIBUTES_SETTING, OperationRouting.WEIGHTED_ROUTING_DEFAULT_WEIGHT, OperationRouting.WEIGHTED_ROUTING_FAILOPEN_ENABLED, + OperationRouting.STRICT_WEIGHTED_SHARD_ROUTING_ENABLED, IndexGraveyard.SETTING_MAX_TOMBSTONES, PersistentTasksClusterService.CLUSTER_TASKS_ALLOCATION_RECHECK_INTERVAL_SETTING, EnableAssignmentDecider.CLUSTER_TASKS_ALLOCATION_ENABLE_SETTING, From 8ec7183740f7a46d47076525d37d5c67a3781db9 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Mon, 16 Jan 2023 20:49:22 +0530 Subject: [PATCH 2/3] fix exception test failure Signed-off-by: Anshu Agarwal --- CHANGELOG.md | 3 ++- .../src/main/java/org/opensearch/OpenSearchException.java | 8 ++++++++ .../java/org/opensearch/ExceptionSerializationTests.java | 2 ++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dbc2bfe54510..af54c35a5e640 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Added support for feature flags in opensearch.yml ([#4959](https://github.com/opensearch-project/OpenSearch/pull/4959)) - Add query for initialized extensions ([#5658](https://github.com/opensearch-project/OpenSearch/pull/5658)) - Revert 'Added jackson dependency to server' and change extension reading ([#5768](https://github.com/opensearch-project/OpenSearch/pull/5768)) +- Add support to disallow search request with preference parameter with strict weighted shard routing([#5874](https://github.com/opensearch-project/OpenSearch/pull/5874)) ### Dependencies - Bumps `log4j-core` from 2.18.0 to 2.19.0 @@ -120,4 +121,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Security [Unreleased 3.0]: https://github.com/opensearch-project/OpenSearch/compare/2.4...HEAD -[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.4...2.x \ No newline at end of file +[Unreleased 2.x]: https://github.com/opensearch-project/OpenSearch/compare/2.4...2.x diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java index 91967dc290433..4fa20f74eb044 100644 --- a/server/src/main/java/org/opensearch/OpenSearchException.java +++ b/server/src/main/java/org/opensearch/OpenSearchException.java @@ -34,6 +34,7 @@ import org.opensearch.action.support.replication.ReplicationOperation; import org.opensearch.cluster.action.shard.ShardStateAction; +import org.opensearch.cluster.routing.PreferenceBasedSearchNotAllowedException; import org.opensearch.cluster.routing.UnsupportedWeightedRoutingStateException; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.common.CheckedFunction; @@ -72,6 +73,7 @@ import static org.opensearch.Version.V_2_1_0; import static org.opensearch.Version.V_2_4_0; import static org.opensearch.Version.V_2_5_0; +import static org.opensearch.Version.V_2_6_0; import static org.opensearch.Version.V_3_0_0; import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_UUID_NA_VALUE; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -1632,6 +1634,12 @@ private enum OpenSearchExceptionHandle { 167, V_2_5_0 ), + PREFERENCE_BASED_SEARCH_NOT_ALLOWED_EXCEPTION( + PreferenceBasedSearchNotAllowedException.class, + PreferenceBasedSearchNotAllowedException::new, + 168, + V_2_6_0 + ), INDEX_CREATE_BLOCK_EXCEPTION( org.opensearch.cluster.block.IndexCreateBlockException.class, org.opensearch.cluster.block.IndexCreateBlockException::new, diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java index aaf82fa42406d..db1aeb3eeb614 100644 --- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java @@ -54,6 +54,7 @@ import org.opensearch.cluster.decommission.NodeDecommissionedException; import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.IllegalShardRoutingStateException; +import org.opensearch.cluster.routing.PreferenceBasedSearchNotAllowedException; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.routing.TestShardRouting; @@ -867,6 +868,7 @@ public void testIds() { ids.put(165, ClusterManagerThrottlingException.class); ids.put(166, SnapshotInUseDeletionException.class); ids.put(167, UnsupportedWeightedRoutingStateException.class); + ids.put(168, PreferenceBasedSearchNotAllowedException.class); ids.put(10001, IndexCreateBlockException.class); Map, Integer> reverse = new HashMap<>(); From fb5e329610b14f83d881cc876a237ca9590bca61 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Tue, 17 Jan 2023 12:05:22 +0530 Subject: [PATCH 3/3] Modify integ test to select at random from multiple preferences Signed-off-by: Anshu Agarwal --- .../opensearch/search/SearchWeightedRoutingIT.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java b/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java index 0d5e256ef1596..4c32929a85373 100644 --- a/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/search/SearchWeightedRoutingIT.java @@ -821,10 +821,16 @@ public void testStrictWeightedRouting() throws Exception { logger.info("--> setting shard routing weights for weighted round robin"); Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); setShardRoutingWeights(weights); + String nodeInZoneA = nodeMap.get("a").get(0); + String customPreference = randomAlphaOfLength(10); assertThrows( PreferenceBasedSearchNotAllowedException.class, - () -> internalCluster().client(nodeMap.get("b").get(0)).prepareSearch().setSize(0).setPreference("_only_local").get() + () -> internalCluster().client(nodeMap.get("b").get(0)) + .prepareSearch() + .setSize(0) + .setPreference(randomFrom("_local", "_only_nodes:" + nodeInZoneA, "_prefer_nodes:" + nodeInZoneA, customPreference)) + .get() ); } @@ -852,10 +858,13 @@ public void testPreferenceSearchWithWeightedRouting() throws Exception { Map weights = Map.of("a", 1.0, "b", 1.0, "c", 0.0); setShardRoutingWeights(weights); + String customPreference = randomAlphaOfLength(10); + String nodeInZoneA = nodeMap.get("a").get(0); + SearchResponse searchResponse = internalCluster().client(nodeMap.get("b").get(0)) .prepareSearch() .setSize(0) - .setPreference("_only_local") + .setPreference(randomFrom("_local", "_only_nodes:" + nodeInZoneA, "_prefer_nodes:" + nodeInZoneA, customPreference)) .get(); assertEquals(RestStatus.OK.getStatus(), searchResponse.status().getStatus()); }