Skip to content
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.

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

HDDS-12551. Replace dnsToUuidMap with dnsToDnIdMap in SCMNodeManager #8087

Merged
merged 4 commits into from
Mar 16, 2025

Conversation

chiacyu
Copy link
Contributor

@chiacyu chiacyu commented Mar 15, 2025

What changes were proposed in this pull request?

In SCMNodeManager, we maintain DNS to Datanode mapping. This is currently using UUID to represent Datanode, we can replace the UUID with DatanodeID.

What is the link to the Apache JIRA

HDDS-12551

How was this patch tested?

It can test by CI.

@chiacyu
Copy link
Contributor Author

chiacyu commented Mar 15, 2025

Hi, @nandakumar131
Please take a look, while you're available, thanks!

Copy link
Contributor

@nandakumar131 nandakumar131 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Over all the change looks good, just a minor comment.

} catch (NodeNotFoundException e) {
LOG.warn("Cannot find node for uuid {}", uuid);
List<DatanodeDetails> datanodeDetails = allNodes.stream().
filter(node -> node.getID().getID().equals(datanodeID.getID())).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
filter(node -> node.getID().getID().equals(datanodeID.getID())).
filter(node -> node.getID().equals(datanodeID)).

@nandakumar131 nandakumar131 merged commit ff3ef51 into apache:master Mar 16, 2025
43 checks passed
@nandakumar131
Copy link
Contributor

Thanks @chiacyu for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants