Skip to content

Yet more refactoring of DiscoveryNode constructors #95824

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 9, 2023
Merged

Yet more refactoring of DiscoveryNode constructors #95824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e537477
Fix compile after DiscoveryNode constructor refactor
thecoop May 4, 2023
7433bfd
Refactor more constructors that specify the current version
thecoop May 2, 2023
fc127f7
Use null rather than empty string
thecoop May 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions benchmarks/src/main/java/org/elasticsearch/benchmark/routing/allocation/Allocators.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.elasticsearch.benchmark.routing.allocation;

import org.elasticsearch.Version;
import org.elasticsearch.cluster.ClusterModule;
import org.elasticsearch.cluster.EmptyClusterInfoService;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand Down Expand Up @@ -95,12 +94,12 @@ public static AllocationDeciders defaultAllocationDeciders(Settings settings, Cl

public static DiscoveryNode newNode(String nodeId, Map<String, String> attributes) {
return new DiscoveryNode(
"",
null,
nodeId,
new TransportAddress(TransportAddress.META_ADDRESS, portGenerator.incrementAndGet()),
attributes,
Sets.newHashSet(DiscoveryNodeRole.MASTER_ROLE, DiscoveryNodeRole.DATA_ROLE),
Version.CURRENT
null
);
}
}
6 changes: 2 additions & 4 deletions ...s/ingest-geoip/src/test/java/org/elasticsearch/ingest/geoip/DatabaseNodeServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.cluster.metadata.AliasMetadata;
import org.elasticsearch.cluster.metadata.IndexMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.node.TestDiscoveryNode;
Expand Down Expand Up @@ -192,13 +191,12 @@ public void testCheckDatabases_dontCheckDatabaseOnNonIngestNode() throws Excepti
ClusterState state = ClusterState.builder(createClusterState(tasksCustomMetadata))
.nodes(
new DiscoveryNodes.Builder().add(
new DiscoveryNode(
TestDiscoveryNode.create(
"_name1",
"_id1",
buildNewFakeTransportAddress(),
Map.of(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
)
).localNodeId("_id1")
)
Expand Down
15 changes: 2 additions & 13 deletions ...er/src/internalClusterTest/java/org/elasticsearch/discovery/StableMasterDisruptionIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

package org.elasticsearch.discovery;

import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
import org.elasticsearch.client.internal.Client;
import org.elasticsearch.cluster.ClusterChangedEvent;
Expand All @@ -21,8 +20,8 @@
import org.elasticsearch.cluster.coordination.LeaderChecker;
import org.elasticsearch.cluster.coordination.MasterHistoryService;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.node.TestDiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.Strings;
Expand Down Expand Up @@ -549,17 +548,7 @@ public void testCannotJoinLeader() throws Exception {
new DiscoveryNodes.Builder().masterNodeId(null)
).build();
ClusterState previousState = new ClusterState.Builder(new ClusterName(internalCluster().getClusterName())).nodes(
new DiscoveryNodes.Builder().masterNodeId("test")
.add(
new DiscoveryNode(
"test",
"test",
buildNewFakeTransportAddress(),
Collections.emptyMap(),
DiscoveryNodeRole.roles(),
Version.CURRENT
)
)
new DiscoveryNodes.Builder().masterNodeId("test").add(TestDiscoveryNode.create("test", "test"))
).build();
ClusterChangedEvent clusterChangedEvent = new ClusterChangedEvent("test", state, previousState);
masterHistoryService.getLocalMasterHistory().clusterChanged(clusterChangedEvent);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/transport/ProxyConnectionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ public void onFailure(Exception e) {
attributes = Collections.singletonMap("server_name", configuredServerName);
}
DiscoveryNode node = new DiscoveryNode(
"",
null,
id,
resolved,
attributes,
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/org/elasticsearch/transport/SniffConnectionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ private static DiscoveryNode resolveSeedNode(String clusterAlias, String address
if (proxyAddress == null || proxyAddress.isEmpty()) {
TransportAddress transportAddress = new TransportAddress(parseConfiguredAddress(address));
return new DiscoveryNode(
"",
null,
clusterAlias + "#" + transportAddress,
transportAddress,
Collections.emptyMap(),
Expand All @@ -509,7 +509,7 @@ private static DiscoveryNode resolveSeedNode(String clusterAlias, String address
TransportAddress transportAddress = new TransportAddress(parseConfiguredAddress(proxyAddress));
String hostName = RemoteConnectionStrategy.parseHost(proxyAddress);
return new DiscoveryNode(
"",
null,
clusterAlias + "#" + address,
UUIDs.randomBase64UUID(),
hostName,
Expand Down
92 changes: 37 additions & 55 deletions ...asticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.elasticsearch.action.admin.cluster.configuration;

import org.elasticsearch.Version;
import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.coordination.CoordinationMetadata;
Expand All @@ -16,6 +15,7 @@
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes.Builder;
import org.elasticsearch.cluster.node.TestDiscoveryNode;
import org.elasticsearch.common.Strings;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.test.ESTestCase;
Expand Down Expand Up @@ -59,40 +59,36 @@ public void testSerializationForNodeIdOrNodeName() throws IOException {
}

public void testResolve() {
final DiscoveryNode localNode = new DiscoveryNode(
final DiscoveryNode localNode = TestDiscoveryNode.create(
"local",
"local",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode);
final DiscoveryNode otherNode1 = new DiscoveryNode(
final DiscoveryNode otherNode1 = TestDiscoveryNode.create(
"other1",
"other1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion otherNode1Exclusion = new VotingConfigExclusion(otherNode1);
final DiscoveryNode otherNode2 = new DiscoveryNode(
final DiscoveryNode otherNode2 = TestDiscoveryNode.create(
"other2",
"other2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion otherNode2Exclusion = new VotingConfigExclusion(otherNode2);
final DiscoveryNode otherDataNode = new DiscoveryNode(
final DiscoveryNode otherDataNode = TestDiscoveryNode.create(
"data",
"data",
buildNewFakeTransportAddress(),
emptyMap(),
emptySet(),
Version.CURRENT
emptySet()
);

final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster"))
Expand Down Expand Up @@ -136,33 +132,30 @@ public void testResolveMoreThanOneNodeIdentifiersSet() {
}

public void testResolveByNodeIds() {
final DiscoveryNode node1 = new DiscoveryNode(
final DiscoveryNode node1 = TestDiscoveryNode.create(
"nodeName1",
"nodeId1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion node1Exclusion = new VotingConfigExclusion(node1);

final DiscoveryNode node2 = new DiscoveryNode(
final DiscoveryNode node2 = TestDiscoveryNode.create(
"nodeName2",
"nodeId2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2);

final DiscoveryNode node3 = new DiscoveryNode(
final DiscoveryNode node3 = TestDiscoveryNode.create(
"nodeName3",
"nodeId3",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final VotingConfigExclusion unresolvableVotingConfigExclusion = new VotingConfigExclusion(
Expand All @@ -188,33 +181,30 @@ public void testResolveByNodeIds() {
}

public void testResolveByNodeNames() {
final DiscoveryNode node1 = new DiscoveryNode(
final DiscoveryNode node1 = TestDiscoveryNode.create(
"nodeName1",
"nodeId1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion node1Exclusion = new VotingConfigExclusion(node1);

final DiscoveryNode node2 = new DiscoveryNode(
final DiscoveryNode node2 = TestDiscoveryNode.create(
"nodeName2",
"nodeId2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2);

final DiscoveryNode node3 = new DiscoveryNode(
final DiscoveryNode node3 = TestDiscoveryNode.create(
"nodeName3",
"nodeId3",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final VotingConfigExclusion unresolvableVotingConfigExclusion = new VotingConfigExclusion(
Expand All @@ -238,22 +228,20 @@ public void testResolveByNodeNames() {
}

public void testResolveAmbiguousName() {
final DiscoveryNode node1 = new DiscoveryNode(
final DiscoveryNode node1 = TestDiscoveryNode.create(
"ambiguous-name",
"nodeId1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final DiscoveryNode node2 = new DiscoveryNode(
final DiscoveryNode node2 = TestDiscoveryNode.create(
"ambiguous-name",
"nodeId2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster"))
Expand All @@ -272,32 +260,29 @@ public void testResolveAmbiguousName() {
}

public void testResolveRemoveExistingVotingConfigExclusions() {
final DiscoveryNode node1 = new DiscoveryNode(
final DiscoveryNode node1 = TestDiscoveryNode.create(
"nodeName1",
"nodeId1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final DiscoveryNode node2 = new DiscoveryNode(
final DiscoveryNode node2 = TestDiscoveryNode.create(
"nodeName2",
"nodeId2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion node2Exclusion = new VotingConfigExclusion(node2);

final DiscoveryNode node3 = new DiscoveryNode(
final DiscoveryNode node3 = TestDiscoveryNode.create(
"nodeName3",
"nodeId3",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final VotingConfigExclusion existingVotingConfigExclusion = new VotingConfigExclusion(node1);
Expand All @@ -319,31 +304,28 @@ public void testResolveRemoveExistingVotingConfigExclusions() {
}

public void testResolveAndCheckMaximum() {
final DiscoveryNode localNode = new DiscoveryNode(
final DiscoveryNode localNode = TestDiscoveryNode.create(
"local",
"local",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion localNodeExclusion = new VotingConfigExclusion(localNode);
final DiscoveryNode otherNode1 = new DiscoveryNode(
final DiscoveryNode otherNode1 = TestDiscoveryNode.create(
"other1",
"other1",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);
final VotingConfigExclusion otherNode1Exclusion = new VotingConfigExclusion(otherNode1);
final DiscoveryNode otherNode2 = new DiscoveryNode(
final DiscoveryNode otherNode2 = TestDiscoveryNode.create(
"other2",
"other2",
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
Set.of(DiscoveryNodeRole.MASTER_ROLE)
);

final ClusterState.Builder builder = ClusterState.builder(new ClusterName("cluster"))
Expand Down
13 changes: 3 additions & 10 deletions ...rch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.cluster.ClusterName;
Expand All @@ -24,6 +23,7 @@
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodeRole;
import org.elasticsearch.cluster.node.DiscoveryNodes.Builder;
import org.elasticsearch.cluster.node.TestDiscoveryNode;
import org.elasticsearch.cluster.service.ClusterService;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
Expand Down Expand Up @@ -82,19 +82,12 @@ public static void createThreadPoolAndClusterService() {
otherNode1Exclusion = new VotingConfigExclusion(otherNode1);
otherNode2 = makeDiscoveryNode("other2");
otherNode2Exclusion = new VotingConfigExclusion(otherNode2);
otherDataNode = new DiscoveryNode("data", "data", buildNewFakeTransportAddress(), emptyMap(), emptySet(), Version.CURRENT);
otherDataNode = TestDiscoveryNode.create("data", "data", buildNewFakeTransportAddress(), emptyMap(), emptySet());
clusterService = createClusterService(threadPool, localNode);
}

private static DiscoveryNode makeDiscoveryNode(String name) {
return new DiscoveryNode(
name,
name,
buildNewFakeTransportAddress(),
emptyMap(),
Set.of(DiscoveryNodeRole.MASTER_ROLE),
Version.CURRENT
);
return TestDiscoveryNode.create(name, name, buildNewFakeTransportAddress(), emptyMap(), Set.of(DiscoveryNodeRole.MASTER_ROLE));
}

@AfterClass
Expand Down
Loading