Skip to content

HDDS-8854. Avoid unnecessary DatanodeDetails creation for NodeStateManager lookup #4925

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
adoroszlai merged 1 commit into from
Jun 23, 2023
Merged

HDDS-8854. Avoid unnecessary DatanodeDetails creation for NodeStateManager lookup #4925

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
7 changes: 6 additions & 1 deletion hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,12 @@ public int getPipelinesCount(DatanodeDetails datanodeDetails) {
*/
public DatanodeInfo getNode(DatanodeDetails datanodeDetails)
throws NodeNotFoundException {
return nodeStateMap.getNodeInfo(datanodeDetails.getUuid());
return getNode(datanodeDetails.getUuid());
}

public DatanodeInfo getNode(UUID uuid)
throws NodeNotFoundException {
return nodeStateMap.getNodeInfo(uuid);
}

/**
Expand Down
30 changes: 12 additions & 18 deletions hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,7 @@ public class SCMNodeManager implements NodeManager {
private final NetworkTopology clusterMap;
private final DNSToSwitchMapping dnsToSwitchMapping;
private final boolean useHostname;
private final ConcurrentHashMap<String, Set<String>> dnsToUuidMap =
new ConcurrentHashMap<>();
private final Map<String, Set<UUID>> dnsToUuidMap = new ConcurrentHashMap<>();
private final int numPipelinesPerMetadataVolume;
private final int heavyNodeCriteria;
private final HDDSLayoutVersionManager scmLayoutVersionManager;
Expand Down Expand Up @@ -400,7 +399,7 @@ public RegisteredCommand register(
// Check that datanode in nodeStateManager has topology parent set
DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
Preconditions.checkState(dn.getParent() != null);
addToDnsToUuidMap(dnsName, datanodeDetails.getUuidString());
addToDnsToUuidMap(dnsName, datanodeDetails.getUuid());
// Updating Node Report, as registration is successful
processNodeReport(datanodeDetails, nodeReport);
LOG.info("Registered Data node : {}", datanodeDetails.toDebugString());
Expand Down Expand Up @@ -434,9 +433,7 @@ public RegisteredCommand register(
} else {
oldDnsName = datanodeInfo.getIpAddress();
}
updateDnsToUuidMap(oldDnsName,
dnsName,
datanodeDetails.getUuidString());
updateDnsToUuidMap(oldDnsName, dnsName, datanodeDetails.getUuid());

nodeStateManager.updateNode(datanodeDetails, layoutInfo);
DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
Expand Down Expand Up @@ -466,20 +463,20 @@ public RegisteredCommand register(
* @param addr the hostname or IP of the node
* @param uuid the UUID of the registered node.
*/
private synchronized void addToDnsToUuidMap(String addr, String uuid) {
private synchronized void addToDnsToUuidMap(String addr, UUID uuid) {
dnsToUuidMap.computeIfAbsent(addr, k -> ConcurrentHashMap.newKeySet())
.add(uuid);
}

private synchronized void removeFromDnsToUuidMap(String addr, String uuid) {
Set<String> dnSet = dnsToUuidMap.get(addr);
private synchronized void removeFromDnsToUuidMap(String addr, UUID uuid) {
Set<UUID> dnSet = dnsToUuidMap.get(addr);
if (dnSet != null && dnSet.remove(uuid) && dnSet.isEmpty()) {
dnsToUuidMap.remove(addr);
}
}

private synchronized void updateDnsToUuidMap(
String oldDnsName, String newDnsName, String uuid) {
String oldDnsName, String newDnsName, UUID uuid) {
Preconditions.checkNotNull(oldDnsName, "old address == null");
Preconditions.checkNotNull(newDnsName, "new address == null");
if (!oldDnsName.equals(newDnsName)) {
Expand Down Expand Up @@ -976,7 +973,7 @@ private SCMNodeStat getNodeStatInternal(DatanodeDetails datanodeDetails) {
return new SCMNodeStat(capacity, used, remaining);
} catch (NodeNotFoundException e) {
LOG.warn("Cannot generate NodeStat, datanode {} not found.",
datanodeDetails.getUuid());
datanodeDetails.getUuidString());
return null;
}
}
Expand Down Expand Up @@ -1350,8 +1347,7 @@ public DatanodeDetails getNodeByUuid(UUID uuid) {
}

try {
return nodeStateManager.getNode(
DatanodeDetails.newBuilder().setUuid(uuid).build());
return nodeStateManager.getNode(uuid);
} catch (NodeNotFoundException e) {
LOG.warn("Cannot find node for uuid {}", uuid);
return null;
Expand All @@ -1372,17 +1368,15 @@ public List<DatanodeDetails> getNodesByAddress(String address) {
LOG.warn("address is null");
return results;
}
Set<String> uuids = dnsToUuidMap.get(address);
Set<UUID> uuids = dnsToUuidMap.get(address);
if (uuids == null) {
LOG.warn("Cannot find node for address {}", address);
return results;
}

for (String uuid : uuids) {
DatanodeDetails temp = DatanodeDetails.newBuilder()
.setUuid(UUID.fromString(uuid)).build();
for (UUID uuid : uuids) {
try {
results.add(nodeStateManager.getNode(temp));
results.add(nodeStateManager.getNode(uuid));
} catch (NodeNotFoundException e) {
LOG.warn("Cannot find node for uuid {}", uuid);
}
Expand Down