Skip to content

HDDS-10507. Use equals() instead of == for nodes in NetworkTopology#6368

Merged
adoroszlai merged 8 commits intoapache:masterfrom
tanvipenumudy:HDDS-10507
Apr 2, 2024
Merged

HDDS-10507. Use equals() instead of == for nodes in NetworkTopology#6368
adoroszlai merged 8 commits intoapache:masterfrom
tanvipenumudy:HDDS-10507

Conversation

@tanvipenumudy
Copy link
Contributor

@tanvipenumudy tanvipenumudy commented Mar 12, 2024

What changes were proposed in this pull request?

  1. The changes introduced in PR: #5391 involve reconstructing the hierarchical cluster tree object (that represents the network topology) on the Ozone Manager side.
  2. Under the NetworkTopology class, various operations employ the ==/!= operator for comparison.
  3. However, this comparator needs to be modified to utilize the InnerNode#equals method instead. This adjustment is required as certain use cases, as described in 1., involve object reconstruction necessitating custom equality comparisons based on individual object attributes.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10507

How was this patch tested?

Existing tests should cover the changes, Green Git CI: https://github.com/tanvipenumudy/ozone/actions/runs/8433408560

@tanvipenumudy tanvipenumudy marked this pull request as ready for review March 13, 2024 08:37
@tanvipenumudy tanvipenumudy marked this pull request as draft March 13, 2024 08:38
@tanvipenumudy tanvipenumudy marked this pull request as ready for review March 26, 2024 08:02
@tanvipenumudy
Copy link
Contributor Author

@adoroszlai, @ChenSammi could you please review the changes? Thanks.

Copy link
Contributor

@dineshchitlangia dineshchitlangia left a comment

Choose a reason for hiding this comment

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

LGTM

@tanvipenumudy tanvipenumudy changed the title HDDS-10507. Update NetworkTopology-based usecases to utilize InnerNode.equals() HDDS-10507. Revise NetworkTopology-based use cases to leverage custom equals() method Mar 26, 2024
@tanvipenumudy
Copy link
Contributor Author

cc: @kerneltime could you also please take a look, thanks.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @tanvipenumudy for the patch. Please use java.utils.Objects#equals for null-safe equals instead of reimplementing it. (Added one suggestion, but also applies to other parts of the change.)

tanvipenumudy and others added 2 commits April 1, 2024 10:15
…t/NetworkTopologyImpl.java

Co-authored-by: Doroszlai, Attila <6454655+adoroszlai@users.noreply.github.com>
@adoroszlai adoroszlai changed the title HDDS-10507. Revise NetworkTopology-based use cases to leverage custom equals() method HDDS-10507. Use equals() instead of == for nodes in NetworkTopology Apr 1, 2024
@tanvipenumudy
Copy link
Contributor Author

Thank you @adoroszlai for the review, I have simplified the patch as suggested utilizing: java.utils.Objects#equals.

@adoroszlai
Copy link
Contributor

Thanks @tanvipenumudy for updating the patch, LGTM. If you don't want to wait for @kerneltime's review, it can be merged.

@tanvipenumudy
Copy link
Contributor Author

Sure, I believe we can check in the change, thank you @adoroszlai for the review!

@adoroszlai adoroszlai merged commit a73fb37 into apache:master Apr 2, 2024
@adoroszlai
Copy link
Contributor

Thanks @tanvipenumudy for the patch, @dineshchitlangia for the review.

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request May 29, 2024
swamirishi pushed a commit to swamirishi/ozone that referenced this pull request Dec 3, 2025
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.

3 participants