Skip to content

Commit 02a3533

Browse files
zacharymornzacharymorn
committed
Refactor out deprecated AddVotingConfigExclusionsRequest constructor
1 parent aaa0f89 commit 02a3533

8 files changed

Lines changed: 48 additions & 38 deletions

File tree

‎server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,6 @@ public class AddVotingConfigExclusionsRequest extends MasterNodeRequest<AddVotin
5454
private final String[] nodeNames;
5555
private final TimeValue timeout;
5656

57-
/**
58-
* Construct a request to add voting config exclusions for master-eligible nodes matching the given descriptions, and wait for a
59-
* default 30 seconds for these exclusions to take effect, removing the nodes from the voting configuration.
60-
* @param nodeDescriptions Descriptions of the nodes to add - see {@link DiscoveryNodes#resolveNodes(String...)}
61-
*/
62-
public AddVotingConfigExclusionsRequest(String[] nodeDescriptions) {
63-
this(nodeDescriptions, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
64-
}
65-
6657
/**
6758
* Construct a request to add voting config exclusions for master-eligible nodes matching the given descriptions, and wait for these
6859
* nodes to be removed from the voting configuration.

‎server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,15 @@ public void testResolve() {
110110
final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder()
111111
.add(localNode).add(otherNode1).add(otherNode2).add(otherDataNode).localNodeId(localNode.getId())).build();
112112

113-
assertThat(makeRequest("_all").resolveVotingConfigExclusions(clusterState),
113+
assertThat(makeRequestWithNodeDescriptions("_all").resolveVotingConfigExclusions(clusterState),
114114
containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion));
115-
assertThat(makeRequest("_local").resolveVotingConfigExclusions(clusterState),
115+
assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState),
116116
contains(localNodeExclusion));
117-
assertThat(makeRequest("other*").resolveVotingConfigExclusions(clusterState),
117+
assertThat(makeRequestWithNodeDescriptions("other*").resolveVotingConfigExclusions(clusterState),
118118
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
119119

120120
assertThat(expectThrows(IllegalArgumentException.class,
121-
() -> makeRequest("not-a-node").resolveVotingConfigExclusions(clusterState)).getMessage(),
121+
() -> makeRequestWithNodeDescriptions("not-a-node").resolveVotingConfigExclusions(clusterState)).getMessage(),
122122
equalTo("add voting config exclusions request for [not-a-node] matched no master-eligible nodes"));
123123
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
124124
}
@@ -316,16 +316,17 @@ public void testResolveAndCheckMaximum() {
316316
.coordinationMetaData(CoordinationMetaData.builder().addVotingConfigExclusion(otherNode1Exclusion).build()));
317317
final ClusterState clusterState = builder.build();
318318

319-
assertThat(makeRequest("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"),
319+
assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"),
320320
contains(localNodeExclusion));
321321
assertThat(expectThrows(IllegalArgumentException.class,
322-
() -> makeRequest("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 1, "setting.name")).getMessage(),
322+
() -> makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 1, "setting.name"))
323+
.getMessage(),
323324
equalTo("add voting config exclusions request for [_local] would add [1] exclusions to the existing [1] which would " +
324325
"exceed the maximum of [1] set by [setting.name]"));
325326
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
326327
}
327328

328-
private static AddVotingConfigExclusionsRequest makeRequest(String... descriptions) {
329-
return new AddVotingConfigExclusionsRequest(descriptions);
329+
private static AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... descriptions) {
330+
return new AddVotingConfigExclusionsRequest(descriptions, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
330331
}
331332
}

‎server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,8 @@ public void testWithdrawsVoteFromANode() throws InterruptedException {
140140

141141
clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
142142
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
143-
new AddVotingConfigExclusionsRequest(new String[]{"other1"}),
143+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
144+
new String[]{"other1"}, TimeValue.timeValueSeconds(30)),
144145
expectSuccess(r -> {
145146
assertNotNull(r);
146147
countDownLatch.countDown();
@@ -149,15 +150,15 @@ public void testWithdrawsVoteFromANode() throws InterruptedException {
149150

150151
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
151152
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
152-
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
153153
}
154154

155155
public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
156156
final CountDownLatch countDownLatch = new CountDownLatch(1);
157157

158158
clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
159159
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
160-
new AddVotingConfigExclusionsRequest(new String[]{"other1", "other2"}),
160+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
161+
new String[]{"other1", "other2"}, TimeValue.timeValueSeconds(30)),
161162
expectSuccess(r -> {
162163
assertNotNull(r);
163164
countDownLatch.countDown();
@@ -167,15 +168,15 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
167168
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
168169
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
169170
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
170-
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
171171
}
172172

173173
public void testWithdrawsVotesFromNodesMatchingWildcard() throws InterruptedException {
174174
final CountDownLatch countDownLatch = new CountDownLatch(1);
175175

176176
clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
177177
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
178-
new AddVotingConfigExclusionsRequest(new String[]{"other*"}),
178+
new AddVotingConfigExclusionsRequest(new String[]{"other*"}, Strings.EMPTY_ARRAY,
179+
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
179180
expectSuccess(r -> {
180181
assertNotNull(r);
181182
countDownLatch.countDown();
@@ -193,7 +194,8 @@ public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedExc
193194

194195
clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
195196
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
196-
new AddVotingConfigExclusionsRequest(new String[]{"_all"}),
197+
new AddVotingConfigExclusionsRequest(new String[]{"_all"}, Strings.EMPTY_ARRAY,
198+
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
197199
expectSuccess(r -> {
198200
assertNotNull(r);
199201
countDownLatch.countDown();
@@ -211,7 +213,8 @@ public void testWithdrawsVoteFromLocalNode() throws InterruptedException {
211213

212214
clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
213215
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
214-
new AddVotingConfigExclusionsRequest(new String[]{"_local"}),
216+
new AddVotingConfigExclusionsRequest(new String[]{"_local"}, Strings.EMPTY_ARRAY,
217+
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
215218
expectSuccess(r -> {
216219
assertNotNull(r);
217220
countDownLatch.countDown();
@@ -236,7 +239,7 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
236239

237240
// no observer to reconfigure
238241
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
239-
new AddVotingConfigExclusionsRequest(new String[]{"other1"}, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.ZERO),
242+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, new String[]{"other1"}, TimeValue.ZERO),
240243
expectSuccess(r -> {
241244
assertNotNull(r);
242245
countDownLatch.countDown();
@@ -246,15 +249,15 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
246249
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
247250
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
248251
contains(otherNode1Exclusion));
249-
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
250252
}
251253

252-
public void testReturnsErrorIfNoMatchingNodes() throws InterruptedException {
254+
public void testReturnsErrorIfNoMatchingNodesWithDeprecatedNodeDescriptions() throws InterruptedException {
253255
final CountDownLatch countDownLatch = new CountDownLatch(1);
254256
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();
255257

256258
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
257-
new AddVotingConfigExclusionsRequest(new String[]{"not-a-node"}),
259+
new AddVotingConfigExclusionsRequest(new String[]{"not-a-node"}, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
260+
TimeValue.timeValueSeconds(30)),
258261
expectError(e -> {
259262
exceptionHolder.set(e);
260263
countDownLatch.countDown();
@@ -274,7 +277,8 @@ public void testOnlyMatchesMasterEligibleNodes() throws InterruptedException {
274277
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();
275278

276279
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
277-
new AddVotingConfigExclusionsRequest(new String[]{"_all", "master:false"}),
280+
new AddVotingConfigExclusionsRequest(new String[]{"_all", "master:false"}, Strings.EMPTY_ARRAY,
281+
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
278282
expectError(e -> {
279283
exceptionHolder.set(e);
280284
countDownLatch.countDown();
@@ -372,7 +376,8 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce
372376
final CountDownLatch countDownLatch = new CountDownLatch(1);
373377

374378
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
375-
new AddVotingConfigExclusionsRequest(new String[]{"other1"}),
379+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
380+
new String[]{"other1"}, TimeValue.timeValueSeconds(30)),
376381
expectSuccess(r -> {
377382
assertNotNull(r);
378383
countDownLatch.countDown();
@@ -382,7 +387,6 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce
382387
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
383388
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
384389
contains(otherNode1Exclusion));
385-
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
386390
}
387391

388392
public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {
@@ -464,7 +468,8 @@ public void testReturnsErrorIfMaximumExclusionCountExceeded() throws Interrupted
464468
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();
465469

466470
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
467-
new AddVotingConfigExclusionsRequest(new String[]{"other*"}),
471+
new AddVotingConfigExclusionsRequest(new String[]{"other*"}, Strings.EMPTY_ARRAY,
472+
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
468473
expectError(e -> {
469474
exceptionHolder.set(e);
470475
countDownLatch.countDown();

‎server/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@
3131
import org.elasticsearch.cluster.node.DiscoveryNode;
3232
import org.elasticsearch.cluster.service.ClusterService;
3333
import org.elasticsearch.common.Priority;
34+
import org.elasticsearch.common.Strings;
3435
import org.elasticsearch.common.settings.Settings;
36+
import org.elasticsearch.common.unit.TimeValue;
3537
import org.elasticsearch.common.util.set.Sets;
3638
import org.elasticsearch.index.query.QueryBuilders;
3739
import org.elasticsearch.plugins.Plugin;
@@ -123,7 +125,8 @@ public void testTwoNodesNoMasterBlock() throws Exception {
123125
String masterNode = internalCluster().getMasterName();
124126
String otherNode = node1Name.equals(masterNode) ? node2Name : node1Name;
125127
logger.info("--> add voting config exclusion for non-master node, to be sure it's not elected");
126-
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(new String[]{otherNode})).get();
128+
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY,
129+
Strings.EMPTY_ARRAY, new String[]{otherNode}, TimeValue.timeValueSeconds(30))).get();
127130
logger.info("--> stop master node, no master block should appear");
128131
Settings masterDataPathSettings = internalCluster().dataPathSettings(masterNode);
129132
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(masterNode));
@@ -170,7 +173,8 @@ public void testTwoNodesNoMasterBlock() throws Exception {
170173
masterNode = internalCluster().getMasterName();
171174
otherNode = node1Name.equals(masterNode) ? node2Name : node1Name;
172175
logger.info("--> add voting config exclusion for master node, to be sure it's not elected");
173-
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(new String[]{masterNode})).get();
176+
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY,
177+
Strings.EMPTY_ARRAY, new String[]{masterNode}, TimeValue.timeValueSeconds(30))).get();
174178
logger.info("--> stop non-master node, no master block should appear");
175179
Settings otherNodeDataPathSettings = internalCluster().dataPathSettings(otherNode);
176180
internalCluster().stopRandomNode(InternalTestCluster.nameFilter(otherNode));

‎server/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@
2222
import org.apache.lucene.search.join.ScoreMode;
2323
import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsAction;
2424
import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsRequest;
25+
import org.elasticsearch.common.Strings;
2526
import org.elasticsearch.common.settings.Settings;
27+
import org.elasticsearch.common.unit.TimeValue;
2628
import org.elasticsearch.discovery.MasterNotDiscoveredException;
2729
import org.elasticsearch.index.query.QueryBuilders;
2830
import org.elasticsearch.node.Node;
@@ -115,7 +117,8 @@ public void testElectOnlyBetweenMasterNodes() throws Exception {
115117

116118
logger.info("--> closing master node (1)");
117119
client().execute(AddVotingConfigExclusionsAction.INSTANCE,
118-
new AddVotingConfigExclusionsRequest(new String[]{masterNodeName})).get();
120+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, new String[]{masterNodeName},
121+
TimeValue.timeValueSeconds(30))).get();
119122
// removing the master from the voting configuration immediately triggers the master to step down
120123
assertBusy(() -> {
121124
assertThat(internalCluster().nonMasterClient().admin().cluster().prepareState()

‎server/src/test/java/org/elasticsearch/cluster/coordination/VotingConfigurationIT.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import org.elasticsearch.cluster.ClusterState;
2525
import org.elasticsearch.cluster.node.DiscoveryNode;
2626
import org.elasticsearch.common.Priority;
27+
import org.elasticsearch.common.Strings;
28+
import org.elasticsearch.common.unit.TimeValue;
2729
import org.elasticsearch.plugins.Plugin;
2830
import org.elasticsearch.test.ESIntegTestCase;
2931
import org.elasticsearch.test.transport.MockTransportService;
@@ -55,7 +57,8 @@ public void testAbdicateAfterVotingConfigExclusionAdded() throws ExecutionExcept
5557

5658
logger.info("--> excluding master node {}", originalMaster);
5759
client().execute(AddVotingConfigExclusionsAction.INSTANCE,
58-
new AddVotingConfigExclusionsRequest(new String[]{originalMaster})).get();
60+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
61+
new String[]{originalMaster}, TimeValue.timeValueSeconds(30))).get();
5962
client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).get();
6063
assertNotEquals(originalMaster, internalCluster().getMasterName());
6164
}

‎server/src/test/java/org/elasticsearch/gateway/RecoveryFromGatewayIT.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.elasticsearch.cluster.service.ClusterService;
3737
import org.elasticsearch.common.Strings;
3838
import org.elasticsearch.common.settings.Settings;
39+
import org.elasticsearch.common.unit.TimeValue;
3940
import org.elasticsearch.common.xcontent.XContentFactory;
4041
import org.elasticsearch.env.NodeEnvironment;
4142
import org.elasticsearch.index.Index;
@@ -309,7 +310,8 @@ public void testTwoNodeFirstNodeCleared() throws Exception {
309310

310311
Map<String, long[]> primaryTerms = assertAndCapturePrimaryTerms(null);
311312

312-
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(new String[]{firstNode})).get();
313+
client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY,
314+
Strings.EMPTY_ARRAY, new String[]{firstNode}, TimeValue.timeValueSeconds(30))).get();
313315

314316
internalCluster().fullRestart(new RestartCallback() {
315317
@Override

‎test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1673,7 +1673,8 @@ private Set<String> excludeMasters(Collection<NodeAndClient> nodeAndClients) {
16731673
logger.info("adding voting config exclusions {} prior to restart/shutdown", excludedNodeIds);
16741674
try {
16751675
client().execute(AddVotingConfigExclusionsAction.INSTANCE,
1676-
new AddVotingConfigExclusionsRequest(excludedNodeIds.toArray(Strings.EMPTY_ARRAY))).get();
1676+
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, excludedNodeIds.toArray(Strings.EMPTY_ARRAY),
1677+
Strings.EMPTY_ARRAY, timeValueSeconds(30))).get();
16771678
} catch (InterruptedException | ExecutionException e) {
16781679
throw new AssertionError("unexpected", e);
16791680
}

0 commit comments

Comments
 (0)