Skip to content

Commit 10295b3

Browse files
authored
Core: Drop nodeName from AbstractComponent (#34487)
`AbstractComponent` is trouble because its name implies that *everything* should extend from it. It *is* useful, but maybe too broadly useful. The things it offers access too, the `Settings` instance for the entire server and a logger are nice to have around, but not really needed *everywhere*. The `Settings` instance especially adds a fair bit of ceremony to testing without any value. This removes the `nodeName` method from `AbstractComponent` so it is more clear where we actually need the node name.
1 parent 5e0b524 commit 10295b3

File tree

12 files changed

+38
-22
lines changed

12 files changed

+38
-22
lines changed

server/src/main/java/org/elasticsearch/cluster/service/ClusterApplierService.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,18 @@ public class ClusterApplierService extends AbstractLifecycleComponent implements
9595

9696
private final AtomicReference<ClusterState> state; // last applied state
9797

98+
private final String nodeName;
99+
98100
private NodeConnectionsService nodeConnectionsService;
99101

100-
public ClusterApplierService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
102+
public ClusterApplierService(String nodeName, Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
101103
super(settings);
102104
this.clusterSettings = clusterSettings;
103105
this.threadPool = threadPool;
104106
this.state = new AtomicReference<>();
105107
this.slowTaskLoggingThreshold = CLUSTER_SERVICE_SLOW_TASK_LOGGING_THRESHOLD_SETTING.get(settings);
106108
this.localNodeMasterListeners = new LocalNodeMasterListeners(threadPool);
109+
this.nodeName = nodeName;
107110
}
108111

109112
public void setSlowTaskLoggingThreshold(TimeValue slowTaskLoggingThreshold) {
@@ -130,7 +133,7 @@ protected synchronized void doStart() {
130133
Objects.requireNonNull(state.get(), "please set initial state before starting");
131134
addListener(localNodeMasterListeners);
132135
threadPoolExecutor = EsExecutors.newSinglePrioritizing(
133-
nodeName() + "/" + CLUSTER_UPDATE_THREAD_NAME,
136+
nodeName + "/" + CLUSTER_UPDATE_THREAD_NAME,
134137
daemonThreadFactory(settings, CLUSTER_UPDATE_THREAD_NAME),
135138
threadPool.getThreadContext(),
136139
threadPool.scheduler());

server/src/main/java/org/elasticsearch/cluster/service/ClusterService.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.common.settings.Setting.Property;
3737
import org.elasticsearch.common.settings.Settings;
3838
import org.elasticsearch.common.unit.TimeValue;
39+
import org.elasticsearch.node.Node;
3940
import org.elasticsearch.threadpool.ThreadPool;
4041

4142
import java.util.Collections;
@@ -60,17 +61,20 @@ public class ClusterService extends AbstractLifecycleComponent {
6061

6162
private final ClusterSettings clusterSettings;
6263

64+
private final String nodeName;
65+
6366
public ClusterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
6467
super(settings);
65-
this.masterService = new MasterService(settings, threadPool);
68+
this.nodeName = Node.NODE_NAME_SETTING.get(settings);
69+
this.masterService = new MasterService(nodeName, settings, threadPool);
6670
this.operationRouting = new OperationRouting(settings, clusterSettings);
6771
this.clusterSettings = clusterSettings;
6872
this.clusterName = ClusterName.CLUSTER_NAME_SETTING.get(settings);
6973
this.clusterSettings.addSettingsUpdateConsumer(CLUSTER_SERVICE_SLOW_TASK_LOGGING_THRESHOLD_SETTING,
7074
this::setSlowTaskLoggingThreshold);
7175
// Add a no-op update consumer so changes are logged
7276
this.clusterSettings.addAffixUpdateConsumer(USER_DEFINED_META_DATA, (first, second) -> {}, (first, second) -> {});
73-
this.clusterApplierService = new ClusterApplierService(settings, clusterSettings, threadPool);
77+
this.clusterApplierService = new ClusterApplierService(nodeName, settings, clusterSettings, threadPool);
7478
}
7579

7680
private void setSlowTaskLoggingThreshold(TimeValue slowTaskLoggingThreshold) {
@@ -199,6 +203,13 @@ public Settings getSettings() {
199203
return settings;
200204
}
201205

206+
/**
207+
* The name of this node.
208+
*/
209+
public final String getNodeName() {
210+
return nodeName;
211+
}
212+
202213
/**
203214
* Submits a cluster state update task; unlike {@link #submitStateUpdateTask(String, Object, ClusterStateTaskConfig,
204215
* ClusterStateTaskExecutor, ClusterStateTaskListener)}, submitted updates will not be batched.

server/src/main/java/org/elasticsearch/cluster/service/MasterService.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ public class MasterService extends AbstractLifecycleComponent {
7070

7171
public static final String MASTER_UPDATE_THREAD_NAME = "masterService#updateTask";
7272

73+
private final String nodeName;
74+
7375
private BiConsumer<ClusterChangedEvent, Discovery.AckListener> clusterStatePublisher;
7476

7577
private java.util.function.Supplier<ClusterState> clusterStateSupplier;
@@ -81,8 +83,9 @@ public class MasterService extends AbstractLifecycleComponent {
8183
private volatile PrioritizedEsThreadPoolExecutor threadPoolExecutor;
8284
private volatile Batcher taskBatcher;
8385

84-
public MasterService(Settings settings, ThreadPool threadPool) {
86+
public MasterService(String nodeName, Settings settings, ThreadPool threadPool) {
8587
super(settings);
88+
this.nodeName = nodeName;
8689
// TODO: introduce a dedicated setting for master service
8790
this.slowTaskLoggingThreshold = CLUSTER_SERVICE_SLOW_TASK_LOGGING_THRESHOLD_SETTING.get(settings);
8891
this.threadPool = threadPool;
@@ -105,7 +108,7 @@ protected synchronized void doStart() {
105108
Objects.requireNonNull(clusterStatePublisher, "please set a cluster state publisher before starting");
106109
Objects.requireNonNull(clusterStateSupplier, "please set a cluster state supplier before starting");
107110
threadPoolExecutor = EsExecutors.newSinglePrioritizing(
108-
nodeName() + "/" + MASTER_UPDATE_THREAD_NAME,
111+
nodeName + "/" + MASTER_UPDATE_THREAD_NAME,
109112
daemonThreadFactory(settings, MASTER_UPDATE_THREAD_NAME),
110113
threadPool.getThreadContext(),
111114
threadPool.scheduler());

server/src/main/java/org/elasticsearch/common/component/AbstractComponent.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.apache.logging.log4j.Logger;
2424
import org.elasticsearch.common.logging.DeprecationLogger;
2525
import org.elasticsearch.common.settings.Settings;
26-
import org.elasticsearch.node.Node;
2726

2827
public abstract class AbstractComponent {
2928

@@ -36,11 +35,4 @@ public AbstractComponent(Settings settings) {
3635
this.deprecationLogger = new DeprecationLogger(logger);
3736
this.settings = settings;
3837
}
39-
40-
/**
41-
* Returns the nodes name from the settings or the empty string if not set.
42-
*/
43-
public final String nodeName() {
44-
return Node.NODE_NAME_SETTING.get(settings);
45-
}
4638
}

server/src/main/java/org/elasticsearch/discovery/zen/UnicastZenPing.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.elasticsearch.common.util.concurrent.EsExecutors;
4646
import org.elasticsearch.common.util.concurrent.EsThreadPoolExecutor;
4747
import org.elasticsearch.common.util.concurrent.KeyedLock;
48+
import org.elasticsearch.node.Node;
4849
import org.elasticsearch.tasks.Task;
4950
import org.elasticsearch.threadpool.ThreadPool;
5051
import org.elasticsearch.transport.ConnectTransportException;
@@ -117,6 +118,8 @@ public class UnicastZenPing extends AbstractComponent implements ZenPing {
117118

118119
private final TimeValue resolveTimeout;
119120

121+
private final String nodeName;
122+
120123
private volatile boolean closed = false;
121124

122125
public UnicastZenPing(Settings settings, ThreadPool threadPool, TransportService transportService,
@@ -131,6 +134,7 @@ public UnicastZenPing(Settings settings, ThreadPool threadPool, TransportService
131134
final int concurrentConnects = DISCOVERY_ZEN_PING_UNICAST_CONCURRENT_CONNECTS_SETTING.get(settings);
132135

133136
resolveTimeout = DISCOVERY_ZEN_PING_UNICAST_HOSTS_RESOLVE_TIMEOUT.get(settings);
137+
nodeName = Node.NODE_NAME_SETTING.get(settings);
134138
logger.debug(
135139
"using concurrent_connects [{}], resolve_timeout [{}]",
136140
concurrentConnects,
@@ -141,7 +145,7 @@ public UnicastZenPing(Settings settings, ThreadPool threadPool, TransportService
141145

142146
final ThreadFactory threadFactory = EsExecutors.daemonThreadFactory(settings, "[unicast_connect]");
143147
unicastZenPingExecutorService = EsExecutors.newScaling(
144-
nodeName() + "/" + "unicast_connect",
148+
nodeName + "/" + "unicast_connect",
145149
0,
146150
concurrentConnects,
147151
60,

server/src/main/java/org/elasticsearch/transport/TcpTransport.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@
6565
import org.elasticsearch.core.internal.io.IOUtils;
6666
import org.elasticsearch.indices.breaker.CircuitBreakerService;
6767
import org.elasticsearch.monitor.jvm.JvmInfo;
68+
import org.elasticsearch.node.Node;
6869
import org.elasticsearch.rest.RestStatus;
6970
import org.elasticsearch.threadpool.ThreadPool;
7071

@@ -209,6 +210,7 @@ public abstract class TcpTransport extends AbstractLifecycleComponent implements
209210
private final ResponseHandlers responseHandlers = new ResponseHandlers();
210211
private final TransportLogger transportLogger;
211212
private final BytesReference pingMessage;
213+
private final String nodeName;
212214

213215
public TcpTransport(String transportName, Settings settings, ThreadPool threadPool, BigArrays bigArrays,
214216
CircuitBreakerService circuitBreakerService, NamedWriteableRegistry namedWriteableRegistry,
@@ -223,6 +225,7 @@ public TcpTransport(String transportName, Settings settings, ThreadPool threadPo
223225
this.networkService = networkService;
224226
this.transportName = transportName;
225227
this.transportLogger = new TransportLogger();
228+
this.nodeName = Node.NODE_NAME_SETTING.get(settings);
226229

227230
final Settings defaultFeatures = DEFAULT_FEATURES_SETTING.get(settings);
228231
if (defaultFeatures == null) {
@@ -947,7 +950,7 @@ public void sendErrorResponse(
947950
stream.setVersion(nodeVersion);
948951
stream.setFeatures(features);
949952
RemoteTransportException tx = new RemoteTransportException(
950-
nodeName(), new TransportAddress(channel.getLocalAddress()), action, error);
953+
nodeName, new TransportAddress(channel.getLocalAddress()), action, error);
951954
threadPool.getThreadContext().writeTo(stream);
952955
stream.writeException(tx);
953956
byte status = 0;

server/src/test/java/org/elasticsearch/cluster/service/ClusterApplierServiceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ static class TimedClusterApplierService extends ClusterApplierService {
412412
public volatile Long currentTimeOverride = null;
413413

414414
TimedClusterApplierService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
415-
super(settings, clusterSettings, threadPool);
415+
super("test_node", settings, clusterSettings, threadPool);
416416
}
417417

418418
@Override

server/src/test/java/org/elasticsearch/cluster/service/MasterServiceTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -906,7 +906,7 @@ static class TimedMasterService extends MasterService {
906906
public volatile Long currentTimeOverride = null;
907907

908908
TimedMasterService(Settings settings, ThreadPool threadPool) {
909-
super(settings, threadPool);
909+
super("test_node", settings, threadPool);
910910
}
911911

912912
@Override

test/framework/src/main/java/org/elasticsearch/test/ClusterServiceUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
public class ClusterServiceUtils {
5151

5252
public static MasterService createMasterService(ThreadPool threadPool, ClusterState initialClusterState) {
53-
MasterService masterService = new MasterService(Settings.EMPTY, threadPool);
53+
MasterService masterService = new MasterService("test_master_node", Settings.EMPTY, threadPool);
5454
AtomicReference<ClusterState> clusterStateRef = new AtomicReference<>(initialClusterState);
5555
masterService.setClusterStatePublisher((event, ackListener) -> clusterStateRef.set(event.state()));
5656
masterService.setClusterStateSupplier(clusterStateRef::get);

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MachineLearning.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ public Collection<Object> createComponents(Client client, ClusterService cluster
363363
return emptyList();
364364
}
365365

366-
Auditor auditor = new Auditor(client, clusterService.nodeName());
366+
Auditor auditor = new Auditor(client, clusterService.getNodeName());
367367
JobResultsProvider jobResultsProvider = new JobResultsProvider(client, settings);
368368
UpdateJobProcessNotifier notifier = new UpdateJobProcessNotifier(settings, client, clusterService, threadPool);
369369
JobManager jobManager = new JobManager(env, settings, jobResultsProvider, clusterService, auditor, client, notifier);

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportDeleteExpiredDataAction.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ protected void doExecute(Task task, DeleteExpiredDataAction.Request request,
5555
}
5656

5757
private void deleteExpiredData(ActionListener<DeleteExpiredDataAction.Response> listener) {
58-
Auditor auditor = new Auditor(client, clusterService.nodeName());
58+
Auditor auditor = new Auditor(client, clusterService.getNodeName());
5959
List<MlDataRemover> dataRemovers = Arrays.asList(
6060
new ExpiredResultsRemover(client, clusterService, auditor),
6161
new ExpiredForecastsRemover(client, threadPool),

x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/job/UpdateJobProcessNotifier.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ void executeProcessUpdates(Iterator<UpdateHolder> updatesIterator) {
112112

113113
if (update.isJobUpdate() && clusterService.localNode().isMasterNode() == false) {
114114
assert clusterService.localNode().isMasterNode();
115-
LOGGER.error("Job update was submitted to non-master node [" + clusterService.nodeName() + "]; update for job ["
115+
LOGGER.error("Job update was submitted to non-master node [" + clusterService.getNodeName() + "]; update for job ["
116116
+ update.getJobId() + "] will be ignored");
117117
executeProcessUpdates(updatesIterator);
118118
return;

0 commit comments

Comments
 (0)