Address review comments

Signed-off-by: Anshu Agarwal <[email protected]>
opensearch-project · Oct 6, 2022 · e8cda62 · e8cda62
1 parent 56fe6af
commit e8cda62
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 17 deletions.
diff --git a/...on/admin/cluster/shards/routing/weighted/delete/ClusterDeleteWeightedRoutingResponse.java b/...on/admin/cluster/shards/routing/weighted/delete/ClusterDeleteWeightedRoutingResponse.java
@@ -25,7 +25,7 @@ public class ClusterDeleteWeightedRoutingResponse extends AcknowledgedResponse {
         super(in);
     }
 
-    ClusterDeleteWeightedRoutingResponse(boolean acknowledged) {
+    public ClusterDeleteWeightedRoutingResponse(boolean acknowledged) {
         super(acknowledged);
     }
 

diff --git a/...on/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java b/...on/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java
@@ -81,14 +81,6 @@ protected void clusterManagerOperation(
         ClusterState state,
         ActionListener<ClusterDeleteWeightedRoutingResponse> listener
     ) throws Exception {
-        weightedRoutingService.deleteWeightedRoutingMetadata(
-            request,
-            ActionListener.delegateFailure(
-                listener,
-                (delegatedListener, response) -> {
-                    delegatedListener.onResponse(new ClusterDeleteWeightedRoutingResponse(response.isAcknowledged()));
-                }
-            )
-        );
+        weightedRoutingService.deleteWeightedRoutingMetadata(request, listener);
     }
 }
diff --git a/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java b/server/src/main/java/org/opensearch/cluster/routing/WeightedRoutingService.java
@@ -14,6 +14,7 @@
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.admin.cluster.shards.routing.weighted.delete.ClusterDeleteWeightedRoutingRequest;
 import org.opensearch.action.ActionRequestValidationException;
+import org.opensearch.action.admin.cluster.shards.routing.weighted.delete.ClusterDeleteWeightedRoutingResponse;
 import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequest;
 import org.opensearch.cluster.ClusterState;
 import org.opensearch.cluster.ClusterStateUpdateTask;
@@ -95,7 +96,7 @@ public void onFailure(String source, Exception e) {
             @Override
             public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
                 logger.debug("cluster weighted routing weights metadata change is processed by all the nodes");
-                listener.onResponse(new ClusterStateUpdateResponse(true));
+                listener.onResponse(new ClusterDeleteWeightedRoutingResponse(true));
             }
         });
     }
@@ -109,7 +110,7 @@ private boolean checkIfSameWeightsInMetadata(
 
     public void deleteWeightedRoutingMetadata(
         final ClusterDeleteWeightedRoutingRequest request,
-        final ActionListener<ClusterStateUpdateResponse> listener
+        final ActionListener<ClusterDeleteWeightedRoutingResponse> listener
     ) {
         clusterService.submitStateUpdateTask("delete_weighted_routing", new ClusterStateUpdateTask(Priority.URGENT) {
             @Override
@@ -122,15 +123,20 @@ public ClusterState execute(ClusterState currentState) {
 
             @Override
             public void onFailure(String source, Exception e) {
-                logger.warn(() -> new ParameterizedMessage("failed to remove weighted routing metadata from cluster state [{}]", e));
+                logger.error(
+                    () -> new ParameterizedMessage(
+                        "failed to remove weighted routing metadata from cluster state with an exception [{}]",
+                        e
+                    )
+                );
                 listener.onFailure(e);
             }
 
             @Override
             public void clusterStateProcessed(String source, ClusterState oldState, ClusterState newState) {
                 logger.debug("cluster weighted routing metadata change is processed by all the nodes");
                 assert newState.metadata().weightedRoutingMetadata() == null;
-                listener.onResponse(new ClusterStateUpdateResponse(true));
+                listener.onResponse(new ClusterDeleteWeightedRoutingResponse(true));
             }
         });
     }

diff --git a/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java b/server/src/test/java/org/opensearch/cluster/routing/WeightedRoutingServiceTests.java
@@ -14,6 +14,7 @@
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.admin.cluster.shards.routing.weighted.delete.ClusterDeleteWeightedRoutingRequest;
 import org.opensearch.action.ActionRequestValidationException;
+import org.opensearch.action.admin.cluster.shards.routing.weighted.delete.ClusterDeleteWeightedRoutingResponse;
 import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterAddWeightedRoutingAction;
 import org.opensearch.action.admin.cluster.shards.routing.weighted.put.ClusterPutWeightedRoutingRequestBuilder;
 import org.opensearch.client.node.NodeClient;
@@ -243,10 +244,10 @@ public void testDeleteWeightedRoutingMetadata() throws InterruptedException {
 
         ClusterDeleteWeightedRoutingRequest clusterDeleteWeightedRoutingRequest = new ClusterDeleteWeightedRoutingRequest();
         final CountDownLatch countDownLatch = new CountDownLatch(1);
-        ActionListener<ClusterStateUpdateResponse> listener = new ActionListener<ClusterStateUpdateResponse>() {
+        ActionListener<ClusterDeleteWeightedRoutingResponse> listener = new ActionListener<ClusterDeleteWeightedRoutingResponse>() {
             @Override
-            public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) {
-                assertTrue(clusterStateUpdateResponse.isAcknowledged());
+            public void onResponse(ClusterDeleteWeightedRoutingResponse clusterDeleteWeightedRoutingResponse) {
+                assertTrue(clusterDeleteWeightedRoutingResponse.isAcknowledged());
                 assertNull(clusterService.state().metadata().weightedRoutingMetadata());
                 countDownLatch.countDown();
             }