From 976f7da496fa27befa1a04db6657e4fb21c53b56 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Thu, 27 Oct 2022 15:49:12 +0530 Subject: [PATCH 01/26] Allow weight updates for non decommissioned attribute Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingService.java | 13 +++++--- .../routing/WeightedRoutingServiceTests.java | 32 +++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 6acb4a1e832cb..67e7557316be6 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -19,6 +19,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.ack.ClusterStateUpdateResponse; +import org.opensearch.cluster.decommission.DecommissionAttribute; import org.opensearch.cluster.decommission.DecommissionAttributeMetadata; import org.opensearch.cluster.decommission.DecommissionStatus; import org.opensearch.cluster.metadata.Metadata; @@ -34,6 +35,7 @@ import java.util.List; import java.util.Locale; +import java.util.Objects; import static org.opensearch.action.ValidateActions.addValidationError; @@ -70,8 +72,8 @@ public void registerWeightedRoutingMetadata( clusterService.submitStateUpdateTask("update_weighted_routing", new ClusterStateUpdateTask(Priority.URGENT) { @Override public ClusterState execute(ClusterState currentState) { - // verify currently no decommission action is ongoing - ensureNoOngoingDecommissionAction(currentState); + // verify weights will not be updated for a decommissioned attribute + ensureNoWeightUpdateForDecommissionedAttribute(currentState, request); Metadata metadata = currentState.metadata(); Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata()); WeightedRoutingMetadata weightedRoutingMetadata = metadata.custom(WeightedRoutingMetadata.TYPE); @@ -159,9 +161,12 @@ public void verifyAwarenessAttribute(String attributeName) { } } - public void ensureNoOngoingDecommissionAction(ClusterState state) { + public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, ClusterPutWeightedRoutingRequest request) { DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); - if (decommissionAttributeMetadata != null && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false) { + if (decommissionAttributeMetadata != null + && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false + && Objects.equals(request.getWeightedRouting().attributeName(), decommissionAttributeMetadata.decommissionAttribute().attributeName()) + && Objects.equals(request.getWeightedRouting().weights().get(decommissionAttributeMetadata.decommissionAttribute().attributeValue()), 0.0) == false) { throw new IllegalStateException( "a decommission action is ongoing with status [" + decommissionAttributeMetadata.status().status() diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 91b8703cacf5c..9c17df07f84a8 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -362,4 +362,36 @@ public void onFailure(Exception e) {} weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); } + + public void testAddWeightedRoutingPassesWhenWeightOfDecommissionedAttributeStillZero() throws InterruptedException { + Map weights = Map.of("zone_A", 0.0, "zone_B", 1.0, "zone_C", 1.0); + DecommissionStatus status = DecommissionStatus.SUCCESSFUL; + ClusterState state = clusterService.state(); + state = setWeightedRoutingWeights(state, weights); + state = setDecommissionAttribute(state, status); + ClusterState.Builder builder = ClusterState.builder(state); + ClusterServiceUtils.setState(clusterService, builder); + + ClusterPutWeightedRoutingRequestBuilder request = new ClusterPutWeightedRoutingRequestBuilder( + client, + ClusterAddWeightedRoutingAction.INSTANCE + ); + Map updatedWeights = Map.of("zone_A", 0.0, "zone_B", 2.0, "zone_C", 1.0); + WeightedRouting updatedWeightedRouting = new WeightedRouting("zone", updatedWeights); + request.setWeightedRouting(updatedWeightedRouting); + final CountDownLatch countDownLatch = new CountDownLatch(1); + ActionListener listener = new ActionListener() { + @Override + public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) { + assertTrue(clusterStateUpdateResponse.isAcknowledged()); + countDownLatch.countDown(); + } + + @Override + public void onFailure(Exception e) {} + }; + weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); + assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); + assertEquals(updatedWeightedRouting, clusterService.state().metadata().weightedRoutingMetadata().getWeightedRouting()); + } } From 34dbe801c322e80100d8ede8e2ba2db5c9223c81 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Thu, 27 Oct 2022 15:51:10 +0530 Subject: [PATCH 02/26] Fix spotless check Signed-off-by: Rishab Nahata --- .../cluster/routing/WeightedRoutingService.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 67e7557316be6..190f45084bd5d 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -19,7 +19,6 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.ack.ClusterStateUpdateResponse; -import org.opensearch.cluster.decommission.DecommissionAttribute; import org.opensearch.cluster.decommission.DecommissionAttributeMetadata; import org.opensearch.cluster.decommission.DecommissionStatus; import org.opensearch.cluster.metadata.Metadata; @@ -165,8 +164,14 @@ public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, C DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); if (decommissionAttributeMetadata != null && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false - && Objects.equals(request.getWeightedRouting().attributeName(), decommissionAttributeMetadata.decommissionAttribute().attributeName()) - && Objects.equals(request.getWeightedRouting().weights().get(decommissionAttributeMetadata.decommissionAttribute().attributeValue()), 0.0) == false) { + && Objects.equals( + request.getWeightedRouting().attributeName(), + decommissionAttributeMetadata.decommissionAttribute().attributeName() + ) + && Objects.equals( + request.getWeightedRouting().weights().get(decommissionAttributeMetadata.decommissionAttribute().attributeValue()), + 0.0 + ) == false) { throw new IllegalStateException( "a decommission action is ongoing with status [" + decommissionAttributeMetadata.status().status() From 16f6c74873e7d37f43f7938687db9c73da627ce2 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Thu, 27 Oct 2022 15:57:52 +0530 Subject: [PATCH 03/26] Add changelog Signed-off-by: Rishab Nahata --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c60d79679a9b0..c7bafbe2025ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Migrate client transports to Apache HttpClient / Core 5.x ([#4459](https://github.com/opensearch-project/OpenSearch/pull/4459)) - Refactored BalancedAllocator.Balancer to LocalShardsBalancer ([#4761](https://github.com/opensearch-project/OpenSearch/pull/4761)) - Fail weight update when decommission ongoing and fail decommission when attribute not weighed away ([#4839](https://github.com/opensearch-project/OpenSearch/pull/4839)) +- Allow weight updates for non decommissioned attribute([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) ### Deprecated ### Removed - Remove deprecated code to add node name into log pattern of log4j property file ([#4568](https://github.com/opensearch-project/OpenSearch/pull/4568)) From 836b4ceb5d79a67a704699929bb1c7d4b09c643f Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 14:21:10 +0530 Subject: [PATCH 04/26] Refactor Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingService.java | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 190f45084bd5d..0ad3be4960778 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -19,6 +19,7 @@ import org.opensearch.cluster.ClusterState; import org.opensearch.cluster.ClusterStateUpdateTask; import org.opensearch.cluster.ack.ClusterStateUpdateResponse; +import org.opensearch.cluster.decommission.DecommissionAttribute; import org.opensearch.cluster.decommission.DecommissionAttributeMetadata; import org.opensearch.cluster.decommission.DecommissionStatus; import org.opensearch.cluster.metadata.Metadata; @@ -162,21 +163,27 @@ public void verifyAwarenessAttribute(String attributeName) { public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, ClusterPutWeightedRoutingRequest request) { DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); - if (decommissionAttributeMetadata != null - && decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED) == false - && Objects.equals( - request.getWeightedRouting().attributeName(), - decommissionAttributeMetadata.decommissionAttribute().attributeName() - ) - && Objects.equals( - request.getWeightedRouting().weights().get(decommissionAttributeMetadata.decommissionAttribute().attributeValue()), - 0.0 - ) == false) { - throw new IllegalStateException( - "a decommission action is ongoing with status [" - + decommissionAttributeMetadata.status().status() - + "], cannot update weight during this state" + if (decommissionAttributeMetadata == null || !decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED)) { + // here either there's no decommission action is ongoing or it is in failed state. In this case, we will allow weight update + return; + } + DecommissionAttribute decommissionAttribute = decommissionAttributeMetadata.decommissionAttribute(); + WeightedRouting weightedRouting = request.getWeightedRouting(); + if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName()) == false) { + // this is unexpected when a different attribute is requested for decommission and weight update is on another attribute + throw new IllegalStateException("decommission action ongoing for attribute [" + + decommissionAttribute.attributeName() + + "], cannot update weight for [" + + weightedRouting.attributeName() + + "]" ); } + if (weightedRouting.weights().containsKey(decommissionAttribute.attributeValue()) == false) { + // weight of an attribute undergoing decommission must be specified + throw new IllegalStateException("weight for [" + decommissionAttribute.attributeValue() + "] is not specified. Please specify its weight as zero as it is under decommission action"); + } + if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { + throw new IllegalStateException("weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0.0]"); + } } } From f2d442b6279c5bc47679ea8b987c73b7e56f1cd6 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 14:22:45 +0530 Subject: [PATCH 05/26] Fix spotless check Signed-off-by: Rishab Nahata --- .../cluster/routing/WeightedRoutingService.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 0ad3be4960778..582fc5b7e6e3b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -171,16 +171,21 @@ public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, C WeightedRouting weightedRouting = request.getWeightedRouting(); if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName()) == false) { // this is unexpected when a different attribute is requested for decommission and weight update is on another attribute - throw new IllegalStateException("decommission action ongoing for attribute [" - + decommissionAttribute.attributeName() - + "], cannot update weight for [" - + weightedRouting.attributeName() - + "]" + throw new IllegalStateException( + "decommission action ongoing for attribute [" + + decommissionAttribute.attributeName() + + "], cannot update weight for [" + + weightedRouting.attributeName() + + "]" ); } if (weightedRouting.weights().containsKey(decommissionAttribute.attributeValue()) == false) { // weight of an attribute undergoing decommission must be specified - throw new IllegalStateException("weight for [" + decommissionAttribute.attributeValue() + "] is not specified. Please specify its weight as zero as it is under decommission action"); + throw new IllegalStateException( + "weight for [" + + decommissionAttribute.attributeValue() + + "] is not specified. Please specify its weight as zero as it is under decommission action" + ); } if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { throw new IllegalStateException("weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0.0]"); From 38dcbf27edc145ac7f0b32d7278c97d867c2d186 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 14:41:42 +0530 Subject: [PATCH 06/26] Changes Signed-off-by: Rishab Nahata --- .../cluster/routing/WeightedRoutingService.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 582fc5b7e6e3b..79d7198b387d4 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -163,7 +163,7 @@ public void verifyAwarenessAttribute(String attributeName) { public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, ClusterPutWeightedRoutingRequest request) { DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); - if (decommissionAttributeMetadata == null || !decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED)) { + if (decommissionAttributeMetadata == null || decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED)) { // here either there's no decommission action is ongoing or it is in failed state. In this case, we will allow weight update return; } @@ -184,11 +184,13 @@ public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, C throw new IllegalStateException( "weight for [" + decommissionAttribute.attributeValue() - + "] is not specified. Please specify its weight as zero as it is under decommission action" + + "] is not specified. Please specify its weight to [0] as it is under decommission action" ); } if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { - throw new IllegalStateException("weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0.0]"); + throw new IllegalStateException( + "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0.0] as it is under decommission action" + ); } } } From 29cbcad190a2e92b0ab6df421ff12c837c00d6ec Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 14:43:21 +0530 Subject: [PATCH 07/26] Add space Signed-off-by: Rishab Nahata --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04712645303df..1fc65916bb044 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Support remote translog transfer for request level durability([#4480](https://github.com/opensearch-project/OpenSearch/pull/4480)) - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) -- Allow weight updates for non decommissioned attribute([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) +- Allow weight updates for non decommissioned attribute ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) + ### Deprecated ### Removed From 8364af3c1d56e9c14dd11fe4bc35748a108f7684 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 15:03:13 +0530 Subject: [PATCH 08/26] Refactor Signed-off-by: Rishab Nahata --- .../opensearch/cluster/routing/WeightedRoutingService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 79d7198b387d4..ff5f3be769870 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -73,7 +73,7 @@ public void registerWeightedRoutingMetadata( @Override public ClusterState execute(ClusterState currentState) { // verify weights will not be updated for a decommissioned attribute - ensureNoWeightUpdateForDecommissionedAttribute(currentState, request); + ensureDecommissionedAttributeHasZeroWeight(currentState, request); Metadata metadata = currentState.metadata(); Metadata.Builder mdBuilder = Metadata.builder(currentState.metadata()); WeightedRoutingMetadata weightedRoutingMetadata = metadata.custom(WeightedRoutingMetadata.TYPE); @@ -161,7 +161,7 @@ public void verifyAwarenessAttribute(String attributeName) { } } - public void ensureNoWeightUpdateForDecommissionedAttribute(ClusterState state, ClusterPutWeightedRoutingRequest request) { + public void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, ClusterPutWeightedRoutingRequest request) { DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); if (decommissionAttributeMetadata == null || decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED)) { // here either there's no decommission action is ongoing or it is in failed state. In this case, we will allow weight update From f5e1b946eee6bec80c7fd374548db6cb3bb502e9 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 15:06:23 +0530 Subject: [PATCH 09/26] Update Signed-off-by: Rishab Nahata --- .../org/opensearch/cluster/routing/WeightedRoutingService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index ff5f3be769870..df7b1ec88024b 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -189,7 +189,7 @@ public void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clust } if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { throw new IllegalStateException( - "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0.0] as it is under decommission action" + "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0] as it is under decommission action" ); } } From dfb1ad77577625c0ad67436f475b0c817bbcbb9a Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 15:43:16 +0530 Subject: [PATCH 10/26] Update test msg Signed-off-by: Rishab Nahata --- .../opensearch/cluster/routing/WeightedRoutingServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 9c17df07f84a8..8cba0153ba814 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -328,7 +328,7 @@ public void onFailure(Exception e) { assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); MatcherAssert.assertThat(exceptionReference.get(), instanceOf(IllegalStateException.class)); - MatcherAssert.assertThat(exceptionReference.get().getMessage(), containsString("a decommission action is ongoing with status")); + MatcherAssert.assertThat(exceptionReference.get().getMessage(), containsString("weight for [zone_A] must be set to [0] as it is under decommission action")); } public void testAddWeightedRoutingPassesWhenDecommissionFailed() throws InterruptedException { From e817d9091c8c366b8f4f5b852fd3da2b87122295 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Fri, 2 Dec 2022 15:57:34 +0530 Subject: [PATCH 11/26] Fix spotless check Signed-off-by: Rishab Nahata --- .../cluster/routing/WeightedRoutingServiceTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 8cba0153ba814..37d4b8d0b0333 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -328,7 +328,10 @@ public void onFailure(Exception e) { assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); MatcherAssert.assertThat(exceptionReference.get(), instanceOf(IllegalStateException.class)); - MatcherAssert.assertThat(exceptionReference.get().getMessage(), containsString("weight for [zone_A] must be set to [0] as it is under decommission action")); + MatcherAssert.assertThat( + exceptionReference.get().getMessage(), + containsString("weight for [zone_A] must be set to [0] as it is under decommission action") + ); } public void testAddWeightedRoutingPassesWhenDecommissionFailed() throws InterruptedException { From 2b06ea840d7b4fcda20092884b597cf550b59149 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 13:49:30 +0530 Subject: [PATCH 12/26] Make rest status 409 when failed to update weight Signed-off-by: Rishab Nahata --- .../org/opensearch/OpenSearchException.java | 8 +++++ .../routing/WeightedRoutingService.java | 6 ++-- ...ghtedRoutingUnsupportedStateException.java | 34 +++++++++++++++++++ .../ExceptionSerializationTests.java | 2 ++ 4 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java index aef098403ec2b..fc10733a8ecd9 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.WeightedRoutingUnsupportedStateException; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.common.CheckedFunction; import org.opensearch.common.Nullable; @@ -70,6 +71,7 @@ import static java.util.Collections.unmodifiableMap; 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_3_0_0; import static org.opensearch.cluster.metadata.IndexMetadata.INDEX_UUID_NA_VALUE; import static org.opensearch.common.xcontent.XContentParserUtils.ensureExpectedToken; @@ -1618,6 +1620,12 @@ private enum OpenSearchExceptionHandle { SnapshotInUseDeletionException::new, 166, UNKNOWN_VERSION_ADDED + ), + WEIGHTED_ROUTING_UNSUPPORTED_STATE_EXCEPTION( + WeightedRoutingUnsupportedStateException.class, + WeightedRoutingUnsupportedStateException::new, + 167, + V_2_5_0 ); final Class exceptionClass; diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index df7b1ec88024b..89dac48d80f46 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -171,7 +171,7 @@ public void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clust WeightedRouting weightedRouting = request.getWeightedRouting(); if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName()) == false) { // this is unexpected when a different attribute is requested for decommission and weight update is on another attribute - throw new IllegalStateException( + throw new WeightedRoutingUnsupportedStateException( "decommission action ongoing for attribute [" + decommissionAttribute.attributeName() + "], cannot update weight for [" @@ -181,14 +181,14 @@ public void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clust } if (weightedRouting.weights().containsKey(decommissionAttribute.attributeValue()) == false) { // weight of an attribute undergoing decommission must be specified - throw new IllegalStateException( + throw new WeightedRoutingUnsupportedStateException( "weight for [" + decommissionAttribute.attributeValue() + "] is not specified. Please specify its weight to [0] as it is under decommission action" ); } if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { - throw new IllegalStateException( + throw new WeightedRoutingUnsupportedStateException( "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0] as it is under decommission action" ); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java new file mode 100644 index 0000000000000..dfffbc8430fbd --- /dev/null +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java @@ -0,0 +1,34 @@ +/* + * 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 when failing to update the routing weight due to an unsupported state. See {@link WeightedRoutingService} for more details. + * + * @opensearch.internal + */ +public class WeightedRoutingUnsupportedStateException extends OpenSearchException { + public WeightedRoutingUnsupportedStateException(StreamInput in) throws IOException { + super(in); + } + public WeightedRoutingUnsupportedStateException(String msg, Object... args) { + super(msg, args); + } + + @Override + public RestStatus status() { + return RestStatus.CONFLICT; + } +} diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java index 559963b0e0b68..96c984fedc21c 100644 --- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java @@ -56,6 +56,7 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.routing.TestShardRouting; +import org.opensearch.cluster.routing.WeightedRoutingUnsupportedStateException; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.common.ParsingException; import org.opensearch.common.Strings; @@ -864,6 +865,7 @@ public void testIds() { ids.put(164, NodeDecommissionedException.class); ids.put(165, ClusterManagerThrottlingException.class); ids.put(166, SnapshotInUseDeletionException.class); + ids.put(167, WeightedRoutingUnsupportedStateException.class); Map, Integer> reverse = new HashMap<>(); for (Map.Entry> entry : ids.entrySet()) { From b93b96bb0b08d11eaf5ea664eb1e1a32ecd64d2b Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 13:54:59 +0530 Subject: [PATCH 13/26] Fix spotless check Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingUnsupportedStateException.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java index dfffbc8430fbd..252512b29126a 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java @@ -23,6 +23,7 @@ public class WeightedRoutingUnsupportedStateException extends OpenSearchExceptio public WeightedRoutingUnsupportedStateException(StreamInput in) throws IOException { super(in); } + public WeightedRoutingUnsupportedStateException(String msg, Object... args) { super(msg, args); } From 5793ad6179fd5ff9dab6b6e858bf8e582f27cc6c Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 14:56:19 +0530 Subject: [PATCH 14/26] Ensure weights for all discovered zones Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingService.java | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 89dac48d80f46..ec79fb871b680 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -33,11 +33,17 @@ import org.opensearch.common.settings.Settings; import org.opensearch.threadpool.ThreadPool; +import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import static org.opensearch.action.ValidateActions.addValidationError; +import static org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING; /** * * Service responsible for updating cluster state metadata with weighted routing weights @@ -47,6 +53,7 @@ public class WeightedRoutingService { private final ClusterService clusterService; private final ThreadPool threadPool; private volatile List awarenessAttributes; + private volatile Map> forcedAwarenessAttributes; @Inject public WeightedRoutingService( @@ -62,6 +69,11 @@ public WeightedRoutingService( AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING, this::setAwarenessAttributes ); + setForcedAwarenessAttributes(CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING.get(settings)); + clusterSettings.addSettingsUpdateConsumer( + CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING, + this::setForcedAwarenessAttributes + ); } public void registerWeightedRoutingMetadata( @@ -72,6 +84,8 @@ public void registerWeightedRoutingMetadata( clusterService.submitStateUpdateTask("update_weighted_routing", new ClusterStateUpdateTask(Priority.URGENT) { @Override public ClusterState execute(ClusterState currentState) { + // verify that request object has weights for all discovered and forced awareness values + ensureWeightsSetForAllDiscoveredAndForcedAwarenessValues(currentState, request); // verify weights will not be updated for a decommissioned attribute ensureDecommissionedAttributeHasZeroWeight(currentState, request); Metadata metadata = currentState.metadata(); @@ -150,6 +164,18 @@ private void setAwarenessAttributes(List awarenessAttributes) { this.awarenessAttributes = awarenessAttributes; } + private void setForcedAwarenessAttributes(Settings forceSettings) { + Map> forcedAwarenessAttributes = new HashMap<>(); + Map forceGroups = forceSettings.getAsGroups(); + for (Map.Entry entry : forceGroups.entrySet()) { + List aValues = entry.getValue().getAsList("values"); + if (aValues.size() > 0) { + forcedAwarenessAttributes.put(entry.getKey(), aValues); + } + } + this.forcedAwarenessAttributes = forcedAwarenessAttributes; + } + public void verifyAwarenessAttribute(String attributeName) { if (getAwarenessAttributes().contains(attributeName) == false) { ActionRequestValidationException validationException = null; @@ -161,7 +187,28 @@ public void verifyAwarenessAttribute(String attributeName) { } } - public void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, ClusterPutWeightedRoutingRequest request) { + private void ensureWeightsSetForAllDiscoveredAndForcedAwarenessValues(ClusterState state, ClusterPutWeightedRoutingRequest request) { + String attributeName = request.getWeightedRouting().attributeName(); + Set discoveredAwarenessValues = new HashSet<>(); + state.nodes().forEach(node -> { + if (node.getAttributes().containsKey(attributeName)) { + discoveredAwarenessValues.add(node.getAttributes().get(attributeName)); + } + }); + Set allAwarenessValues = new HashSet<>(forcedAwarenessAttributes.get(attributeName)); + allAwarenessValues.addAll(discoveredAwarenessValues); + allAwarenessValues.forEach(awarenessValue -> { + if (request.getWeightedRouting().weights().containsKey(awarenessValue) == false) { + throw new WeightedRoutingUnsupportedStateException( + "weight for [" + + awarenessValue + + "] is not set and it is part of forced awareness value or a node has this attribute." + ); + } + }); + } + + private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, ClusterPutWeightedRoutingRequest request) { DecommissionAttributeMetadata decommissionAttributeMetadata = state.metadata().decommissionAttributeMetadata(); if (decommissionAttributeMetadata == null || decommissionAttributeMetadata.status().equals(DecommissionStatus.FAILED)) { // here either there's no decommission action is ongoing or it is in failed state. In this case, we will allow weight update From c8e222ad0ae7eac2e0bd0ee34a6820757233eb13 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 15:14:29 +0530 Subject: [PATCH 15/26] Bug fix Signed-off-by: Rishab Nahata --- .../cluster/routing/WeightedRoutingService.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index ec79fb871b680..5f51814ee5635 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -40,7 +40,6 @@ import java.util.Map; import java.util.Objects; import java.util.Set; -import java.util.stream.Collectors; import static org.opensearch.action.ValidateActions.addValidationError; import static org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_FORCE_GROUP_SETTING; @@ -195,14 +194,17 @@ private void ensureWeightsSetForAllDiscoveredAndForcedAwarenessValues(ClusterSta discoveredAwarenessValues.add(node.getAttributes().get(attributeName)); } }); - Set allAwarenessValues = new HashSet<>(forcedAwarenessAttributes.get(attributeName)); + Set allAwarenessValues; + if (forcedAwarenessAttributes.get(attributeName) == null) { + allAwarenessValues = new HashSet<>(); + } else { + allAwarenessValues = new HashSet<>(forcedAwarenessAttributes.get(attributeName)); + } allAwarenessValues.addAll(discoveredAwarenessValues); allAwarenessValues.forEach(awarenessValue -> { if (request.getWeightedRouting().weights().containsKey(awarenessValue) == false) { throw new WeightedRoutingUnsupportedStateException( - "weight for [" - + awarenessValue - + "] is not set and it is part of forced awareness value or a node has this attribute." + "weight for [" + awarenessValue + "] is not set and it is part of forced awareness value or a node has this attribute." ); } }); From 05b208463fe07ce29ed0f4d4a811b662a6f78c28 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 15:21:27 +0530 Subject: [PATCH 16/26] Relax zero weight count check Signed-off-by: Rishab Nahata --- .../put/ClusterPutWeightedRoutingRequest.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java index af229fb12b4f0..3e64fc6152304 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java @@ -127,26 +127,17 @@ public ActionRequestValidationException validate() { if (weightedRouting.weights() == null || weightedRouting.weights().isEmpty()) { validationException = addValidationError("Weights are missing", validationException); } - int countValueWithZeroWeights = 0; - double weight; try { for (Object value : weightedRouting.weights().values()) { if (value == null) { validationException = addValidationError(("Weight is null"), validationException); } else { - weight = Double.parseDouble(value.toString()); - countValueWithZeroWeights = (weight == 0) ? countValueWithZeroWeights + 1 : countValueWithZeroWeights; + Double.parseDouble(value.toString()); } } } catch (NumberFormatException e) { validationException = addValidationError(("Weight is not a number"), validationException); } - if (countValueWithZeroWeights > 1) { - validationException = addValidationError( - (String.format(Locale.ROOT, "More than one [%d] value has weight set as 0", countValueWithZeroWeights)), - validationException - ); - } return validationException; } From ec1e84d6e774f6f0b9c8b5d98cb2642ac12d82eb Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 15:32:08 +0530 Subject: [PATCH 17/26] Fix test Signed-off-by: Rishab Nahata --- .../weighted/put/ClusterPutWeightedRoutingRequest.java | 1 - .../put/ClusterPutWeightedRoutingRequestTests.java | 9 --------- 2 files changed, 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java index 3e64fc6152304..5474f4effa829 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java @@ -28,7 +28,6 @@ import java.io.IOException; import java.util.HashMap; -import java.util.Locale; import java.util.Map; import static org.opensearch.action.ValidateActions.addValidationError; diff --git a/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java b/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java index 186e7e8638f17..cdec66d6683eb 100644 --- a/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java +++ b/server/src/test/java/org/opensearch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java @@ -35,15 +35,6 @@ public void testValidate_ValuesAreProper() { assertNull(actionRequestValidationException); } - public void testValidate_TwoZonesWithZeroWeight() { - String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"0\",\"us-east-1a\":\"1\"}"; - ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); - request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON); - ActionRequestValidationException actionRequestValidationException = request.validate(); - assertNotNull(actionRequestValidationException); - assertTrue(actionRequestValidationException.getMessage().contains("More than one [2] value has weight set as " + "0")); - } - public void testValidate_MissingWeights() { String reqString = "{}"; ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone"); From 4f0e91b58269a73cb9b73a22acc176011ee32bc1 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 16:10:56 +0530 Subject: [PATCH 18/26] Fix test Signed-off-by: Rishab Nahata --- .../opensearch/cluster/routing/WeightedRoutingServiceTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 37d4b8d0b0333..9f488292c7cf1 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -327,7 +327,7 @@ public void onFailure(Exception e) { weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); - MatcherAssert.assertThat(exceptionReference.get(), instanceOf(IllegalStateException.class)); + MatcherAssert.assertThat(exceptionReference.get(), instanceOf(WeightedRoutingUnsupportedStateException.class)); MatcherAssert.assertThat( exceptionReference.get().getMessage(), containsString("weight for [zone_A] must be set to [0] as it is under decommission action") From 8ea9dee5ddf52f62cf7dbe127a83d053c5d5ab52 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 16:44:46 +0530 Subject: [PATCH 19/26] Update changelog Signed-off-by: Rishab Nahata --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1fc65916bb044..4d4a69ef54c49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Support remote translog transfer for request level durability([#4480](https://github.com/opensearch-project/OpenSearch/pull/4480)) - Changed http code on create index API with bad input raising NotXContentException from 500 to 400 ([#4773](https://github.com/opensearch-project/OpenSearch/pull/4773)) - Change http code for DecommissioningFailedException from 500 to 400 ([#5283](https://github.com/opensearch-project/OpenSearch/pull/5283)) -- Allow weight updates for non decommissioned attribute ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) +- Pre conditions check before updating weighted routing metadata ([#4955](https://github.com/opensearch-project/OpenSearch/pull/4955)) ### Deprecated From 622fe6d70a83b12c1294d2d05019c704e1a7c554 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 5 Dec 2022 16:54:55 +0530 Subject: [PATCH 20/26] Add UT Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingServiceTests.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 9f488292c7cf1..29aa0193da66a 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -295,6 +295,38 @@ public void testVerifyAwarenessAttribute_ValidAttributeName() { } } + public void testAddWeightedRoutingFailsWhenWeightsNotSetForAllDiscoveredZones() throws InterruptedException { + ClusterPutWeightedRoutingRequestBuilder request = new ClusterPutWeightedRoutingRequestBuilder( + client, + ClusterAddWeightedRoutingAction.INSTANCE + ); + Map weights = Map.of("zone_A", 1.0, "zone_C", 1.0); + WeightedRouting weightedRouting = new WeightedRouting("zone", weights); + request.setWeightedRouting(weightedRouting); + final CountDownLatch countDownLatch = new CountDownLatch(1); + final AtomicReference exceptionReference = new AtomicReference<>(); + ActionListener listener = new ActionListener() { + @Override + public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) { + countDownLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + exceptionReference.set(e); + countDownLatch.countDown(); + } + }; + weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); + assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); + MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); + MatcherAssert.assertThat(exceptionReference.get(), instanceOf(WeightedRoutingUnsupportedStateException.class)); + MatcherAssert.assertThat( + exceptionReference.get().getMessage(), + containsString("weight for [zone_B] is not set and it is part of forced awareness value or a node has this attribute.") + ); + } + public void testAddWeightedRoutingFailsWhenDecommissionOngoing() throws InterruptedException { Map weights = Map.of("zone_A", 1.0, "zone_B", 1.0, "zone_C", 1.0); DecommissionStatus status = randomFrom(DecommissionStatus.INIT, DecommissionStatus.IN_PROGRESS, DecommissionStatus.SUCCESSFUL); From 5c6ce4a8748dcac5642173c7e005d6175d7c327a Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Tue, 6 Dec 2022 14:26:42 +0530 Subject: [PATCH 21/26] Refactor Signed-off-by: Rishab Nahata --- .../src/main/java/org/opensearch/OpenSearchException.java | 8 ++++---- ...java => UnsupportedWeightedRoutingStateException.java} | 6 +++--- .../cluster/routing/WeightedRoutingService.java | 8 ++++---- .../java/org/opensearch/ExceptionSerializationTests.java | 4 ++-- .../cluster/routing/WeightedRoutingServiceTests.java | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) rename server/src/main/java/org/opensearch/cluster/routing/{WeightedRoutingUnsupportedStateException.java => UnsupportedWeightedRoutingStateException.java} (80%) diff --git a/server/src/main/java/org/opensearch/OpenSearchException.java b/server/src/main/java/org/opensearch/OpenSearchException.java index fc10733a8ecd9..78f6b50b3a039 100644 --- a/server/src/main/java/org/opensearch/OpenSearchException.java +++ b/server/src/main/java/org/opensearch/OpenSearchException.java @@ -34,7 +34,7 @@ import org.opensearch.action.support.replication.ReplicationOperation; import org.opensearch.cluster.action.shard.ShardStateAction; -import org.opensearch.cluster.routing.WeightedRoutingUnsupportedStateException; +import org.opensearch.cluster.routing.UnsupportedWeightedRoutingStateException; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.common.CheckedFunction; import org.opensearch.common.Nullable; @@ -1621,9 +1621,9 @@ private enum OpenSearchExceptionHandle { 166, UNKNOWN_VERSION_ADDED ), - WEIGHTED_ROUTING_UNSUPPORTED_STATE_EXCEPTION( - WeightedRoutingUnsupportedStateException.class, - WeightedRoutingUnsupportedStateException::new, + UNSUPPORTED_WEIGHTED_ROUTING_STATE_EXCEPTION( + UnsupportedWeightedRoutingStateException.class, + UnsupportedWeightedRoutingStateException::new, 167, V_2_5_0 ); diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java b/server/src/main/java/org/opensearch/cluster/routing/UnsupportedWeightedRoutingStateException.java similarity index 80% rename from server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java rename to server/src/main/java/org/opensearch/cluster/routing/UnsupportedWeightedRoutingStateException.java index 252512b29126a..fd4fd4163ede6 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingUnsupportedStateException.java +++ b/server/src/main/java/org/opensearch/cluster/routing/UnsupportedWeightedRoutingStateException.java @@ -19,12 +19,12 @@ * * @opensearch.internal */ -public class WeightedRoutingUnsupportedStateException extends OpenSearchException { - public WeightedRoutingUnsupportedStateException(StreamInput in) throws IOException { +public class UnsupportedWeightedRoutingStateException extends OpenSearchException { + public UnsupportedWeightedRoutingStateException(StreamInput in) throws IOException { super(in); } - public WeightedRoutingUnsupportedStateException(String msg, Object... args) { + public UnsupportedWeightedRoutingStateException(String msg, Object... args) { super(msg, args); } diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 5f51814ee5635..93b94a3805488 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -203,7 +203,7 @@ private void ensureWeightsSetForAllDiscoveredAndForcedAwarenessValues(ClusterSta allAwarenessValues.addAll(discoveredAwarenessValues); allAwarenessValues.forEach(awarenessValue -> { if (request.getWeightedRouting().weights().containsKey(awarenessValue) == false) { - throw new WeightedRoutingUnsupportedStateException( + throw new UnsupportedWeightedRoutingStateException( "weight for [" + awarenessValue + "] is not set and it is part of forced awareness value or a node has this attribute." ); } @@ -220,7 +220,7 @@ private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clus WeightedRouting weightedRouting = request.getWeightedRouting(); if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName()) == false) { // this is unexpected when a different attribute is requested for decommission and weight update is on another attribute - throw new WeightedRoutingUnsupportedStateException( + throw new UnsupportedWeightedRoutingStateException( "decommission action ongoing for attribute [" + decommissionAttribute.attributeName() + "], cannot update weight for [" @@ -230,14 +230,14 @@ private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clus } if (weightedRouting.weights().containsKey(decommissionAttribute.attributeValue()) == false) { // weight of an attribute undergoing decommission must be specified - throw new WeightedRoutingUnsupportedStateException( + throw new UnsupportedWeightedRoutingStateException( "weight for [" + decommissionAttribute.attributeValue() + "] is not specified. Please specify its weight to [0] as it is under decommission action" ); } if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { - throw new WeightedRoutingUnsupportedStateException( + throw new UnsupportedWeightedRoutingStateException( "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0] as it is under decommission action" ); } diff --git a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java index 96c984fedc21c..a601d20af5a3f 100644 --- a/server/src/test/java/org/opensearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/opensearch/ExceptionSerializationTests.java @@ -56,7 +56,7 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.routing.ShardRoutingState; import org.opensearch.cluster.routing.TestShardRouting; -import org.opensearch.cluster.routing.WeightedRoutingUnsupportedStateException; +import org.opensearch.cluster.routing.UnsupportedWeightedRoutingStateException; import org.opensearch.cluster.service.ClusterManagerThrottlingException; import org.opensearch.common.ParsingException; import org.opensearch.common.Strings; @@ -865,7 +865,7 @@ public void testIds() { ids.put(164, NodeDecommissionedException.class); ids.put(165, ClusterManagerThrottlingException.class); ids.put(166, SnapshotInUseDeletionException.class); - ids.put(167, WeightedRoutingUnsupportedStateException.class); + ids.put(167, UnsupportedWeightedRoutingStateException.class); Map, Integer> reverse = new HashMap<>(); for (Map.Entry> entry : ids.entrySet()) { diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 29aa0193da66a..03638d21466dd 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -320,7 +320,7 @@ public void onFailure(Exception e) { weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); - MatcherAssert.assertThat(exceptionReference.get(), instanceOf(WeightedRoutingUnsupportedStateException.class)); + MatcherAssert.assertThat(exceptionReference.get(), instanceOf(UnsupportedWeightedRoutingStateException.class)); MatcherAssert.assertThat( exceptionReference.get().getMessage(), containsString("weight for [zone_B] is not set and it is part of forced awareness value or a node has this attribute.") @@ -359,7 +359,7 @@ public void onFailure(Exception e) { weightedRoutingService.registerWeightedRoutingMetadata(request.request(), listener); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); MatcherAssert.assertThat("Expected onFailure to be called", exceptionReference.get(), notNullValue()); - MatcherAssert.assertThat(exceptionReference.get(), instanceOf(WeightedRoutingUnsupportedStateException.class)); + MatcherAssert.assertThat(exceptionReference.get(), instanceOf(UnsupportedWeightedRoutingStateException.class)); MatcherAssert.assertThat( exceptionReference.get().getMessage(), containsString("weight for [zone_A] must be set to [0] as it is under decommission action") From 1539c25feadd6766906dafeb0e987263dd770e38 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Tue, 6 Dec 2022 14:51:15 +0530 Subject: [PATCH 22/26] Add constant Signed-off-by: Rishab Nahata --- .../opensearch/cluster/routing/WeightedRoutingService.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index 93b94a3805488..ef17f13c10a07 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -53,6 +53,7 @@ public class WeightedRoutingService { private final ThreadPool threadPool; private volatile List awarenessAttributes; private volatile Map> forcedAwarenessAttributes; + private static final Double DECOMMISSIONED_AWARENESS_VALUE_WEIGHT = 0.0; @Inject public WeightedRoutingService( @@ -236,7 +237,10 @@ private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clus + "] is not specified. Please specify its weight to [0] as it is under decommission action" ); } - if (Objects.equals(weightedRouting.weights().get(decommissionAttribute.attributeValue()), 0.0) == false) { + if (Objects.equals( + weightedRouting.weights().get(decommissionAttribute.attributeValue()), + DECOMMISSIONED_AWARENESS_VALUE_WEIGHT + ) == false) { throw new UnsupportedWeightedRoutingStateException( "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0] as it is under decommission action" ); From c0ba7a53ffa8ea251a304e3d8285c389dc590d42 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Wed, 7 Dec 2022 11:34:27 +0530 Subject: [PATCH 23/26] Refactor log msg Signed-off-by: Rishab Nahata --- .../routing/WeightedRoutingService.java | 18 +++++++++--------- .../routing/WeightedRoutingServiceTests.java | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java index ef17f13c10a07..2b5961c7340c1 100644 --- a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java +++ b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java @@ -222,19 +222,17 @@ private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clus if (weightedRouting.attributeName().equals(decommissionAttribute.attributeName()) == false) { // this is unexpected when a different attribute is requested for decommission and weight update is on another attribute throw new UnsupportedWeightedRoutingStateException( - "decommission action ongoing for attribute [" - + decommissionAttribute.attributeName() - + "], cannot update weight for [" - + weightedRouting.attributeName() - + "]" + "decommission action ongoing for attribute [{}], cannot update weight for [{}]", + decommissionAttribute.attributeName(), + weightedRouting.attributeName() ); } if (weightedRouting.weights().containsKey(decommissionAttribute.attributeValue()) == false) { // weight of an attribute undergoing decommission must be specified throw new UnsupportedWeightedRoutingStateException( - "weight for [" - + decommissionAttribute.attributeValue() - + "] is not specified. Please specify its weight to [0] as it is under decommission action" + "weight for [{}] is not specified. Please specify its weight to [{}] as it is under decommission action", + decommissionAttribute.attributeValue(), + DECOMMISSIONED_AWARENESS_VALUE_WEIGHT ); } if (Objects.equals( @@ -242,7 +240,9 @@ private void ensureDecommissionedAttributeHasZeroWeight(ClusterState state, Clus DECOMMISSIONED_AWARENESS_VALUE_WEIGHT ) == false) { throw new UnsupportedWeightedRoutingStateException( - "weight for [" + decommissionAttribute.attributeValue() + "] must be set to [0] as it is under decommission action" + "weight for [{}] must be set to [{}] as it is under decommission action", + decommissionAttribute.attributeValue(), + DECOMMISSIONED_AWARENESS_VALUE_WEIGHT ); } } diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java index 03638d21466dd..89d9555fe225b 100644 --- a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java +++ b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java @@ -362,7 +362,7 @@ public void onFailure(Exception e) { MatcherAssert.assertThat(exceptionReference.get(), instanceOf(UnsupportedWeightedRoutingStateException.class)); MatcherAssert.assertThat( exceptionReference.get().getMessage(), - containsString("weight for [zone_A] must be set to [0] as it is under decommission action") + containsString("weight for [zone_A] must be set to [0.0] as it is under decommission action") ); } From a4f024e93c297c5a9d4e523c5d68cc458ff7314e Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 12 Dec 2022 13:15:38 +0530 Subject: [PATCH 24/26] Empty-Commit Signed-off-by: Rishab Nahata From 694f71a818b9fc830c44ccb02abc744d507d22d8 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 12 Dec 2022 14:36:09 +0530 Subject: [PATCH 25/26] Empty-Commit Signed-off-by: Rishab Nahata From 9c5a041cf0610304731efe989992b2f796279d39 Mon Sep 17 00:00:00 2001 From: Rishab Nahata Date: Mon, 12 Dec 2022 16:40:27 +0530 Subject: [PATCH 26/26] Empty-Commit Signed-off-by: Rishab Nahata