Skip to content

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

  • Let LeakTracker implement UncheckedAutoCloseable and change methods that return it to use the interface. Tracked objects should not call other methods exposed due to LeakTracker extends WeakReference.
  • Distinguish detectors with the same name by appending a unique integer.
  • Split two test cases in TestLeakDetector.

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

How was this patch tested?

CI:
https://github.com/adoroszlai/ozone/actions/runs/7401596618

@adoroszlai adoroszlai self-assigned this Jan 3, 2024
@adoroszlai adoroszlai requested a review from duongkame January 4, 2024 13:28
Copy link
Contributor

@duongkame duongkame left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @adoroszlai , LGTM.

*/
package org.apache.hadoop.hdds.utils;

import org.apache.ratis.util.UncheckedAutoCloseable;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't love that Ozone depends on ratis for such a basic need. I guess it's ok for this PR as it's a general issue in Ozone.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now I think of Ratis as a library that provides Raft-implementation as well as useful utilities used by both Ratis and Ozone. The problem is that we don't have a top-level "utilities" Apache (or Apache-licensed other) project.

@adoroszlai adoroszlai merged commit 6df0237 into apache:master Jan 4, 2024
@adoroszlai
Copy link
Contributor Author

Thanks @duongkame for the review.

@adoroszlai adoroszlai deleted the HDDS-10000 branch January 4, 2024 20:06
adoroszlai added a commit to adoroszlai/ozone that referenced this pull request Mar 5, 2024
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