From e8cda62bd6cbc8e7d68bf375bc9ee4b0f44ff235 Mon Sep 17 00:00:00 2001 From: Anshu Agarwal Date: Thu, 6 Oct 2022 18:46:14 +0530 Subject: [PATCH] Address review comments Signed-off-by: Anshu Agarwal --- .../ClusterDeleteWeightedRoutingResponse.java | 2 +- .../TransportDeleteWeightedRoutingAction.java | 10 +--------- .../cluster/routing/WeightedRoutingService.java | 14 ++++++++++---- .../routing/WeightedRoutingServiceTests.java | 7 ++++--- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/ClusterDeleteWeightedRoutingResponse.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/ClusterDeleteWeightedRoutingResponse.java index edeb5b3b3a97c..b98ac6c0c55be 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/ClusterDeleteWeightedRoutingResponse.java +++ b/server/src/main/java/org/opensearch/action/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/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java index 868fa5907fe85..8f88d8af71b70 100644 --- a/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java +++ b/server/src/main/java/org/opensearch/action/admin/cluster/shards/routing/weighted/delete/TransportDeleteWeightedRoutingAction.java @@ -81,14 +81,6 @@ protected void clusterManagerOperation( ClusterState state, ActionListener 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 index 5dce6d386fa34..39404b0e2f687 100644 --- 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 listener + final ActionListener listener ) { clusterService.submitStateUpdateTask("delete_weighted_routing", new ClusterStateUpdateTask(Priority.URGENT) { @Override @@ -122,7 +123,12 @@ 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); } @@ -130,7 +136,7 @@ public void onFailure(String source, Exception e) { 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 index f4bfa5208b0e9..fc5d46ef84c79 100644 --- 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 listener = new ActionListener() { + ActionListener listener = new ActionListener() { @Override - public void onResponse(ClusterStateUpdateResponse clusterStateUpdateResponse) { - assertTrue(clusterStateUpdateResponse.isAcknowledged()); + public void onResponse(ClusterDeleteWeightedRoutingResponse clusterDeleteWeightedRoutingResponse) { + assertTrue(clusterDeleteWeightedRoutingResponse.isAcknowledged()); assertNull(clusterService.state().metadata().weightedRoutingMetadata()); countDownLatch.countDown(); }