Skip to content

Commit 5b4089e

Browse files
authored
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.
1 parent 34a86cc commit 5b4089e

File tree

27 files changed

+74
-109
lines changed

27 files changed

+74
-109
lines changed

server/src/main/java/org/elasticsearch/action/admin/cluster/node/hotthreads/TransportNodesHotThreadsAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ protected NodesHotThreadsResponse newResponse(NodesHotThreadsRequest request,
5454
}
5555

5656
@Override
57-
protected NodeRequest newNodeRequest(String nodeId, NodesHotThreadsRequest request) {
58-
return new NodeRequest(nodeId, request);
57+
protected NodeRequest newNodeRequest(NodesHotThreadsRequest request) {
58+
return new NodeRequest(request);
5959
}
6060

6161
@Override
@@ -85,8 +85,7 @@ public static class NodeRequest extends BaseNodeRequest {
8585
public NodeRequest() {
8686
}
8787

88-
NodeRequest(String nodeId, NodesHotThreadsRequest request) {
89-
super(nodeId);
88+
NodeRequest(NodesHotThreadsRequest request) {
9089
this.request = request;
9190
}
9291

server/src/main/java/org/elasticsearch/action/admin/cluster/node/info/TransportNodesInfoAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ protected NodesInfoResponse newResponse(NodesInfoRequest nodesInfoRequest,
5656
}
5757

5858
@Override
59-
protected NodeInfoRequest newNodeRequest(String nodeId, NodesInfoRequest request) {
60-
return new NodeInfoRequest(nodeId, request);
59+
protected NodeInfoRequest newNodeRequest(NodesInfoRequest request) {
60+
return new NodeInfoRequest(request);
6161
}
6262

6363
@Override
@@ -79,8 +79,7 @@ public static class NodeInfoRequest extends BaseNodeRequest {
7979
public NodeInfoRequest() {
8080
}
8181

82-
public NodeInfoRequest(String nodeId, NodesInfoRequest request) {
83-
super(nodeId);
82+
public NodeInfoRequest(NodesInfoRequest request) {
8483
this.request = request;
8584
}
8685

server/src/main/java/org/elasticsearch/action/admin/cluster/node/reload/TransportNodesReloadSecureSettingsAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ protected NodesReloadSecureSettingsResponse newResponse(NodesReloadSecureSetting
6868
}
6969

7070
@Override
71-
protected NodeRequest newNodeRequest(String nodeId, NodesReloadSecureSettingsRequest request) {
72-
return new NodeRequest(nodeId, request);
71+
protected NodeRequest newNodeRequest(NodesReloadSecureSettingsRequest request) {
72+
return new NodeRequest(request);
7373
}
7474

7575
@Override
@@ -116,8 +116,7 @@ public static class NodeRequest extends BaseNodeRequest {
116116
public NodeRequest() {
117117
}
118118

119-
NodeRequest(String nodeId, NodesReloadSecureSettingsRequest request) {
120-
super(nodeId);
119+
NodeRequest(NodesReloadSecureSettingsRequest request) {
121120
this.request = request;
122121
}
123122

server/src/main/java/org/elasticsearch/action/admin/cluster/node/stats/TransportNodesStatsAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ protected NodesStatsResponse newResponse(NodesStatsRequest request, List<NodeSta
5555
}
5656

5757
@Override
58-
protected NodeStatsRequest newNodeRequest(String nodeId, NodesStatsRequest request) {
59-
return new NodeStatsRequest(nodeId, request);
58+
protected NodeStatsRequest newNodeRequest(NodesStatsRequest request) {
59+
return new NodeStatsRequest(request);
6060
}
6161

6262
@Override
@@ -79,8 +79,7 @@ public static class NodeStatsRequest extends BaseNodeRequest {
7979
public NodeStatsRequest() {
8080
}
8181

82-
NodeStatsRequest(String nodeId, NodesStatsRequest request) {
83-
super(nodeId);
82+
NodeStatsRequest(NodesStatsRequest request) {
8483
this.request = request;
8584
}
8685

server/src/main/java/org/elasticsearch/action/admin/cluster/node/usage/TransportNodesUsageAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ protected NodesUsageResponse newResponse(NodesUsageRequest request, List<NodeUsa
5353
}
5454

5555
@Override
56-
protected NodeUsageRequest newNodeRequest(String nodeId, NodesUsageRequest request) {
57-
return new NodeUsageRequest(nodeId, request);
56+
protected NodeUsageRequest newNodeRequest(NodesUsageRequest request) {
57+
return new NodeUsageRequest(request);
5858
}
5959

6060
@Override
@@ -75,8 +75,7 @@ public static class NodeUsageRequest extends BaseNodeRequest {
7575
public NodeUsageRequest() {
7676
}
7777

78-
NodeUsageRequest(String nodeId, NodesUsageRequest request) {
79-
super(nodeId);
78+
NodeUsageRequest(NodesUsageRequest request) {
8079
this.request = request;
8180
}
8281

server/src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportNodesSnapshotsStatus.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ public TransportNodesSnapshotsStatus(ThreadPool threadPool, ClusterService clust
6868
}
6969

7070
@Override
71-
protected NodeRequest newNodeRequest(String nodeId, Request request) {
72-
return new NodeRequest(nodeId, request);
71+
protected NodeRequest newNodeRequest(Request request) {
72+
return new NodeRequest(request);
7373
}
7474

7575
@Override
@@ -168,8 +168,7 @@ public static class NodeRequest extends BaseNodeRequest {
168168
public NodeRequest() {
169169
}
170170

171-
NodeRequest(String nodeId, TransportNodesSnapshotsStatus.Request request) {
172-
super(nodeId);
171+
NodeRequest(TransportNodesSnapshotsStatus.Request request) {
173172
snapshots = Arrays.asList(request.snapshots);
174173
}
175174

server/src/main/java/org/elasticsearch/action/admin/cluster/stats/TransportClusterStatsAction.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,8 @@ protected ClusterStatsResponse newResponse(ClusterStatsRequest request,
8181
}
8282

8383
@Override
84-
protected ClusterStatsNodeRequest newNodeRequest(String nodeId, ClusterStatsRequest request) {
85-
return new ClusterStatsNodeRequest(nodeId, request);
84+
protected ClusterStatsNodeRequest newNodeRequest(ClusterStatsRequest request) {
85+
return new ClusterStatsNodeRequest(request);
8686
}
8787

8888
@Override
@@ -142,8 +142,7 @@ public static class ClusterStatsNodeRequest extends BaseNodeRequest {
142142
public ClusterStatsNodeRequest() {
143143
}
144144

145-
ClusterStatsNodeRequest(String nodeId, ClusterStatsRequest request) {
146-
super(nodeId);
145+
ClusterStatsNodeRequest(ClusterStatsRequest request) {
147146
this.request = request;
148147
}
149148

server/src/main/java/org/elasticsearch/action/support/nodes/BaseNodeRequest.java

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,31 @@
1919

2020
package org.elasticsearch.action.support.nodes;
2121

22+
import org.elasticsearch.Version;
2223
import org.elasticsearch.common.io.stream.StreamInput;
2324
import org.elasticsearch.common.io.stream.StreamOutput;
2425
import org.elasticsearch.transport.TransportRequest;
2526

2627
import java.io.IOException;
2728

29+
// TODO: this class can be removed in master once 7.x is bumped to 7.4.0
2830
public abstract class BaseNodeRequest extends TransportRequest {
2931

30-
private String nodeId;
31-
32-
public BaseNodeRequest() {
33-
34-
}
35-
36-
protected BaseNodeRequest(String nodeId) {
37-
this.nodeId = nodeId;
38-
}
32+
public BaseNodeRequest() {}
3933

4034
@Override
4135
public void readFrom(StreamInput in) throws IOException {
4236
super.readFrom(in);
43-
nodeId = in.readString();
37+
if (in.getVersion().before(Version.V_7_3_0)) {
38+
in.readString(); // previously nodeId
39+
}
4440
}
4541

4642
@Override
4743
public void writeTo(StreamOutput out) throws IOException {
4844
super.writeTo(out);
49-
out.writeString(nodeId);
45+
if (out.getVersion().before(Version.V_7_3_0)) {
46+
out.writeString(""); // previously nodeId
47+
}
5048
}
5149
}

server/src/main/java/org/elasticsearch/action/support/nodes/TransportNodesAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ protected NodesResponse newResponse(NodesRequest request, AtomicReferenceArray n
119119
*/
120120
protected abstract NodesResponse newResponse(NodesRequest request, List<NodeResponse> responses, List<FailedNodeException> failures);
121121

122-
protected abstract NodeRequest newNodeRequest(String nodeId, NodesRequest request);
122+
protected abstract NodeRequest newNodeRequest(NodesRequest request);
123123

124124
protected abstract NodeResponse newNodeResponse();
125125

@@ -174,7 +174,7 @@ void start() {
174174
final DiscoveryNode node = nodes[i];
175175
final String nodeId = node.getId();
176176
try {
177-
TransportRequest nodeRequest = newNodeRequest(nodeId, request);
177+
TransportRequest nodeRequest = newNodeRequest(request);
178178
if (task != null) {
179179
nodeRequest.setParentTask(clusterService.localNode().getId(), task.getId());
180180
}

server/src/main/java/org/elasticsearch/gateway/TransportNodesListGatewayMetaState.java

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ public ActionFuture<NodesGatewayMetaState> list(String[] nodesIds, @Nullable Tim
6767
}
6868

6969
@Override
70-
protected NodeRequest newNodeRequest(String nodeId, Request request) {
71-
return new NodeRequest(nodeId);
70+
protected NodeRequest newNodeRequest(Request request) {
71+
return new NodeRequest();
7272
}
7373

7474
@Override
@@ -114,14 +114,6 @@ protected void writeNodesTo(StreamOutput out, List<NodeGatewayMetaState> nodes)
114114
}
115115

116116
public static class NodeRequest extends BaseNodeRequest {
117-
118-
public NodeRequest() {
119-
}
120-
121-
NodeRequest(String nodeId) {
122-
super(nodeId);
123-
}
124-
125117
}
126118

127119
public static class NodeGatewayMetaState extends BaseNodeResponse {

0 commit comments

Comments
 (0)