Remove nodeId from BaseNodeRequest (#43658)

TransportNodesAction provides a mechanism to easily broadcast a request
to many nodes, and collect the respones into a high level response. Each
node has its own request type, with a base class of BaseNodeRequest.
This base request requires passing the nodeId to which the request will
be sent. However, that nodeId is not used anywhere. It is private to the
base class, yet serialized to each node, where the node could just as
easily find the nodeId of the node it is on locally.

This commit removes passing the nodeId through to the node request
creation, and guards its serialization so that we can remove the base
request class altogether in the future.
This commit is contained in:
Ryan Ernst 2019-06-27 18:45:14 -07:00 committed by GitHub
parent 34a86cc321
commit 5b4089e57e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
27 changed files with 74 additions and 109 deletions

View File

@ -54,8 +54,8 @@ public class TransportNodesHotThreadsAction extends TransportNodesAction<NodesHo
}
@Override
protected NodeRequest newNodeRequest(String nodeId, NodesHotThreadsRequest request) {
return new NodeRequest(nodeId, request);
protected NodeRequest newNodeRequest(NodesHotThreadsRequest request) {
return new NodeRequest(request);
}
@Override
@ -85,8 +85,7 @@ public class TransportNodesHotThreadsAction extends TransportNodesAction<NodesHo
public NodeRequest() {
}
NodeRequest(String nodeId, NodesHotThreadsRequest request) {
super(nodeId);
NodeRequest(NodesHotThreadsRequest request) {
this.request = request;
}

View File

@ -56,8 +56,8 @@ public class TransportNodesInfoAction extends TransportNodesAction<NodesInfoRequ
}
@Override
protected NodeInfoRequest newNodeRequest(String nodeId, NodesInfoRequest request) {
return new NodeInfoRequest(nodeId, request);
protected NodeInfoRequest newNodeRequest(NodesInfoRequest request) {
return new NodeInfoRequest(request);
}
@Override
@ -79,8 +79,7 @@ public class TransportNodesInfoAction extends TransportNodesAction<NodesInfoRequ
public NodeInfoRequest() {
}
public NodeInfoRequest(String nodeId, NodesInfoRequest request) {
super(nodeId);
public NodeInfoRequest(NodesInfoRequest request) {
this.request = request;
}

View File

@ -68,8 +68,8 @@ public class TransportNodesReloadSecureSettingsAction extends TransportNodesActi
}
@Override
protected NodeRequest newNodeRequest(String nodeId, NodesReloadSecureSettingsRequest request) {
return new NodeRequest(nodeId, request);
protected NodeRequest newNodeRequest(NodesReloadSecureSettingsRequest request) {
return new NodeRequest(request);
}
@Override
@ -116,8 +116,7 @@ public class TransportNodesReloadSecureSettingsAction extends TransportNodesActi
public NodeRequest() {
}
NodeRequest(String nodeId, NodesReloadSecureSettingsRequest request) {
super(nodeId);
NodeRequest(NodesReloadSecureSettingsRequest request) {
this.request = request;
}

View File

@ -55,8 +55,8 @@ public class TransportNodesStatsAction extends TransportNodesAction<NodesStatsRe
}
@Override
protected NodeStatsRequest newNodeRequest(String nodeId, NodesStatsRequest request) {
return new NodeStatsRequest(nodeId, request);
protected NodeStatsRequest newNodeRequest(NodesStatsRequest request) {
return new NodeStatsRequest(request);
}
@Override
@ -79,8 +79,7 @@ public class TransportNodesStatsAction extends TransportNodesAction<NodesStatsRe
public NodeStatsRequest() {
}
NodeStatsRequest(String nodeId, NodesStatsRequest request) {
super(nodeId);
NodeStatsRequest(NodesStatsRequest request) {
this.request = request;
}

View File

@ -53,8 +53,8 @@ public class TransportNodesUsageAction
}
@Override
protected NodeUsageRequest newNodeRequest(String nodeId, NodesUsageRequest request) {
return new NodeUsageRequest(nodeId, request);
protected NodeUsageRequest newNodeRequest(NodesUsageRequest request) {
return new NodeUsageRequest(request);
}
@Override
@ -75,8 +75,7 @@ public class TransportNodesUsageAction
public NodeUsageRequest() {
}
NodeUsageRequest(String nodeId, NodesUsageRequest request) {
super(nodeId);
NodeUsageRequest(NodesUsageRequest request) {
this.request = request;
}

View File

@ -68,8 +68,8 @@ public class TransportNodesSnapshotsStatus extends TransportNodesAction<Transpor
}
@Override
protected NodeRequest newNodeRequest(String nodeId, Request request) {
return new NodeRequest(nodeId, request);
protected NodeRequest newNodeRequest(Request request) {
return new NodeRequest(request);
}
@Override
@ -168,8 +168,7 @@ public class TransportNodesSnapshotsStatus extends TransportNodesAction<Transpor
public NodeRequest() {
}
NodeRequest(String nodeId, TransportNodesSnapshotsStatus.Request request) {
super(nodeId);
NodeRequest(TransportNodesSnapshotsStatus.Request request) {
snapshots = Arrays.asList(request.snapshots);
}

View File

@ -81,8 +81,8 @@ public class TransportClusterStatsAction extends TransportNodesAction<ClusterSta
}
@Override
protected ClusterStatsNodeRequest newNodeRequest(String nodeId, ClusterStatsRequest request) {
return new ClusterStatsNodeRequest(nodeId, request);
protected ClusterStatsNodeRequest newNodeRequest(ClusterStatsRequest request) {
return new ClusterStatsNodeRequest(request);
}
@Override
@ -142,8 +142,7 @@ public class TransportClusterStatsAction extends TransportNodesAction<ClusterSta
public ClusterStatsNodeRequest() {
}
ClusterStatsNodeRequest(String nodeId, ClusterStatsRequest request) {
super(nodeId);
ClusterStatsNodeRequest(ClusterStatsRequest request) {
this.request = request;
}

View File

@ -19,33 +19,31 @@
package org.elasticsearch.action.support.nodes;
import org.elasticsearch.Version;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.transport.TransportRequest;
import java.io.IOException;
// TODO: this class can be removed in master once 7.x is bumped to 7.4.0
public abstract class BaseNodeRequest extends TransportRequest {
private String nodeId;
public BaseNodeRequest() {
}
protected BaseNodeRequest(String nodeId) {
this.nodeId = nodeId;
}
public BaseNodeRequest() {}
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
nodeId = in.readString();
if (in.getVersion().before(Version.V_7_3_0)) {
in.readString(); // previously nodeId
}
}
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(nodeId);
if (out.getVersion().before(Version.V_7_3_0)) {
out.writeString(""); // previously nodeId
}
}
}

View File

@ -119,7 +119,7 @@ public abstract class TransportNodesAction<NodesRequest extends BaseNodesRequest
*/
protected abstract NodesResponse newResponse(NodesRequest request, List<NodeResponse> responses, List<FailedNodeException> failures);
protected abstract NodeRequest newNodeRequest(String nodeId, NodesRequest request);
protected abstract NodeRequest newNodeRequest(NodesRequest request);
protected abstract NodeResponse newNodeResponse();
@ -174,7 +174,7 @@ public abstract class TransportNodesAction<NodesRequest extends BaseNodesRequest
final DiscoveryNode node = nodes[i];
final String nodeId = node.getId();
try {
TransportRequest nodeRequest = newNodeRequest(nodeId, request);
TransportRequest nodeRequest = newNodeRequest(request);
if (task != null) {
nodeRequest.setParentTask(clusterService.localNode().getId(), task.getId());
}

View File

@ -67,8 +67,8 @@ public class TransportNodesListGatewayMetaState extends TransportNodesAction<Tra
}
@Override
protected NodeRequest newNodeRequest(String nodeId, Request request) {
return new NodeRequest(nodeId);
protected NodeRequest newNodeRequest(Request request) {
return new NodeRequest();
}
@Override
@ -114,14 +114,6 @@ public class TransportNodesListGatewayMetaState extends TransportNodesAction<Tra
}
public static class NodeRequest extends BaseNodeRequest {
public NodeRequest() {
}
NodeRequest(String nodeId) {
super(nodeId);
}
}
public static class NodeGatewayMetaState extends BaseNodeResponse {

View File

@ -93,8 +93,8 @@ public class TransportNodesListGatewayStartedShards extends
}
@Override
protected NodeRequest newNodeRequest(String nodeId, Request request) {
return new NodeRequest(nodeId, request);
protected NodeRequest newNodeRequest(Request request) {
return new NodeRequest(request);
}
@Override
@ -223,8 +223,7 @@ public class TransportNodesListGatewayStartedShards extends
public NodeRequest() {
}
public NodeRequest(String nodeId, Request request) {
super(nodeId);
public NodeRequest(Request request) {
this.shardId = request.shardId();
}

View File

@ -91,8 +91,8 @@ public class TransportNodesListShardStoreMetaData extends TransportNodesAction<T
}
@Override
protected NodeRequest newNodeRequest(String nodeId, Request request) {
return new NodeRequest(nodeId, request);
protected NodeRequest newNodeRequest(Request request) {
return new NodeRequest(request);
}
@Override
@ -290,8 +290,7 @@ public class TransportNodesListShardStoreMetaData extends TransportNodesAction<T
public NodeRequest() {
}
NodeRequest(String nodeId, TransportNodesListShardStoreMetaData.Request request) {
super(nodeId);
NodeRequest(TransportNodesListShardStoreMetaData.Request request) {
this.shardId = request.shardId;
}

View File

@ -60,35 +60,30 @@ public class CancellableTasksTests extends TaskManagerTestCase {
public static class CancellableNodeRequest extends BaseNodeRequest {
protected String requestName;
protected String nodeId;
public CancellableNodeRequest() {
super();
}
public CancellableNodeRequest(CancellableNodesRequest request, String nodeId) {
super(nodeId);
public CancellableNodeRequest(CancellableNodesRequest request) {
requestName = request.requestName;
this.nodeId = nodeId;
}
@Override
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
requestName = in.readString();
nodeId = in.readString();
}
@Override
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(requestName);
out.writeString(nodeId);
}
@Override
public String getDescription() {
return "CancellableNodeRequest[" + requestName + ", " + nodeId + "]";
return "CancellableNodeRequest[" + requestName + "]";
}
@Override
@ -161,19 +156,19 @@ public class CancellableTasksTests extends TaskManagerTestCase {
}
@Override
protected CancellableNodeRequest newNodeRequest(String nodeId, CancellableNodesRequest request) {
return new CancellableNodeRequest(request, nodeId);
protected CancellableNodeRequest newNodeRequest(CancellableNodesRequest request) {
return new CancellableNodeRequest(request);
}
@Override
protected NodeResponse nodeOperation(CancellableNodeRequest request, Task task) {
assert task instanceof CancellableTask;
debugDelay(request.nodeId, "op1");
debugDelay("op1");
if (actionStartedLatch != null) {
actionStartedLatch.countDown();
}
debugDelay(request.nodeId, "op2");
debugDelay("op2");
if (shouldBlock) {
// Simulate a job that takes forever to finish
// Using periodic checks method to identify that the task was cancelled
@ -189,7 +184,7 @@ public class CancellableTasksTests extends TaskManagerTestCase {
Thread.currentThread().interrupt();
}
}
debugDelay(request.nodeId, "op4");
debugDelay("op4");
return new NodeResponse(clusterService.localNode());
}
@ -426,9 +421,9 @@ public class CancellableTasksTests extends TaskManagerTestCase {
}
private static void debugDelay(String nodeId, String name) {
private static void debugDelay(String name) {
// Introduce an additional pseudo random repeatable race conditions
String delayName = RandomizedContext.current().getRunnerSeedAsString() + ":" + nodeId + ":" + name;
String delayName = RandomizedContext.current().getRunnerSeedAsString() + ":" + name;
Random random = new Random(delayName.hashCode());
if (RandomNumbers.randomIntBetween(random, 0, 10) < 1) {
try {

View File

@ -169,17 +169,14 @@ public class TestTaskPlugin extends Plugin implements ActionPlugin, NetworkPlugi
public static class NodeRequest extends BaseNodeRequest {
protected String requestName;
protected String nodeId;
protected boolean shouldBlock;
public NodeRequest() {
super();
}
public NodeRequest(NodesRequest request, String nodeId, boolean shouldBlock) {
super(nodeId);
public NodeRequest(NodesRequest request, boolean shouldBlock) {
requestName = request.requestName;
this.nodeId = nodeId;
this.shouldBlock = shouldBlock;
}
@ -187,7 +184,6 @@ public class TestTaskPlugin extends Plugin implements ActionPlugin, NetworkPlugi
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
requestName = in.readString();
nodeId = in.readString();
shouldBlock = in.readBoolean();
}
@ -195,13 +191,12 @@ public class TestTaskPlugin extends Plugin implements ActionPlugin, NetworkPlugi
public void writeTo(StreamOutput out) throws IOException {
super.writeTo(out);
out.writeString(requestName);
out.writeString(nodeId);
out.writeBoolean(shouldBlock);
}
@Override
public String getDescription() {
return "NodeRequest[" + requestName + ", " + nodeId + "]";
return "NodeRequest[" + requestName + "]";
}
@Override
@ -301,8 +296,8 @@ public class TestTaskPlugin extends Plugin implements ActionPlugin, NetworkPlugi
}
@Override
protected NodeRequest newNodeRequest(String nodeId, NodesRequest request) {
return new NodeRequest(request, nodeId, request.getShouldBlock());
protected NodeRequest newNodeRequest(NodesRequest request) {
return new NodeRequest(request, request.getShouldBlock());
}
@Override

View File

@ -84,8 +84,7 @@ public class TransportTasksActionTests extends TaskManagerTestCase {
super();
}
public NodeRequest(NodesRequest request, String nodeId) {
super(nodeId);
public NodeRequest(NodesRequest request) {
requestName = request.requestName;
}
@ -157,8 +156,8 @@ public class TransportTasksActionTests extends TaskManagerTestCase {
}
@Override
protected NodeRequest newNodeRequest(String nodeId, NodesRequest request) {
return new NodeRequest(request, nodeId);
protected NodeRequest newNodeRequest(NodesRequest request) {
return new NodeRequest(request);
}
@Override

View File

@ -257,7 +257,7 @@ public class TransportNodesActionTests extends ESTestCase {
}
@Override
protected TestNodeRequest newNodeRequest(String nodeId, TestNodesRequest request) {
protected TestNodeRequest newNodeRequest(TestNodesRequest request) {
return new TestNodeRequest();
}

View File

@ -41,8 +41,7 @@ public class NodesDeprecationCheckAction extends Action<NodesDeprecationCheckRes
NodesDeprecationCheckRequest request;
public NodeRequest() {}
public NodeRequest(String nodeId, NodesDeprecationCheckRequest request) {
super(nodeId);
public NodeRequest(NodesDeprecationCheckRequest request) {
this.request = request;
}

View File

@ -89,8 +89,7 @@ public class ClearRealmCacheRequest extends BaseNodesRequest<ClearRealmCacheRequ
public Node() {
}
public Node(ClearRealmCacheRequest request, String nodeId) {
super(nodeId);
public Node(ClearRealmCacheRequest request) {
this.realms = request.realms;
this.usernames = request.usernames;
}

View File

@ -54,8 +54,7 @@ public class ClearRolesCacheRequest extends BaseNodesRequest<ClearRolesCacheRequ
public Node() {
}
public Node(ClearRolesCacheRequest request, String nodeId) {
super(nodeId);
public Node(ClearRolesCacheRequest request) {
this.names = request.names();
}

View File

@ -83,8 +83,7 @@ public class WatcherStatsRequest extends BaseNodesRequest<WatcherStatsRequest> {
public Node() {}
public Node(WatcherStatsRequest request, String nodeId) {
super(nodeId);
public Node(WatcherStatsRequest request) {
includeCurrentWatches = request.includeCurrentWatches();
includeQueuedWatches = request.includeQueuedWatches();
includeStats = request.includeStats();

View File

@ -52,8 +52,8 @@ public class TransportNodeDeprecationCheckAction extends TransportNodesAction<No
}
@Override
protected NodesDeprecationCheckAction.NodeRequest newNodeRequest(String nodeId, NodesDeprecationCheckRequest request) {
return new NodesDeprecationCheckAction.NodeRequest(nodeId, request);
protected NodesDeprecationCheckAction.NodeRequest newNodeRequest(NodesDeprecationCheckRequest request) {
return new NodesDeprecationCheckAction.NodeRequest(request);
}
@Override

View File

@ -46,8 +46,8 @@ public class TransportClearRealmCacheAction extends TransportNodesAction<ClearRe
}
@Override
protected ClearRealmCacheRequest.Node newNodeRequest(String nodeId, ClearRealmCacheRequest request) {
return new ClearRealmCacheRequest.Node(request, nodeId);
protected ClearRealmCacheRequest.Node newNodeRequest(ClearRealmCacheRequest request) {
return new ClearRealmCacheRequest.Node(request);
}
@Override

View File

@ -39,8 +39,8 @@ public class TransportClearRolesCacheAction extends TransportNodesAction<ClearRo
}
@Override
protected ClearRolesCacheRequest.Node newNodeRequest(String nodeId, ClearRolesCacheRequest request) {
return new ClearRolesCacheRequest.Node(request, nodeId);
protected ClearRolesCacheRequest.Node newNodeRequest(ClearRolesCacheRequest request) {
return new ClearRolesCacheRequest.Node(request);
}
@Override

View File

@ -53,8 +53,7 @@ public class SqlStatsRequest extends BaseNodesRequest<SqlStatsRequest> {
NodeStatsRequest() {}
NodeStatsRequest(SqlStatsRequest request, String nodeId) {
super(nodeId);
NodeStatsRequest(SqlStatsRequest request) {
includeStats = request.includeStats();
}

View File

@ -41,8 +41,8 @@ public class TransportSqlStatsAction extends TransportNodesAction<SqlStatsReques
}
@Override
protected SqlStatsRequest.NodeStatsRequest newNodeRequest(String nodeId, SqlStatsRequest request) {
return new SqlStatsRequest.NodeStatsRequest(request, nodeId);
protected SqlStatsRequest.NodeStatsRequest newNodeRequest(SqlStatsRequest request) {
return new SqlStatsRequest.NodeStatsRequest(request);
}
@Override

View File

@ -52,8 +52,8 @@ public class TransportWatcherStatsAction extends TransportNodesAction<WatcherSta
}
@Override
protected WatcherStatsRequest.Node newNodeRequest(String nodeId, WatcherStatsRequest request) {
return new WatcherStatsRequest.Node(request, nodeId);
protected WatcherStatsRequest.Node newNodeRequest(WatcherStatsRequest request) {
return new WatcherStatsRequest.Node(request);
}
@Override

View File

@ -85,8 +85,8 @@ public class TransportWatcherStatsActionTests extends ESTestCase {
public void testWatcherStats() throws Exception {
WatcherStatsRequest request = new WatcherStatsRequest();
request.includeStats(true);
WatcherStatsResponse.Node nodeResponse1 = action.nodeOperation(new WatcherStatsRequest.Node(request, "nodeId"));
WatcherStatsResponse.Node nodeResponse2 = action.nodeOperation(new WatcherStatsRequest.Node(request, "nodeId2"));
WatcherStatsResponse.Node nodeResponse1 = action.nodeOperation(new WatcherStatsRequest.Node(request));
WatcherStatsResponse.Node nodeResponse2 = action.nodeOperation(new WatcherStatsRequest.Node(request));
WatcherStatsResponse response = action.newResponse(request,
Arrays.asList(nodeResponse1, nodeResponse2), Collections.emptyList());