Skip to content

Commit 1b36ee4

Browse files
gargharsh3134gargharsh3134
authored
Retaining the old constructors for classes marked as API changed as part of opensearch-project#12333 (opensearch-project#13926)
* Retaining the old constructors for classes marked as API changed as part of opensearch-project#12333 --------- Signed-off-by: Harsh Garg <gkharsh@amazon.com> Co-authored-by: Harsh Garg <gkharsh@amazon.com>
1 parent 05af414 commit 1b36ee4

8 files changed

+103
-70
lines changed

‎server/src/main/java/org/opensearch/cluster/service/ClusterApplierService.java

+5
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
import org.opensearch.common.util.concurrent.PrioritizedOpenSearchThreadPoolExecutor;
6363
import org.opensearch.common.util.concurrent.ThreadContext;
6464
import org.opensearch.core.concurrency.OpenSearchRejectedExecutionException;
65+
import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
6566
import org.opensearch.telemetry.metrics.tags.Tags;
6667
import org.opensearch.threadpool.Scheduler;
6768
import org.opensearch.threadpool.ThreadPool;
@@ -125,6 +126,10 @@ public class ClusterApplierService extends AbstractLifecycleComponent implements
125126
private NodeConnectionsService nodeConnectionsService;
126127
private final ClusterManagerMetrics clusterManagerMetrics;
127128

129+
public ClusterApplierService(String nodeName, Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
130+
this(nodeName, settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE));
131+
}
132+
128133
public ClusterApplierService(
129134
String nodeName,
130135
Settings settings,

‎server/src/main/java/org/opensearch/cluster/service/ClusterManagerService.java

+6
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.opensearch.common.annotation.PublicApi;
1313
import org.opensearch.common.settings.ClusterSettings;
1414
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
1516
import org.opensearch.threadpool.ThreadPool;
1617

1718
/**
@@ -21,6 +22,11 @@
2122
*/
2223
@PublicApi(since = "2.2.0")
2324
public class ClusterManagerService extends MasterService {
25+
26+
public ClusterManagerService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
27+
super(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE));
28+
}
29+
2430
public ClusterManagerService(
2531
Settings settings,
2632
ClusterSettings clusterSettings,

‎server/src/main/java/org/opensearch/cluster/service/ClusterService.java

+5
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.opensearch.common.settings.Settings;
5555
import org.opensearch.index.IndexingPressureService;
5656
import org.opensearch.node.Node;
57+
import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
5758
import org.opensearch.threadpool.ThreadPool;
5859

5960
import java.util.Collections;
@@ -92,6 +93,10 @@ public class ClusterService extends AbstractLifecycleComponent {
9293

9394
private IndexingPressureService indexingPressureService;
9495

96+
public ClusterService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
97+
this(settings, clusterSettings, threadPool, new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE));
98+
}
99+
95100
public ClusterService(
96101
Settings settings,
97102
ClusterSettings clusterSettings,

‎server/src/test/java/org/opensearch/cluster/InternalClusterInfoServiceSchedulingTests.java

+1-8
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@
5454
import org.opensearch.core.action.ActionListener;
5555
import org.opensearch.core.action.ActionResponse;
5656
import org.opensearch.node.Node;
57-
import org.opensearch.telemetry.metrics.noop.NoopMetricsRegistry;
5857
import org.opensearch.test.ClusterServiceUtils;
5958
import org.opensearch.test.OpenSearchTestCase;
6059
import org.opensearch.test.client.NoOpClient;
@@ -84,13 +83,7 @@ public void testScheduling() {
8483
final DeterministicTaskQueue deterministicTaskQueue = new DeterministicTaskQueue(settings, random());
8584
final ThreadPool threadPool = deterministicTaskQueue.getThreadPool();
8685

87-
final ClusterApplierService clusterApplierService = new ClusterApplierService(
88-
"test",
89-
settings,
90-
clusterSettings,
91-
threadPool,
92-
new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)
93-
) {
86+
final ClusterApplierService clusterApplierService = new ClusterApplierService("test", settings, clusterSettings, threadPool) {
9487
@Override
9588
protected PrioritizedOpenSearchThreadPoolExecutor createThreadPoolExecutor() {
9689
return new MockSinglePrioritizingExecutor("mock-executor", deterministicTaskQueue, threadPool);

‎server/src/test/java/org/opensearch/cluster/service/ClusterApplierServiceTests.java

+54-40
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import java.util.Collections;
6868
import java.util.List;
6969
import java.util.Map;
70+
import java.util.Optional;
7071
import java.util.concurrent.CountDownLatch;
7172
import java.util.concurrent.atomic.AtomicBoolean;
7273
import java.util.concurrent.atomic.AtomicReference;
@@ -92,15 +93,15 @@ public class ClusterApplierServiceTests extends OpenSearchTestCase {
9293
private static ThreadPool threadPool;
9394
private TimedClusterApplierService clusterApplierService;
9495
private static MetricsRegistry metricsRegistry;
95-
private static Histogram applierslatenctHistogram;
96-
private static Histogram listenerslatenctHistogram;
96+
private static Histogram applierslatencyHistogram;
97+
private static Histogram listenerslatencyHistogram;
9798

9899
@BeforeClass
99100
public static void createThreadPool() {
100101
threadPool = new TestThreadPool(ClusterApplierServiceTests.class.getName());
101102
metricsRegistry = mock(MetricsRegistry.class);
102-
applierslatenctHistogram = mock(Histogram.class);
103-
listenerslatenctHistogram = mock(Histogram.class);
103+
applierslatencyHistogram = mock(Histogram.class);
104+
listenerslatencyHistogram = mock(Histogram.class);
104105
}
105106

106107
@AfterClass
@@ -117,11 +118,11 @@ public void setUp() throws Exception {
117118
when(metricsRegistry.createHistogram(anyString(), anyString(), anyString())).thenAnswer(invocationOnMock -> {
118119
String histogramName = (String) invocationOnMock.getArguments()[0];
119120
if (histogramName.contains("appliers.latency")) {
120-
return applierslatenctHistogram;
121+
return applierslatencyHistogram;
121122
}
122-
return listenerslatenctHistogram;
123+
return listenerslatencyHistogram;
123124
});
124-
clusterApplierService = createTimedClusterService(true);
125+
clusterApplierService = createTimedClusterService(true, Optional.of(metricsRegistry));
125126
}
126127

127128
@After
@@ -130,14 +131,26 @@ public void tearDown() throws Exception {
130131
super.tearDown();
131132
}
132133

133-
private TimedClusterApplierService createTimedClusterService(boolean makeClusterManager) {
134+
private TimedClusterApplierService createTimedClusterService(
135+
boolean makeClusterManager,
136+
Optional<MetricsRegistry> metricsRegistryOptional
137+
) {
134138
DiscoveryNode localNode = new DiscoveryNode("node1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT);
135-
TimedClusterApplierService timedClusterApplierService = new TimedClusterApplierService(
136-
Settings.builder().put("cluster.name", "ClusterApplierServiceTests").build(),
137-
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
138-
threadPool,
139-
new ClusterManagerMetrics(metricsRegistry)
140-
);
139+
TimedClusterApplierService timedClusterApplierService;
140+
if (metricsRegistryOptional != null && metricsRegistryOptional.isPresent()) {
141+
timedClusterApplierService = new TimedClusterApplierService(
142+
Settings.builder().put("cluster.name", "ClusterApplierServiceTests").build(),
143+
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
144+
threadPool,
145+
new ClusterManagerMetrics(metricsRegistry)
146+
);
147+
} else {
148+
timedClusterApplierService = new TimedClusterApplierService(
149+
Settings.builder().put("cluster.name", "ClusterApplierServiceTests").build(),
150+
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
151+
threadPool
152+
);
153+
}
141154
timedClusterApplierService.setNodeConnectionsService(createNoOpNodeConnectionsService());
142155
timedClusterApplierService.setInitialState(
143156
ClusterState.builder(new ClusterName("ClusterApplierServiceTests"))
@@ -220,8 +233,8 @@ public void onFailure(String source, Exception e) {
220233
});
221234
assertBusy(mockAppender::assertAllExpectationsMatched);
222235
}
223-
verifyNoInteractions(applierslatenctHistogram);
224-
verifyNoInteractions(listenerslatenctHistogram);
236+
verifyNoInteractions(applierslatencyHistogram);
237+
verifyNoInteractions(listenerslatencyHistogram);
225238
}
226239

227240
@TestLogging(value = "org.opensearch.cluster.service:WARN", reason = "to ensure that we log cluster state events on WARN level")
@@ -319,12 +332,12 @@ public void onFailure(String source, Exception e) {
319332
latch.await();
320333
mockAppender.assertAllExpectationsMatched();
321334
}
322-
verifyNoInteractions(applierslatenctHistogram);
323-
verifyNoInteractions(listenerslatenctHistogram);
335+
verifyNoInteractions(applierslatencyHistogram);
336+
verifyNoInteractions(listenerslatencyHistogram);
324337
}
325338

326339
public void testLocalNodeClusterManagerListenerCallbacks() {
327-
TimedClusterApplierService timedClusterApplierService = createTimedClusterService(false);
340+
TimedClusterApplierService timedClusterApplierService = createTimedClusterService(false, Optional.empty());
328341

329342
AtomicBoolean isClusterManager = new AtomicBoolean();
330343
timedClusterApplierService.addLocalNodeClusterManagerListener(new LocalNodeClusterManagerListener() {
@@ -359,9 +372,7 @@ public void offClusterManager() {
359372
setState(timedClusterApplierService, state);
360373
assertThat(isClusterManager.get(), is(true));
361374

362-
verify(listenerslatenctHistogram, atLeastOnce()).record(anyDouble(), any());
363-
clearInvocations(listenerslatenctHistogram);
364-
verifyNoInteractions(applierslatenctHistogram);
375+
verifyNoInteractions(applierslatencyHistogram, listenerslatencyHistogram);
365376

366377
timedClusterApplierService.close();
367378
}
@@ -372,7 +383,7 @@ public void offClusterManager() {
372383
* To support inclusive language, LocalNodeMasterListener is deprecated in 2.2.
373384
*/
374385
public void testDeprecatedLocalNodeMasterListenerCallbacks() {
375-
TimedClusterApplierService timedClusterApplierService = createTimedClusterService(false);
386+
TimedClusterApplierService timedClusterApplierService = createTimedClusterService(false, Optional.empty());
376387

377388
AtomicBoolean isClusterManager = new AtomicBoolean();
378389
timedClusterApplierService.addLocalNodeMasterListener(new LocalNodeMasterListener() {
@@ -400,9 +411,7 @@ public void offMaster() {
400411
setState(timedClusterApplierService, state);
401412
assertThat(isClusterManager.get(), is(false));
402413

403-
verify(listenerslatenctHistogram, atLeastOnce()).record(anyDouble(), any());
404-
clearInvocations(listenerslatenctHistogram);
405-
verifyNoInteractions(applierslatenctHistogram);
414+
verifyNoInteractions(applierslatencyHistogram, listenerslatencyHistogram);
406415

407416
timedClusterApplierService.close();
408417
}
@@ -444,9 +453,9 @@ public void onFailure(String source, Exception e) {
444453
assertNull(error.get());
445454
assertTrue(applierCalled.get());
446455

447-
verify(applierslatenctHistogram, atLeastOnce()).record(anyDouble(), any());
448-
clearInvocations(applierslatenctHistogram);
449-
verifyNoInteractions(listenerslatenctHistogram);
456+
verify(applierslatencyHistogram, atLeastOnce()).record(anyDouble(), any());
457+
clearInvocations(applierslatencyHistogram);
458+
verifyNoInteractions(listenerslatencyHistogram);
450459
}
451460

452461
public void testClusterStateApplierBubblesUpExceptionsInApplier() throws InterruptedException {
@@ -478,8 +487,8 @@ public void onFailure(String source, Exception e) {
478487
assertNotNull(error.get());
479488
assertThat(error.get().getMessage(), containsString("dummy exception"));
480489

481-
verifyNoInteractions(applierslatenctHistogram);
482-
verifyNoInteractions(listenerslatenctHistogram);
490+
verifyNoInteractions(applierslatencyHistogram);
491+
verifyNoInteractions(listenerslatencyHistogram);
483492
}
484493

485494
public void testClusterStateApplierBubblesUpExceptionsInSettingsApplier() throws InterruptedException {
@@ -524,8 +533,8 @@ public void onFailure(String source, Exception e) {
524533
assertNotNull(error.get());
525534
assertThat(error.get().getMessage(), containsString("illegal value can't update"));
526535

527-
verifyNoInteractions(applierslatenctHistogram);
528-
verifyNoInteractions(listenerslatenctHistogram);
536+
verifyNoInteractions(applierslatencyHistogram);
537+
verifyNoInteractions(listenerslatencyHistogram);
529538
}
530539

531540
public void testClusterStateApplierSwallowsExceptionInListener() throws InterruptedException {
@@ -558,8 +567,8 @@ public void onFailure(String source, Exception e) {
558567
assertNull(error.get());
559568
assertTrue(applierCalled.get());
560569

561-
verifyNoInteractions(applierslatenctHistogram);
562-
verifyNoInteractions(listenerslatenctHistogram);
570+
verifyNoInteractions(applierslatencyHistogram);
571+
verifyNoInteractions(listenerslatencyHistogram);
563572
}
564573

565574
public void testClusterStateApplierCanCreateAnObserver() throws InterruptedException {
@@ -617,9 +626,9 @@ public void onFailure(String source, Exception e) {
617626
assertNull(error.get());
618627
assertTrue(applierCalled.get());
619628

620-
verify(applierslatenctHistogram, atLeastOnce()).record(anyDouble(), any());
621-
clearInvocations(applierslatenctHistogram);
622-
verifyNoInteractions(listenerslatenctHistogram);
629+
verify(applierslatencyHistogram, atLeastOnce()).record(anyDouble(), any());
630+
clearInvocations(applierslatencyHistogram);
631+
verifyNoInteractions(listenerslatencyHistogram);
623632
}
624633

625634
public void testThreadContext() throws InterruptedException {
@@ -665,8 +674,8 @@ public void onFailure(String source, Exception e) {
665674

666675
latch.await();
667676

668-
verifyNoInteractions(applierslatenctHistogram);
669-
verifyNoInteractions(listenerslatenctHistogram);
677+
verifyNoInteractions(applierslatencyHistogram);
678+
verifyNoInteractions(listenerslatencyHistogram);
670679
}
671680

672681
static class TimedClusterApplierService extends ClusterApplierService {
@@ -675,6 +684,11 @@ static class TimedClusterApplierService extends ClusterApplierService {
675684
volatile Long currentTimeOverride = null;
676685
boolean applicationMayFail;
677686

687+
TimedClusterApplierService(Settings settings, ClusterSettings clusterSettings, ThreadPool threadPool) {
688+
super("test_node", settings, clusterSettings, threadPool);
689+
this.clusterSettings = clusterSettings;
690+
}
691+
678692
TimedClusterApplierService(
679693
Settings settings,
680694
ClusterSettings clusterSettings,

‎server/src/test/java/org/opensearch/cluster/service/ClusterServiceTests.java

+2-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import org.opensearch.common.settings.ClusterSettings;
1212
import org.opensearch.common.settings.Settings;
13-
import org.opensearch.test.ClusterServiceUtils;
1413
import org.opensearch.test.OpenSearchTestCase;
1514
import org.opensearch.threadpool.TestThreadPool;
1615
import org.junit.After;
@@ -27,11 +26,11 @@ public void terminateThreadPool() {
2726

2827
public void testDeprecatedGetMasterServiceBWC() {
2928
try (
30-
ClusterService clusterService = ClusterServiceUtils.createClusterService(
29+
ClusterService clusterService = new ClusterService(
3130
Settings.EMPTY,
3231
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
3332
threadPool
34-
);
33+
)
3534
) {
3635
MasterService masterService = clusterService.getMasterService();
3736
ClusterManagerService clusterManagerService = clusterService.getClusterManagerService();

‎server/src/test/java/org/opensearch/cluster/service/MasterServiceTests.java

+29-12
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import java.util.HashSet;
8181
import java.util.List;
8282
import java.util.Map;
83+
import java.util.Optional;
8384
import java.util.Set;
8485
import java.util.concurrent.BrokenBarrierException;
8586
import java.util.concurrent.ConcurrentHashMap;
@@ -136,20 +137,36 @@ public void randomizeCurrentTime() {
136137
}
137138

138139
private ClusterManagerService createClusterManagerService(boolean makeClusterManager) {
139-
return createClusterManagerService(makeClusterManager, NoopMetricsRegistry.INSTANCE);
140+
return createClusterManagerService(makeClusterManager, Optional.empty());
140141
}
141142

142-
private ClusterManagerService createClusterManagerService(boolean makeClusterManager, MetricsRegistry metricsRegistry) {
143+
private ClusterManagerService createClusterManagerService(
144+
boolean makeClusterManager,
145+
Optional<MetricsRegistry> metricsRegistryOptional
146+
) {
143147
final DiscoveryNode localNode = new DiscoveryNode("node1", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT);
144-
final ClusterManagerService clusterManagerService = new ClusterManagerService(
145-
Settings.builder()
146-
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), MasterServiceTests.class.getSimpleName())
147-
.put(Node.NODE_NAME_SETTING.getKey(), "test_node")
148-
.build(),
149-
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
150-
threadPool,
151-
new ClusterManagerMetrics(metricsRegistry)
152-
);
148+
final ClusterManagerService clusterManagerService;
149+
if (metricsRegistryOptional != null && metricsRegistryOptional.isPresent()) {
150+
clusterManagerService = new ClusterManagerService(
151+
Settings.builder()
152+
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), MasterServiceTests.class.getSimpleName())
153+
.put(Node.NODE_NAME_SETTING.getKey(), "test_node")
154+
.build(),
155+
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
156+
threadPool,
157+
new ClusterManagerMetrics(metricsRegistryOptional.get())
158+
);
159+
} else {
160+
clusterManagerService = new ClusterManagerService(
161+
Settings.builder()
162+
.put(ClusterName.CLUSTER_NAME_SETTING.getKey(), MasterServiceTests.class.getSimpleName())
163+
.put(Node.NODE_NAME_SETTING.getKey(), "test_node")
164+
.build(),
165+
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
166+
threadPool
167+
);
168+
}
169+
153170
final ClusterState initialClusterState = ClusterState.builder(new ClusterName(MasterServiceTests.class.getSimpleName()))
154171
.nodes(
155172
DiscoveryNodes.builder()
@@ -181,7 +198,7 @@ public void testClusterManagerAwareExecution() throws Exception {
181198
return clusterStatePublishHistogram;
182199
});
183200

184-
final ClusterManagerService nonClusterManager = createClusterManagerService(false, metricsRegistry);
201+
final ClusterManagerService nonClusterManager = createClusterManagerService(false, Optional.of(metricsRegistry));
185202

186203
final boolean[] taskFailed = { false };
187204
final CountDownLatch latch1 = new CountDownLatch(1);

‎server/src/test/java/org/opensearch/snapshots/SnapshotResiliencyTests.java

+1-7
Original file line numberDiff line numberDiff line change
@@ -1921,13 +1921,7 @@ private final class TestClusterNode {
19211921
settings,
19221922
clusterSettings,
19231923
clusterManagerService,
1924-
new ClusterApplierService(
1925-
node.getName(),
1926-
settings,
1927-
clusterSettings,
1928-
threadPool,
1929-
new ClusterManagerMetrics(NoopMetricsRegistry.INSTANCE)
1930-
) {
1924+
new ClusterApplierService(node.getName(), settings, clusterSettings, threadPool) {
19311925
@Override
19321926
protected PrioritizedOpenSearchThreadPoolExecutor createThreadPoolExecutor() {
19331927
return new MockSinglePrioritizingExecutor(node.getName(), deterministicTaskQueue, threadPool);

0 commit comments

Comments
 (0)