adoroszlai commented on code in PR #11376:
URL: https://github.com/apache/ozone/pull/11376#discussion_r4156698515
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshot.java:
##########
@@ -345,19 +353,21 @@ public UUID getSnapshotID() {
public void close() throws IOException {
// Close DB
omMetadataManager.getStore().close();
+ // Closed properly: stop tracking so the leak reporter does not fire at GC.
+ leakTracker.close();
}
- @Override
- protected void finalize() throws Throwable {
- // Verify that the DB handle has been closed, log warning otherwise
- // https://softwareengineering.stackexchange.com/a/288724
- if (!omMetadataManager.getStore().isClosed()) {
- LOG.warn("{} is not closed properly. snapshotName: {}",
- // Print hash code for debugging
- omMetadataManager.getStore().toString(),
- snapshotName);
- }
- super.finalize();
+ /**
+ * @return a leak reporter that captures only the {@code store} and {@code
snapshotName} objects,
+ * never the {@link OmSnapshot} itself.
+ */
+ static Runnable newLeakReporter(DBStore store, String snapshotName) {
+ return () -> {
+ if (!store.isClosed()) {
+ // Print hash code for debugging
+ LOG.warn("{} is not closed properly. snapshotName: {}", store,
snapshotName);
+ }
+ };
Review Comment:
I don't think we should keep a strong reference to `store` in the leak
reporter.
- I think checking `store.isClosed()` is unnecessary. `LeakTracker`
discards properly closed instances and will not call the reporter.
- For "print hash code for debugging", we should get that eagerly and keep
reference only to the string to be included in the message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
