Address review comments

Signed-off-by: Anshu Agarwal <anshukag@amazon.com>
opensearch-project · Sep 26, 2022 · 7763067 · 7763067
1 parent 0240ff0
commit 7763067
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 35 deletions.
diff --git a/...ch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java b/...ch/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequest.java
@@ -43,6 +43,8 @@ public class ClusterPutWeightedRoutingRequest extends ClusterManagerNodeRequest<
     private WeightedRouting weightedRouting;
     private String attributeName;
 
+    public ClusterPutWeightedRoutingRequest() {}
+
     public WeightedRouting getWeightedRouting() {
         return weightedRouting;
     }
@@ -61,7 +63,9 @@ public ClusterPutWeightedRoutingRequest(StreamInput in) throws IOException {
         weightedRouting = new WeightedRouting(in);
     }
 
-    public ClusterPutWeightedRoutingRequest() {}
+    public ClusterPutWeightedRoutingRequest(String attributeName) {
+        this.attributeName = attributeName;
+    }
 
     public void setWeightedRouting(Map<String, String> source) {
         try {

diff --git a/...h/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java b/...h/action/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingResponse.java
@@ -10,7 +10,6 @@
 
 import org.opensearch.action.support.master.AcknowledgedResponse;
 import org.opensearch.common.io.stream.StreamInput;
-import org.opensearch.common.xcontent.ToXContentObject;
 
 import java.io.IOException;
 
@@ -19,7 +18,7 @@
  *
  * @opensearch.internal
  */
-public class ClusterPutWeightedRoutingResponse extends AcknowledgedResponse implements ToXContentObject {
+public class ClusterPutWeightedRoutingResponse extends AcknowledgedResponse {
     public ClusterPutWeightedRoutingResponse(boolean acknowledged) {
         super(acknowledged);
     }

diff --git a/...h/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java b/...h/action/admin/cluster/shards/routing/weighted/put/TransportAddWeightedRoutingAction.java
@@ -14,6 +14,7 @@
 import org.opensearch.action.support.clustermanager.TransportClusterManagerNodeAction;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.block.ClusterBlockException;
+import org.opensearch.cluster.block.ClusterBlockLevel;
 import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
 import org.opensearch.cluster.routing.WeightedRoutingService;
 import org.opensearch.cluster.routing.allocation.decider.AwarenessAllocationDecider;
@@ -122,7 +123,7 @@ private void verifyAwarenessAttribute(String attributeName) {
 
     @Override
     protected ClusterBlockException checkBlock(ClusterPutWeightedRoutingRequest request, ClusterState state) {
-        return null;
+        return state.blocks().globalBlockedException(ClusterBlockLevel.METADATA_WRITE);
     }
 
 }
diff --git a/server/src/main/java/org/opensearch/client/Requests.java b/server/src/main/java/org/opensearch/client/Requests.java
@@ -555,7 +555,7 @@ public static SnapshotsStatusRequest snapshotsStatusRequest(String repository) {
      *
      * @return update weight request
      */
-    public static ClusterPutWeightedRoutingRequest putWeightedRoutingRequest() {
-        return new ClusterPutWeightedRoutingRequest();
+    public static ClusterPutWeightedRoutingRequest putWeightedRoutingRequest(String attributeName) {
+        return new ClusterPutWeightedRoutingRequest(attributeName);
     }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java
@@ -20,17 +20,14 @@
 import org.opensearch.cluster.metadata.WeightedRoutingMetadata;
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.Priority;
-import org.opensearch.common.component.AbstractLifecycleComponent;
 import org.opensearch.common.inject.Inject;
 
 import org.opensearch.threadpool.ThreadPool;
 
-import java.io.IOException;
-
 /**
  * * Service responsible for updating cluster state metadata with weighted routing weights
  */
-public class WeightedRoutingService extends AbstractLifecycleComponent {
+public class WeightedRoutingService {
     private static final Logger logger = LogManager.getLogger(WeightedRoutingService.class);
     private final ClusterService clusterService;
     private final ThreadPool threadPool;
@@ -70,13 +67,13 @@ public ClusterState execute(ClusterState currentState) {
 
             @Override
             public void onFailure(String source, Exception e) {
-                logger.warn(() -> new ParameterizedMessage("failed to update cluster state for weighted routing " + "weights [{}]", e));
+                logger.warn(() -> new ParameterizedMessage("failed to update cluster state for weighted routing weights [{}]", e));
                 listener.onFailure(e);
             }
 
             @Override
             public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
-                logger.info("cluster weighted routing weights metadata change is processed by all the nodes");
+                logger.debug("cluster weighted routing weights metadata change is processed by all the nodes");
                 listener.onResponse(new ClusterStateUpdateResponse(true));
             }
         });
@@ -88,13 +85,4 @@ private boolean checkIfSameWeightsInMetadata(
     ) {
         return newWeightedRoutingMetadata.getWeightedRouting().equals(oldWeightedRoutingMetadata.getWeightedRouting());
     }
-
-    @Override
-    protected void doStart() {}
-
-    @Override
-    protected void doStop() {}
-
-    @Override
-    protected void doClose() throws IOException {}
 }
diff --git a/...in/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java b/...in/java/org/opensearch/rest/action/admin/cluster/RestClusterPutWeightedRoutingAction.java
@@ -50,12 +50,7 @@ protected RestChannelConsumer prepareRequest(RestRequest request, NodeClient cli
     }
 
     public static ClusterPutWeightedRoutingRequest createRequest(RestRequest request) throws IOException {
-        ClusterPutWeightedRoutingRequest putWeightedRoutingRequest = Requests.putWeightedRoutingRequest();
-        String attributeName = null;
-        if (request.hasParam("attribute")) {
-            attributeName = request.param("attribute");
-        }
-        putWeightedRoutingRequest.attributeName(attributeName);
+        ClusterPutWeightedRoutingRequest putWeightedRoutingRequest = Requests.putWeightedRoutingRequest(request.param("attribute"));
         request.applyContentParser(p -> putWeightedRoutingRequest.source(p.mapStrings()));
         return putWeightedRoutingRequest;
     }

diff --git a/...tion/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java b/...tion/admin/cluster/shards/routing/weighted/put/ClusterPutWeightedRoutingRequestTests.java
@@ -20,27 +20,24 @@ public class ClusterPutWeightedRoutingRequestTests extends OpenSearchTestCase {
 
     public void testSetWeightedRoutingWeight() {
         String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"1\",\"us-east-1a\":\"1\"}";
-        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest();
+        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone");
         Map<String, Double> weights = Map.of("us-east-1a", 1.0, "us-east-1b", 1.0, "us-east-1c", 0.0);
         WeightedRouting weightedRouting = new WeightedRouting("zone", weights);
-        request.attributeName("zone");
         request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON);
         assertEquals(request.getWeightedRouting(), weightedRouting);
     }
 
     public void testValidate_ValuesAreProper() {
         String reqString = "{\"us-east-1c\" : \"1\", \"us-east-1b\":\"0\",\"us-east-1a\":\"1\"}";
-        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest();
-        request.attributeName("zone");
+        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone");
         request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON);
         ActionRequestValidationException actionRequestValidationException = request.validate();
         assertNull(actionRequestValidationException);
     }
 
     public void testValidate_TwoZonesWithZeroWeight() {
         String reqString = "{\"us-east-1c\" : \"0\", \"us-east-1b\":\"0\",\"us-east-1a\":\"1\"}";
-        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest();
-        request.attributeName("zone");
+        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone");
         request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON);
         ActionRequestValidationException actionRequestValidationException = request.validate();
         assertNotNull(actionRequestValidationException);
@@ -49,8 +46,7 @@ public void testValidate_TwoZonesWithZeroWeight() {
 
     public void testValidate_MissingWeights() {
         String reqString = "{}";
-        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest();
-        request.attributeName("zone");
+        ClusterPutWeightedRoutingRequest request = new ClusterPutWeightedRoutingRequest("zone");
         request.setWeightedRouting(new BytesArray(reqString), XContentType.JSON);
         ActionRequestValidationException actionRequestValidationException = request.validate();
         assertNotNull(actionRequestValidationException);