Remove Redundant Cluster State Update Response Classes (#63646) (#64870)

These intermediary response types don't contain any information outside of what the shard acknowledged and acknowledged responses contain so this PR removes them. Also, it adds three constants for the three possible states of `ShardsAcknowledgedResponse`. Follow up to #63335
elastic · Nov 10, 2020 · 6181ba3 · 6181ba3
1 parent d7697b4
commit 6181ba3
Show file tree

Hide file tree

Showing 54 changed files with 231 additions and 538 deletions.
diff --git a/...st-high-level/src/test/java/org/elasticsearch/client/indices/CloseIndexResponseTests.java b/...st-high-level/src/test/java/org/elasticsearch/client/indices/CloseIndexResponseTests.java
@@ -191,7 +191,7 @@ public final void testBwcFromXContent() throws IOException {
         {
             final boolean acknowledged = randomBoolean();
             final boolean shardsAcknowledged = acknowledged ? randomBoolean() : false;
-            final ShardsAcknowledgedResponse expected = new ShardsAcknowledgedResponse(acknowledged, shardsAcknowledged){};
+            final ShardsAcknowledgedResponse expected = ShardsAcknowledgedResponse.of(acknowledged, shardsAcknowledged);
 
             final XContentType xContentType = randomFrom(XContentType.values());
             final BytesReference bytes = toShuffledXContent(expected, xContentType, getParams(), randomBoolean());

diff --git a/server/src/internalClusterTest/java/org/elasticsearch/cluster/service/ClusterServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/cluster/service/ClusterServiceIT.java
@@ -59,12 +59,7 @@ public void testAckedUpdateTask() throws Exception {
         final CountDownLatch latch = new CountDownLatch(1);
         final CountDownLatch processedLatch = new CountDownLatch(1);
         clusterService.submitStateUpdateTask("test",
-                new AckedClusterStateUpdateTask<Void>(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
-            @Override
-            protected Void newResponse(boolean acknowledged) {
-                return null;
-            }
-
+                new AckedClusterStateUpdateTask(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
             @Override
             public boolean mustAck(DiscoveryNode discoveryNode) {
                 return true;
@@ -123,12 +118,7 @@ public void testAckedUpdateTaskSameClusterState() throws Exception {
         final CountDownLatch latch = new CountDownLatch(1);
         final CountDownLatch processedLatch = new CountDownLatch(1);
         clusterService.submitStateUpdateTask("test",
-                new AckedClusterStateUpdateTask<Void>(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
-            @Override
-            protected Void newResponse(boolean acknowledged) {
-                return null;
-            }
-
+                new AckedClusterStateUpdateTask(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
             @Override
             public void onAllNodesAcked(@Nullable Exception e) {
                 allNodesAcked.set(true);
@@ -182,12 +172,7 @@ public void testAckedUpdateTaskNoAckExpected() throws Exception {
         final CountDownLatch latch = new CountDownLatch(1);
 
         clusterService.submitStateUpdateTask(
-                "test", new AckedClusterStateUpdateTask<Void>(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
-            @Override
-            protected Void newResponse(boolean acknowledged) {
-                return null;
-            }
-
+                "test", new AckedClusterStateUpdateTask(MasterServiceTests.ackedRequest(TEN_SECONDS, TEN_SECONDS), null) {
             @Override
             public boolean mustAck(DiscoveryNode discoveryNode) {
                 return false;
@@ -243,12 +228,7 @@ public void testAckedUpdateTaskTimeoutZero() throws Exception {
         final CountDownLatch latch = new CountDownLatch(1);
         final CountDownLatch processedLatch = new CountDownLatch(1);
         clusterService.submitStateUpdateTask("test",
-                new AckedClusterStateUpdateTask<Void>(MasterServiceTests.ackedRequest(TimeValue.ZERO, TEN_SECONDS), null) {
-            @Override
-            protected Void newResponse(boolean acknowledged) {
-                return null;
-            }
-
+                new AckedClusterStateUpdateTask(MasterServiceTests.ackedRequest(TimeValue.ZERO, TEN_SECONDS), null) {
             @Override
             public boolean mustAck(DiscoveryNode discoveryNode) {
                 return false;

diff --git a/...in/java/org/elasticsearch/action/admin/cluster/reroute/TransportClusterRerouteAction.java b/...in/java/org/elasticsearch/action/admin/cluster/reroute/TransportClusterRerouteAction.java
@@ -149,7 +149,7 @@ private void submitStateUpdate(final ClusterRerouteRequest request, final Action
                     })));
     }
 
-    static class ClusterRerouteResponseAckedClusterStateUpdateTask extends AckedClusterStateUpdateTask<ClusterRerouteResponse> {
+    static class ClusterRerouteResponseAckedClusterStateUpdateTask extends AckedClusterStateUpdateTask {
 
         private final ClusterRerouteRequest request;
         private final ActionListener<ClusterRerouteResponse> listener;

diff --git a/...org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java b/...org/elasticsearch/action/admin/cluster/settings/TransportClusterUpdateSettingsAction.java
@@ -82,7 +82,7 @@ protected void masterOperation(final ClusterUpdateSettingsRequest request, final
                                    final ActionListener<ClusterUpdateSettingsResponse> listener) {
         final SettingsUpdater updater = new SettingsUpdater(clusterSettings);
         clusterService.submitStateUpdateTask("cluster_update_settings",
-                new AckedClusterStateUpdateTask<ClusterUpdateSettingsResponse>(Priority.IMMEDIATE, request, listener) {
+                new AckedClusterStateUpdateTask(Priority.IMMEDIATE, request, listener) {
 
             private volatile boolean changed = false;
 
@@ -125,7 +125,7 @@ private void reroute(final boolean updateSettingsAcked) {
                 // to the components until the ClusterStateListener instances have been invoked, but are visible after
                 // the first update task has been completed.
                 clusterService.submitStateUpdateTask("reroute_after_cluster_update_settings",
-                        new AckedClusterStateUpdateTask<ClusterUpdateSettingsResponse>(Priority.URGENT, request, listener) {
+                        new AckedClusterStateUpdateTask(Priority.URGENT, request, listener) {
 
                     @Override
                     public boolean mustAck(DiscoveryNode discoveryNode) {

diff --git a/...main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java b/...main/java/org/elasticsearch/action/admin/indices/alias/TransportIndicesAliasesAction.java
@@ -29,7 +29,6 @@
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.AliasAction;
@@ -146,10 +145,10 @@ protected void masterOperation(final IndicesAliasesRequest request, final Cluste
         IndicesAliasesClusterStateUpdateRequest updateRequest = new IndicesAliasesClusterStateUpdateRequest(unmodifiableList(finalActions))
                 .ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout());
 
-        indexAliasesService.indicesAliases(updateRequest, new ActionListener<ClusterStateUpdateResponse>() {
+        indexAliasesService.indicesAliases(updateRequest, new ActionListener<AcknowledgedResponse>() {
             @Override
-            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(AcknowledgedResponse.of(response.isAcknowledged()));
+            public void onResponse(AcknowledgedResponse response) {
+                listener.onResponse(response);
             }
 
             @Override

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java b/server/src/main/java/org/elasticsearch/action/admin/indices/create/AutoCreateAction.java
@@ -23,11 +23,11 @@
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.ActiveShardCount;
 import org.elasticsearch.action.support.ActiveShardsObserver;
+import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.AutoCreateIndex;
 import org.elasticsearch.action.support.master.TransportMasterNodeAction;
 import org.elasticsearch.cluster.AckedClusterStateUpdateTask;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.ComposableIndexTemplate;
@@ -84,7 +84,7 @@ protected void masterOperation(CreateIndexRequest request,
                                        ClusterState state,
                                        ActionListener<CreateIndexResponse> finalListener) {
             AtomicReference<String> indexNameRef = new AtomicReference<>();
-            ActionListener<ClusterStateUpdateResponse> listener = ActionListener.wrap(
+            ActionListener<AcknowledgedResponse> listener = ActionListener.wrap(
                 response -> {
                     String indexName = indexNameRef.get();
                     assert indexName != null;
@@ -105,12 +105,7 @@ protected void masterOperation(CreateIndexRequest request,
                 finalListener::onFailure
             );
             clusterService.submitStateUpdateTask("auto create [" + request.index() + "]",
-                new AckedClusterStateUpdateTask<ClusterStateUpdateResponse>(Priority.URGENT, request, listener) {
-
-                @Override
-                protected ClusterStateUpdateResponse newResponse(boolean acknowledged) {
-                    return new ClusterStateUpdateResponse(acknowledged);
-                }
+                new AckedClusterStateUpdateTask(Priority.URGENT, request, listener) {
 
                 @Override
                 public ClusterState execute(ClusterState currentState) throws Exception {

diff --git a/...lasticsearch/action/admin/indices/dangling/delete/TransportDeleteDanglingIndexAction.java b/...lasticsearch/action/admin/indices/dangling/delete/TransportDeleteDanglingIndexAction.java
@@ -121,14 +121,7 @@ public void onFailure(Exception e) {
                 final String taskSource = "delete-dangling-index [" + indexName + "] [" + indexUUID + "]";
 
                 clusterService.submitStateUpdateTask(
-                    taskSource,
-                    new AckedClusterStateUpdateTask<AcknowledgedResponse>(deleteRequest, clusterStateUpdatedListener) {
-
-                        @Override
-                        protected AcknowledgedResponse newResponse(boolean acknowledged) {
-                            return AcknowledgedResponse.of(acknowledged);
-                        }
-
+                    taskSource, new AckedClusterStateUpdateTask(deleteRequest, clusterStateUpdatedListener) {
                         @Override
                         public ClusterState execute(final ClusterState currentState) {
                             return deleteDanglingIndex(currentState, indexToDelete);

diff --git a/...c/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java b/...c/main/java/org/elasticsearch/action/admin/indices/delete/TransportDeleteIndexAction.java
@@ -28,7 +28,6 @@
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MetadataDeleteIndexService;
@@ -88,11 +87,11 @@ protected void masterOperation(final DeleteIndexRequest request, final ClusterSt
             .ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout())
             .indices(concreteIndices.toArray(new Index[concreteIndices.size()]));
 
-        deleteIndexService.deleteIndices(deleteRequest, new ActionListener<ClusterStateUpdateResponse>() {
+        deleteIndexService.deleteIndices(deleteRequest, new ActionListener<AcknowledgedResponse>() {
 
             @Override
-            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(AcknowledgedResponse.of(response.isAcknowledged()));
+            public void onResponse(AcknowledgedResponse response) {
+                listener.onResponse(response);
             }
 
             @Override

diff --git a/...in/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java b/...in/java/org/elasticsearch/action/admin/indices/mapping/put/TransportPutMappingAction.java
@@ -28,7 +28,6 @@
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
@@ -127,11 +126,11 @@ static void performMappingUpdate(Index[] concreteIndices,
                     .indices(concreteIndices).type(request.type())
                     .source(request.source());
 
-        metadataMappingService.putMapping(updateRequest, new ActionListener<ClusterStateUpdateResponse>() {
+        metadataMappingService.putMapping(updateRequest, new ActionListener<AcknowledgedResponse>() {
 
             @Override
-            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(AcknowledgedResponse.of(response.isAcknowledged()));
+            public void onResponse(AcknowledgedResponse response) {
+                listener.onResponse(response);
             }
 
             @Override

diff --git a/...r/src/main/java/org/elasticsearch/action/admin/indices/open/TransportOpenIndexAction.java b/...r/src/main/java/org/elasticsearch/action/admin/indices/open/TransportOpenIndexAction.java
@@ -25,9 +25,9 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.DestructiveOperations;
+import org.elasticsearch.action.support.master.ShardsAcknowledgedResponse;
 import org.elasticsearch.action.support.master.TransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.OpenIndexClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
@@ -84,10 +84,10 @@ protected void masterOperation(final OpenIndexRequest request, final ClusterStat
                 .ackTimeout(request.timeout()).masterNodeTimeout(request.masterNodeTimeout())
                 .indices(concreteIndices).waitForActiveShards(request.waitForActiveShards());
 
-        indexStateService.openIndex(updateRequest, new ActionListener<OpenIndexClusterStateUpdateResponse>() {
+        indexStateService.openIndex(updateRequest, new ActionListener<ShardsAcknowledgedResponse>() {
 
             @Override
-            public void onResponse(OpenIndexClusterStateUpdateResponse response) {
+            public void onResponse(ShardsAcknowledgedResponse response) {
                 listener.onResponse(new OpenIndexResponse(response.isAcknowledged(), response.isShardsAcknowledged()));
             }
 

diff --git a/...va/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java b/...va/org/elasticsearch/action/admin/indices/settings/put/TransportUpdateSettingsAction.java
@@ -27,7 +27,6 @@
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -82,10 +81,10 @@ protected void masterOperation(final UpdateSettingsRequest request, final Cluste
                 .ackTimeout(request.timeout())
                 .masterNodeTimeout(request.masterNodeTimeout());
 
-        updateSettingsService.updateSettings(clusterStateUpdateRequest, new ActionListener<ClusterStateUpdateResponse>() {
+        updateSettingsService.updateSettings(clusterStateUpdateRequest, new ActionListener<AcknowledgedResponse>() {
             @Override
-            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(AcknowledgedResponse.of(response.isAcknowledged()));
+            public void onResponse(AcknowledgedResponse response) {
+                listener.onResponse(response);
             }
 
             @Override

diff --git a/...lasticsearch/action/admin/indices/template/delete/TransportDeleteIndexTemplateAction.java b/...lasticsearch/action/admin/indices/template/delete/TransportDeleteIndexTemplateAction.java
@@ -67,8 +67,8 @@ protected void masterOperation(final DeleteIndexTemplateRequest request, final C
                 .masterTimeout(request.masterNodeTimeout()),
             new MetadataIndexTemplateService.RemoveListener() {
                 @Override
-                public void onResponse(MetadataIndexTemplateService.RemoveResponse response) {
-                    listener.onResponse(AcknowledgedResponse.of(response.acknowledged()));
+                public void onResponse(AcknowledgedResponse response) {
+                    listener.onResponse(response);
                 }
 
                 @Override

diff --git a/...a/org/elasticsearch/action/admin/indices/upgrade/post/TransportUpgradeSettingsAction.java b/...a/org/elasticsearch/action/admin/indices/upgrade/post/TransportUpgradeSettingsAction.java
@@ -27,7 +27,6 @@
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.action.support.master.AcknowledgedTransportMasterNodeAction;
 import org.elasticsearch.cluster.ClusterState;
-import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.cluster.block.ClusterBlockException;
 import org.elasticsearch.cluster.block.ClusterBlockLevel;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
@@ -65,10 +64,10 @@ protected void masterOperation(final UpgradeSettingsRequest request, final Clust
                 .versions(request.versions())
                 .masterNodeTimeout(request.masterNodeTimeout());
 
-        updateSettingsService.upgradeIndexSettings(clusterStateUpdateRequest, new ActionListener<ClusterStateUpdateResponse>() {
+        updateSettingsService.upgradeIndexSettings(clusterStateUpdateRequest, new ActionListener<AcknowledgedResponse>() {
             @Override
-            public void onResponse(ClusterStateUpdateResponse response) {
-                listener.onResponse(AcknowledgedResponse.of(response.isAcknowledged()));
+            public void onResponse(AcknowledgedResponse response) {
+                listener.onResponse(response);
             }
 
             @Override

diff --git a/server/src/main/java/org/elasticsearch/action/support/master/ShardsAcknowledgedResponse.java b/server/src/main/java/org/elasticsearch/action/support/master/ShardsAcknowledgedResponse.java
@@ -31,7 +31,7 @@
 
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
 
-public abstract class ShardsAcknowledgedResponse extends AcknowledgedResponse {
+public class ShardsAcknowledgedResponse extends AcknowledgedResponse {
 
     protected static final ParseField SHARDS_ACKNOWLEDGED = new ParseField("shards_acknowledged");
 
@@ -42,6 +42,10 @@ protected static <T extends ShardsAcknowledgedResponse> void declareAcknowledged
                 ObjectParser.ValueType.BOOLEAN);
     }
 
+    public static final ShardsAcknowledgedResponse NOT_ACKNOWLEDGED = new ShardsAcknowledgedResponse(false, false);
+    private static final ShardsAcknowledgedResponse SHARDS_NOT_ACKNOWLEDGED = new ShardsAcknowledgedResponse(true, false);
+    private static final ShardsAcknowledgedResponse ACKNOWLEDGED = new ShardsAcknowledgedResponse(true, true);
+
     private final boolean shardsAcknowledged;
 
     protected ShardsAcknowledgedResponse(StreamInput in, boolean readShardsAcknowledged, boolean readAcknowledged) throws IOException {
@@ -53,6 +57,15 @@ protected ShardsAcknowledgedResponse(StreamInput in, boolean readShardsAcknowled
         }
     }
 
+    public static ShardsAcknowledgedResponse of(boolean acknowledged, boolean shardsAcknowledged) {
+        if (acknowledged) {
+            return shardsAcknowledged ? ACKNOWLEDGED : SHARDS_NOT_ACKNOWLEDGED;
+        } else {
+            assert shardsAcknowledged == false;
+            return NOT_ACKNOWLEDGED;
+        }
+    }
+
     protected ShardsAcknowledgedResponse(boolean acknowledged, boolean shardsAcknowledged) {
         super(acknowledged);
         assert acknowledged || shardsAcknowledged == false; // if it's not acknowledged, then shards acked should be false too
@@ -90,5 +103,4 @@ public boolean equals(Object o) {
     public int hashCode() {
         return Objects.hash(super.hashCode(), isShardsAcknowledged());
     }
-
 }