Skip to content

Commit af19101

Browse files
Cleanup TransportRequestOptions Usage (#65248) (#65257)
A class with 2 fields does not need a builder, especially when in many cases the builder result is just equivalent to the `EMPTY` singleton to begin with. Removed the builder and simplified related code accordingly.
1 parent b20aa80 commit af19101

File tree

36 files changed

+95
-148
lines changed

36 files changed

+95
-148
lines changed

server/src/main/java/org/elasticsearch/action/ActionType.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.elasticsearch.action;
2121

2222
import org.elasticsearch.common.io.stream.Writeable;
23-
import org.elasticsearch.common.settings.Settings;
2423
import org.elasticsearch.transport.TransportRequestOptions;
2524

2625
/**
@@ -57,7 +56,7 @@ public Writeable.Reader<Response> getResponseReader() {
5756
/**
5857
* Optional request options for the action.
5958
*/
60-
public TransportRequestOptions transportOptions(Settings settings) {
59+
public TransportRequestOptions transportOptions() {
6160
return TransportRequestOptions.EMPTY;
6261
}
6362

server/src/main/java/org/elasticsearch/action/TransportActionNodeProxy.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
package org.elasticsearch.action;
2121

2222
import org.elasticsearch.cluster.node.DiscoveryNode;
23-
import org.elasticsearch.common.settings.Settings;
2423
import org.elasticsearch.transport.TransportRequestOptions;
2524
import org.elasticsearch.transport.TransportService;
2625

@@ -33,10 +32,10 @@ public class TransportActionNodeProxy<Request extends ActionRequest, Response ex
3332
private final ActionType<Response> action;
3433
private final TransportRequestOptions transportOptions;
3534

36-
public TransportActionNodeProxy(Settings settings, ActionType<Response> action, TransportService transportService) {
35+
public TransportActionNodeProxy(ActionType<Response> action, TransportService transportService) {
3736
this.action = action;
3837
this.transportService = transportService;
39-
this.transportOptions = action.transportOptions(settings);
38+
this.transportOptions = action.transportOptions();
4039
}
4140

4241
public void execute(final DiscoveryNode node, final Request request, final ActionListener<Response> listener) {

server/src/main/java/org/elasticsearch/action/admin/cluster/node/tasks/get/TransportGetTaskAction.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,6 @@ protected void doExecute(Task thisTask, GetTaskRequest request, ActionListener<G
9696
* {@link #getFinishedTaskFromIndex(Task, GetTaskRequest, ActionListener)} on this node.
9797
*/
9898
private void runOnNodeWithTaskIfPossible(Task thisTask, GetTaskRequest request, ActionListener<GetTaskResponse> listener) {
99-
TransportRequestOptions.Builder builder = TransportRequestOptions.builder();
100-
if (request.getTimeout() != null) {
101-
builder.withTimeout(request.getTimeout());
102-
}
10399
DiscoveryNode node = clusterService.state().nodes().get(request.getTaskId().getNodeId());
104100
if (node == null) {
105101
// Node is no longer part of the cluster! Try and look the task up from the results index.
@@ -115,7 +111,7 @@ private void runOnNodeWithTaskIfPossible(Task thisTask, GetTaskRequest request,
115111
return;
116112
}
117113
GetTaskRequest nodeRequest = request.nodeRequest(clusterService.localNode().getId(), thisTask.getId());
118-
transportService.sendRequest(node, GetTaskAction.NAME, nodeRequest, builder.build(),
114+
transportService.sendRequest(node, GetTaskAction.NAME, nodeRequest, TransportRequestOptions.timeout(request.getTimeout()),
119115
new ActionListenerResponseHandler<>(listener, GetTaskResponse::new, ThreadPool.Names.SAME));
120116
}
121117

server/src/main/java/org/elasticsearch/action/bulk/BulkAction.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,22 @@
2020
package org.elasticsearch.action.bulk;
2121

2222
import org.elasticsearch.action.ActionType;
23-
import org.elasticsearch.common.settings.Settings;
2423
import org.elasticsearch.transport.TransportRequestOptions;
2524

2625
public class BulkAction extends ActionType<BulkResponse> {
2726

2827
public static final BulkAction INSTANCE = new BulkAction();
2928
public static final String NAME = "indices:data/write/bulk";
3029

30+
private static final TransportRequestOptions TRANSPORT_REQUEST_OPTIONS =
31+
TransportRequestOptions.of(null, TransportRequestOptions.Type.BULK);
32+
3133
private BulkAction() {
3234
super(NAME, BulkResponse::new);
3335
}
3436

3537
@Override
36-
public TransportRequestOptions transportOptions(Settings settings) {
37-
return TransportRequestOptions.builder().withType(TransportRequestOptions.Type.BULK).build();
38+
public TransportRequestOptions transportOptions() {
39+
return TRANSPORT_REQUEST_OPTIONS;
3840
}
3941
}

server/src/main/java/org/elasticsearch/action/bulk/TransportShardBulkAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ public TransportShardBulkAction(Settings settings, TransportService transportSer
110110
}
111111

112112
@Override
113-
protected TransportRequestOptions transportOptions(Settings settings) {
114-
return BulkAction.INSTANCE.transportOptions(settings);
113+
protected TransportRequestOptions transportOptions() {
114+
return BulkAction.INSTANCE.transportOptions();
115115
}
116116

117117
@Override

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,10 +200,7 @@ void start() {
200200
threadPool.generic().execute(() -> listener.onResponse(newResponse(request, responses)));
201201
return;
202202
}
203-
TransportRequestOptions.Builder builder = TransportRequestOptions.builder();
204-
if (request.timeout() != null) {
205-
builder.withTimeout(request.timeout());
206-
}
203+
final TransportRequestOptions transportRequestOptions = TransportRequestOptions.timeout(request.timeout());
207204
for (int i = 0; i < nodes.length; i++) {
208205
final int idx = i;
209206
final DiscoveryNode node = nodes[i];
@@ -214,7 +211,7 @@ void start() {
214211
nodeRequest.setParentTask(clusterService.localNode().getId(), task.getId());
215212
}
216213

217-
transportService.sendRequest(node, getTransportNodeAction(node), nodeRequest, builder.build(),
214+
transportService.sendRequest(node, getTransportNodeAction(node), nodeRequest, transportRequestOptions,
218215
new TransportResponseHandler<NodeResponse>() {
219216
@Override
220217
public NodeResponse read(StreamInput in) throws IOException {

server/src/main/java/org/elasticsearch/action/support/replication/TransportReplicationAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ protected TransportReplicationAction(Settings settings, String actionName, Trans
170170
transportService.registerRequestHandler(transportReplicaAction, executor, true, true,
171171
in -> new ConcreteReplicaRequest<>(replicaRequestReader, in), this::handleReplicaRequest);
172172

173-
this.transportOptions = transportOptions(settings);
173+
this.transportOptions = transportOptions();
174174

175175
this.syncGlobalCheckpointAfterOperation = syncGlobalCheckpointAfterOperation;
176176

@@ -249,7 +249,7 @@ public ClusterBlockLevel indexBlockLevel() {
249249
return null;
250250
}
251251

252-
protected TransportRequestOptions transportOptions(Settings settings) {
252+
protected TransportRequestOptions transportOptions() {
253253
return TransportRequestOptions.EMPTY;
254254
}
255255

server/src/main/java/org/elasticsearch/action/support/tasks/TransportTasksAction.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,7 @@ private void start() {
246246
listener.onFailure(e);
247247
}
248248
} else {
249-
TransportRequestOptions.Builder builder = TransportRequestOptions.builder();
250-
if (request.getTimeout() != null) {
251-
builder.withTimeout(request.getTimeout());
252-
}
249+
final TransportRequestOptions transportRequestOptions = TransportRequestOptions.timeout(request.getTimeout());
253250
for (int i = 0; i < nodesIds.length; i++) {
254251
final String nodeId = nodesIds[i];
255252
final int idx = i;
@@ -260,7 +257,7 @@ private void start() {
260257
} else {
261258
NodeTaskRequest nodeRequest = new NodeTaskRequest(request);
262259
nodeRequest.setParentTask(clusterService.localNode().getId(), task.getId());
263-
transportService.sendRequest(node, transportNodeAction, nodeRequest, builder.build(),
260+
transportService.sendRequest(node, transportNodeAction, nodeRequest, transportRequestOptions,
264261
new TransportResponseHandler<NodeTasksResponse>() {
265262
@Override
266263
public NodeTasksResponse read(StreamInput in) throws IOException {

server/src/main/java/org/elasticsearch/client/transport/TransportClient.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ private static ClientTemplate buildTemplate(Settings providedSettings, Settings
223223
final List<? extends ActionType<?>> baseActions =
224224
actionModule.getActions().values().stream().map(ActionPlugin.ActionHandler::getAction).collect(Collectors.toList());
225225
clientActions.addAll(baseActions);
226-
final TransportProxyClient proxy = new TransportProxyClient(settings, transportService, nodesService, clientActions);
226+
final TransportProxyClient proxy = new TransportProxyClient(transportService, nodesService, clientActions);
227227

228228
List<LifecycleComponent> pluginLifecycleComponents = new ArrayList<>(pluginsService.getGuiceServiceClasses().stream()
229229
.map(injector::getInstance).collect(Collectors.toList()));

server/src/main/java/org/elasticsearch/client/transport/TransportClientNodesService.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ final class TransportClientNodesService implements Closeable {
7676

7777
private final TimeValue nodesSamplerInterval;
7878

79-
private final long pingTimeout;
79+
private final TimeValue pingTimeout;
8080

8181
private final ClusterName clusterName;
8282

@@ -132,7 +132,7 @@ final class TransportClientNodesService implements Closeable {
132132
this.minCompatibilityVersion = Version.CURRENT.minimumCompatibilityVersion();
133133

134134
this.nodesSamplerInterval = TransportClient.CLIENT_TRANSPORT_NODES_SAMPLER_INTERVAL.get(settings);
135-
this.pingTimeout = TransportClient.CLIENT_TRANSPORT_PING_TIMEOUT.get(settings).millis();
135+
this.pingTimeout = TransportClient.CLIENT_TRANSPORT_PING_TIMEOUT.get(settings);
136136
this.ignoreClusterName = TransportClient.CLIENT_TRANSPORT_IGNORE_CLUSTER_NAME.get(settings);
137137

138138
if (logger.isDebugEnabled()) {
@@ -417,7 +417,7 @@ public LivenessResponse read(StreamInput in) throws IOException {
417417
}
418418
});
419419
transportService.sendRequest(connection, TransportLivenessAction.NAME, new LivenessRequest(),
420-
TransportRequestOptions.builder().withType(TransportRequestOptions.Type.STATE).withTimeout(pingTimeout).build(),
420+
TransportRequestOptions.of(pingTimeout, TransportRequestOptions.Type.STATE),
421421
handler);
422422
final LivenessResponse livenessResponse = handler.txGet();
423423
if (!ignoreClusterName && !clusterName.equals(livenessResponse.getClusterName())) {
@@ -507,8 +507,7 @@ protected void doRun() throws Exception {
507507
}
508508
transportService.sendRequest(pingConnection, ClusterStateAction.NAME,
509509
Requests.clusterStateRequest().clear().nodes(true).local(true),
510-
TransportRequestOptions.builder().withType(TransportRequestOptions.Type.STATE)
511-
.withTimeout(pingTimeout).build(),
510+
TransportRequestOptions.of(pingTimeout, TransportRequestOptions.Type.STATE),
512511
new TransportResponseHandler<ClusterStateResponse>() {
513512

514513
@Override

0 commit comments

Comments
 (0)