Skip to content

HDDS-16043. Use EnumMap for ports in DatanodeDetails #10948

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
jojochuang merged 1 commit into from
Aug 6, 2026
Merged

HDDS-16043. Use EnumMap for ports in DatanodeDetails #10948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 32 additions & 37 deletions hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumMap;
import java.util.EnumSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.hdds.DatanodeVersion;
import org.apache.hadoop.hdds.HddsUtils;
Expand Down Expand Up @@ -84,7 +85,7 @@ public class DatanodeDetails extends NodeImpl implements Comparable<DatanodeDeta
private final String threadNamePrefix;
private StringWithByteString ipAddress;
private StringWithByteString hostName;
private final List<Port> ports;
private final Map<Port.Name, Port> ports;
private String certSerialId;
private String version;
private long setupTime;
Expand Down Expand Up @@ -244,10 +245,8 @@ public StringWithByteString getHostNameAsByteString() {
* @param port DataNode port
*/
public synchronized void setPort(Port port) {
// If the port is already in the list remove it first and add the
// new/updated port value.
ports.remove(port);
ports.add(port);
// Overwrites any existing port with the same name.
ports.put(port.getName(), port);
}

public synchronized void setPort(Name name, int port) {
Expand All @@ -272,11 +271,11 @@ public void setStandalonePort(int port) {
* @return DataNode Ports
*/
public synchronized List<Port> getPorts() {
return new ArrayList<>(ports);
return new ArrayList<>(ports.values());
}

public synchronized boolean hasPort(int port) {
for (Port p : ports) {
for (Port p : ports.values()) {
if (p.getValue() == port) {
return true;
}
Expand Down Expand Up @@ -356,51 +355,46 @@ public void setPersistedOpStateExpiryEpochSec(long expiry) {
* @return Port
*/
public synchronized Port getPort(Port.Name name) {
Port ratisPort = null;
for (Port port : ports) {
if (port.getName().equals(name)) {
return port;
}
if (port.getName().equals(Name.RATIS)) {
ratisPort = port;
}
final Port port = ports.get(name);
if (port != null) {
return port;
}
// if no separate admin/server/datastream port,
// return single Ratis one for compatibility
if (name == Name.RATIS_ADMIN || name == Name.RATIS_SERVER ||
name == Name.RATIS_DATASTREAM) {
return ratisPort;
return ports.get(Name.RATIS);
}
return null;
}

// CHANGE: add a helper to check whether a port is explicitly present
// without applying compatibility fallback.
// Checks whether a port is explicitly present without applying the
// compatibility fallback in getPort.
public synchronized boolean hasPort(Port.Name name) {
for (Port port : ports) {
if (port.getName().equals(name)) {
return true;
}
}
return false;
return ports.containsKey(name);
}

/**
* Whether this datanode's exposed ports differ from {@code other}'s.
* Compared as a set of name=value entries, since {@link Port#equals}
* ignores the port value.
* Compared by name and value, since {@link Port#equals} ignores the port
* value.
*
* @param other another snapshot of this datanode
* @return true if the two port sets are not identical
*/
public boolean portsChanged(DatanodeDetails other) {
return !portValues(this).equals(portValues(other));
return !portValues().equals(other.portValues());
}

private static Set<String> portValues(DatanodeDetails datanodeDetails) {
return datanodeDetails.getPorts().stream()
.map(port -> port.getName() + "=" + port.getValue())
.collect(Collectors.toSet());
/**
* A name-to-value snapshot of this datanode's ports, taken under its lock
* so {@link #portsChanged} can compare two nodes value-aware without holding
* both locks at once.
*/
private synchronized Map<Port.Name, Integer> portValues() {
final Map<Port.Name, Integer> values = new EnumMap<>(Port.Name.class);
ports.forEach((name, port) -> values.put(name, port.getValue()));
return values;
}

/**
Expand Down Expand Up @@ -595,7 +589,7 @@ public HddsProtos.DatanodeDetailsProto.Builder toProtoBuilder(
.compareTo(VERSION_HANDLES_UNKNOWN_DN_PORTS) >= 0;
final int requestedPortCount = filterPorts.size();
final boolean maySkip = requestedPortCount > 0;
for (Port port : ports) {
for (Port port : ports.values()) {
if (maySkip && !filterPorts.contains(port.getName())) {
if (LOG.isDebugEnabled()) {
LOG.debug("Skip adding {} port {} to proto message",
Expand Down Expand Up @@ -738,7 +732,7 @@ public static final class Builder {
private StringWithByteString networkName;
private StringWithByteString networkLocation;
private int level;
private List<Port> ports;
private Map<Port.Name, Port> ports;
private String certSerialId;
private String version;
private long setupTime;
Expand All @@ -753,7 +747,7 @@ public static final class Builder {
* DatanodeDetails#newBuilder.
*/
private Builder() {
ports = new ArrayList<>();
ports = new EnumMap<>(Port.Name.class);
}

/**
Expand All @@ -769,7 +763,8 @@ public Builder setDatanodeDetails(DatanodeDetails details) {
this.networkName = details.getNetworkNameAsByteString();
this.networkLocation = details.getNetworkLocationAsByteString();
this.level = details.getLevel();
this.ports = details.getPorts();
this.ports = new EnumMap<>(Port.Name.class);
details.getPorts().forEach(this::addPort);
this.certSerialId = details.getCertSerialId();
this.version = details.getVersion();
this.setupTime = details.getSetupTime();
Expand Down Expand Up @@ -885,7 +880,7 @@ public Builder setLevel(int level) {
* @return DatanodeDetails.Builder
*/
public Builder addPort(Port port) {
this.ports.add(port);
this.ports.put(port.getName(), port);
return this;
}

Expand Down
37 changes: 37 additions & 0 deletions hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/protocol/TestDatanodeDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import static org.apache.hadoop.ozone.ClientVersion.VERSION_HANDLES_UNKNOWN_DN_PORTS;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.google.common.collect.ImmutableSet;
import java.util.Set;
Expand Down Expand Up @@ -86,6 +88,41 @@ public void testNewBuilderCurrentVersion() {
assertEquals(DatanodeVersion.CURRENT.toProtoValue(), dn3.getCurrentVersion());
}

@Test
void portsChangedComparesNameAndValue() {
DatanodeID id = DatanodeID.randomID();
DatanodeDetails base = DatanodeDetails.newBuilder()
.setID(id)
.addPort(DatanodeDetails.newStandalonePort(9858))
.addPort(DatanodeDetails.newRatisPort(9859))
.build();

// Identical name/value set: no change.
DatanodeDetails same = DatanodeDetails.newBuilder()
.setID(id)
.addPort(DatanodeDetails.newStandalonePort(9858))
.addPort(DatanodeDetails.newRatisPort(9859))
.build();
assertFalse(base.portsChanged(same));

// Same names, one different value: detected (Port.equals ignores value).
DatanodeDetails changedValue = DatanodeDetails.newBuilder()
.setID(id)
.addPort(DatanodeDetails.newStandalonePort(9858))
.addPort(DatanodeDetails.newRatisPort(1234))
.build();
assertTrue(base.portsChanged(changedValue));

// Extra port: detected (key set differs).
DatanodeDetails extraPort = DatanodeDetails.newBuilder()
.setID(id)
.addPort(DatanodeDetails.newStandalonePort(9858))
.addPort(DatanodeDetails.newRatisPort(9859))
.addPort(DatanodeDetails.newPort(Name.RATIS_DATASTREAM, 9860))
.build();
assertTrue(base.portsChanged(extraPort));
}

public static void assertPorts(HddsProtos.DatanodeDetailsProto dn,
Set<Port.Name> expectedPorts) throws IllegalArgumentException {
assertEquals(expectedPorts.size(), dn.getPortsCount());
Expand Down
Loading