From 78994d90ce851485da7615c927384d04732bbaac Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Wed, 20 Dec 2023 10:33:18 -0800 Subject: [PATCH 01/18] HDDS-9528. Managed objects should not override finalize() --- .../hadoop/hdds/resource/LeakDetector.java | 67 +++++++++++++++++++ .../apache/hadoop/hdds/resource/Leakable.java | 37 ++++++++++ .../hadoop/hdds/resource/package-info.java | 22 ++++++ .../utils/db/managed/ManagedBloomFilter.java | 10 ++- .../managed/ManagedColumnFamilyOptions.java | 10 +-- .../managed/ManagedCompactRangeOptions.java | 10 ++- .../utils/db/managed/ManagedDBOptions.java | 10 ++- .../utils/db/managed/ManagedEnvOptions.java | 10 ++- .../utils/db/managed/ManagedFlushOptions.java | 11 +-- .../ManagedIngestExternalFileOptions.java | 12 ++-- .../utils/db/managed/ManagedLRUCache.java | 9 +-- .../hdds/utils/db/managed/ManagedObject.java | 8 ++- .../hdds/utils/db/managed/ManagedOptions.java | 10 ++- .../utils/db/managed/ManagedReadOptions.java | 10 ++- .../db/managed/ManagedRocksObjectMetrics.java | 11 +++ .../db/managed/ManagedRocksObjectUtils.java | 4 ++ .../hdds/utils/db/managed/ManagedSlice.java | 12 ++-- .../db/managed/ManagedSstFileWriter.java | 8 ++- .../utils/db/managed/ManagedStatistics.java | 11 +-- .../utils/db/managed/ManagedWriteBatch.java | 10 +-- .../utils/db/managed/ManagedWriteOptions.java | 11 +-- .../managed/TestRocksObjectLeakDetector.java | 58 ++++++++++++++++ 22 files changed, 304 insertions(+), 57 deletions(-) create mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java create mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java create mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java create mode 100644 hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java new file mode 100644 index 000000000000..54907bbfc031 --- /dev/null +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -0,0 +1,67 @@ +package org.apache.hadoop.hdds.resource; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; + +/** + * Simple general resource leak detector using {@link ReferenceQueue} to observe resource object life-cycle + * and perform assertions to verify proper resource closure after they are GCed. + * + *
 {@code
+ * class MyResource implements Leakable {
+ *   public MyResource() {
+ *     ResourceLeakDetector.LEAK_DETECTOR.watch(this);
+ *   }
+ *   public void check() {
+ *     // assertions to verify this resource closure.
+ *   }
+ * }
+ * class ResourceLeakDetector {
+ *    public final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
+ * }
+ * }
+ * @see Leakable + */ +public class LeakDetector implements Runnable { + public static final Logger LOG = LoggerFactory.getLogger(LeakDetector.class); + private final ReferenceQueue queue = new ReferenceQueue<>(); + private final String name; + + public LeakDetector(String name) { + this.name = name; + start(); + } + + private void start() { + Thread t = new Thread(this); + t.setName(LeakDetector.class.getSimpleName() + "-" + name); + t.setDaemon(true); + t.start(); + } + + @Override + public void run() { + while (true) { + try { + WeakReference ref = (WeakReference) queue.remove(); + Leakable leakable = ref.get(); + if (leakable != null) { + leakable.check(); + } + } catch (InterruptedException e) { + LOG.warn("Thread interrupted, exiting."); + break; + } + } + + LOG.warn("Existing leak detector {}.", name); + } + + public void watch(Leakable leakable) { + // Intentionally ignored. + WeakReference ignored = new WeakReference(leakable, queue); + } +} diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java new file mode 100644 index 000000000000..e835975a37de --- /dev/null +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ +package org.apache.hadoop.hdds.resource; + +/** + * Interface for a leakable resource. This interface should be implemented by the resource to monitor itself. + * + * Creating proxy objects, e.g. using lambda, like below does not work because we expect the resource object + * itself is monitored, not the immediate proxy object. + *
 {@code
+ * LEAK_DETECTOR.watch(() -> { assertClose(resource) });
+ * }
+ * + * @see LeakDetector + */ +public interface Leakable { + /** + * Perform assertions to verify proper resource closure. This is invoked after the resource object is GCed. + */ + void check(); +} diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java new file mode 100644 index 000000000000..378c2304f5c7 --- /dev/null +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java @@ -0,0 +1,22 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.resource; + +/** + * Contain utilities for resource management. + */ \ No newline at end of file diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java index 9e7bad651f77..4814d950d7e2 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.BloomFilter; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,16 @@ /** * Managed BloomFilter. */ -public class ManagedBloomFilter extends BloomFilter { +public class ManagedBloomFilter extends BloomFilter implements Leakable { + public ManagedBloomFilter() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java index 39ee7b0ce7ac..1f1043b458d8 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java @@ -18,11 +18,13 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.TableFormatConfig; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -30,7 +32,7 @@ /** * Managed ColumnFamilyOptions. */ -public class ManagedColumnFamilyOptions extends ColumnFamilyOptions { +public class ManagedColumnFamilyOptions extends ColumnFamilyOptions implements Leakable { @Nullable private final StackTraceElement[] elements = getStackTrace(); @@ -42,11 +44,12 @@ public class ManagedColumnFamilyOptions extends ColumnFamilyOptions { private boolean reused = false; public ManagedColumnFamilyOptions() { - super(); + LEAK_DETECTOR.watch(this); } public ManagedColumnFamilyOptions(ColumnFamilyOptions columnFamilyOptions) { super(columnFamilyOptions); + LEAK_DETECTOR.watch(this); } @Override @@ -85,9 +88,8 @@ public boolean isReused() { } @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } /** diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java index 6f7da30cdba7..4f8562ad42e4 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.CompactRangeOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,16 @@ /** * Managed CompactRangeOptions. */ -public class ManagedCompactRangeOptions extends CompactRangeOptions { +public class ManagedCompactRangeOptions extends CompactRangeOptions implements Leakable { + public ManagedCompactRangeOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java index a66a04eae90a..4276d1377af4 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.DBOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,16 @@ /** * Managed DBOptions. */ -public class ManagedDBOptions extends DBOptions { +public class ManagedDBOptions extends DBOptions implements Leakable { + public ManagedDBOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java index 38d6f95ce708..ef8e015dd0a3 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.EnvOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,16 @@ /** * Managed EnvOptions. */ -public class ManagedEnvOptions extends EnvOptions { +public class ManagedEnvOptions extends EnvOptions implements Leakable { + public ManagedEnvOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java index 8801f7d241d0..f7ebf440a914 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.FlushOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,15 @@ /** * Managed FlushOptions. */ -public class ManagedFlushOptions extends FlushOptions { - +public class ManagedFlushOptions extends FlushOptions implements Leakable { + public ManagedFlushOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java index 94b34e20d2ca..2f5305cb0491 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.IngestExternalFileOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,15 +31,17 @@ /** * Managed IngestExternalFileOptions. */ -public class ManagedIngestExternalFileOptions extends - IngestExternalFileOptions { +public class ManagedIngestExternalFileOptions extends IngestExternalFileOptions implements + Leakable { + public ManagedIngestExternalFileOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java index 8bf9147c1571..3b250e2cead3 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.LRUCache; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,18 +31,17 @@ /** * Managed LRUCache. */ -public class ManagedLRUCache extends LRUCache { - +public class ManagedLRUCache extends LRUCache implements Leakable { @Nullable private final StackTraceElement[] elements = getStackTrace(); public ManagedLRUCache(long capacity) { super(capacity); + LEAK_DETECTOR.watch(this); } @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java index 0ab87f156ec9..1ee7938d0263 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java @@ -18,17 +18,19 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.RocksObject; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; /** * General template for a managed RocksObject. * @param */ -class ManagedObject implements AutoCloseable { +class ManagedObject implements AutoCloseable, Leakable { private final T original; @Nullable @@ -37,6 +39,7 @@ class ManagedObject implements AutoCloseable { ManagedObject(T original) { this.original = original; this.elements = ManagedRocksObjectUtils.getStackTrace(); + LEAK_DETECTOR.watch(this); } public T get() { @@ -49,9 +52,8 @@ public void close() { } @Override - protected void finalize() throws Throwable { + public void check() { ManagedRocksObjectUtils.assertClosed(this); - super.finalize(); } public String getStackTrace() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java index 4ae96e1b7682..865af6856e8e 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.Options; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,16 @@ /** * Managed Options. */ -public class ManagedOptions extends Options { +public class ManagedOptions extends Options implements Leakable { + public ManagedOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java index a281722d2480..7dcbd230a930 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.ReadOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,15 +31,17 @@ /** * Managed {@link ReadOptions}. */ -public class ManagedReadOptions extends ReadOptions { +public class ManagedReadOptions extends ReadOptions implements Leakable { + public ManagedReadOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectMetrics.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectMetrics.java index 8d9d7fe78d7e..706aa9bb345f 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectMetrics.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectMetrics.java @@ -18,6 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.metrics2.annotation.Metric; import org.apache.hadoop.metrics2.annotation.Metrics; @@ -64,4 +65,14 @@ public void assertNoLeaks() { void increaseManagedObject() { totalManagedObjects.incr(); } + + @VisibleForTesting + long totalLeakObjects() { + return totalLeakObjects.value(); + } + + @VisibleForTesting + long totalManagedObjects() { + return totalManagedObjects.value(); + } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index b32f1111cbcb..a51dc50bf2ed 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -19,6 +19,7 @@ package org.apache.hadoop.hdds.utils.db.managed; import org.apache.hadoop.hdds.HddsUtils; +import org.apache.hadoop.hdds.resource.LeakDetector; import org.awaitility.Awaitility; import org.awaitility.core.ConditionTimeoutException; import org.rocksdb.RocksDB; @@ -41,6 +42,9 @@ private ManagedRocksObjectUtils() { public static final Logger LOG = LoggerFactory.getLogger(ManagedRocksObjectUtils.class); + + static final LeakDetector LEAK_DETECTOR = new LeakDetector("ManagedRocksObject"); + private static final Duration POLL_DELAY_DURATION = Duration.ZERO; private static final Duration POLL_INTERVAL_DURATION = Duration.ofMillis(100); diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java index 2de2031fb9c0..69be32936b01 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java @@ -18,23 +18,26 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.Slice; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; /** * Managed Slice. */ -public class ManagedSlice extends Slice { +public class ManagedSlice extends Slice implements Leakable { @Nullable private final StackTraceElement[] elements; - public ManagedSlice(byte[] var1) { - super(var1); + public ManagedSlice(byte[] data) { + super(data); this.elements = ManagedRocksObjectUtils.getStackTrace(); + LEAK_DETECTOR.watch(this); } @Override @@ -43,12 +46,11 @@ public synchronized long getNativeHandle() { } @Override - protected void finalize() throws Throwable { + public void check() { ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject(); if (isOwningHandle()) { ManagedRocksObjectUtils.reportLeak(this, formatStackTrace(elements)); } - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java index e6fdcbc2ed0b..35bd7de86964 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java @@ -18,12 +18,14 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.EnvOptions; import org.rocksdb.Options; import org.rocksdb.SstFileWriter; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -31,7 +33,7 @@ /** * Managed SstFileWriter. */ -public class ManagedSstFileWriter extends SstFileWriter { +public class ManagedSstFileWriter extends SstFileWriter implements Leakable { @Nullable private final StackTraceElement[] elements = getStackTrace(); @@ -39,11 +41,11 @@ public class ManagedSstFileWriter extends SstFileWriter { public ManagedSstFileWriter(EnvOptions envOptions, Options options) { super(envOptions, options); + LEAK_DETECTOR.watch(this); } @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java index 4cbd6f98287b..3eb8f12c3975 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.Statistics; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,15 @@ /** * Managed Statistics. */ -public class ManagedStatistics extends Statistics { - +public class ManagedStatistics extends Statistics implements Leakable { + public ManagedStatistics() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java index 51c9bcc0df02..a7775cb73309 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.WriteBatch; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,22 +31,22 @@ /** * Managed WriteBatch. */ -public class ManagedWriteBatch extends WriteBatch { +public class ManagedWriteBatch extends WriteBatch implements Leakable { @Nullable private final StackTraceElement[] elements = getStackTrace(); public ManagedWriteBatch() { - super(); + LEAK_DETECTOR.watch(this); } public ManagedWriteBatch(byte[] data) { super(data); + LEAK_DETECTOR.watch(this); } @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java index 7ba05a0ee6d9..33bb74283365 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java @@ -18,10 +18,12 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import org.apache.hadoop.hdds.resource.Leakable; import org.rocksdb.WriteOptions; import javax.annotation.Nullable; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; @@ -29,14 +31,15 @@ /** * Managed {@link WriteOptions}. */ -public class ManagedWriteOptions extends WriteOptions { - +public class ManagedWriteOptions extends WriteOptions implements Leakable { + public ManagedWriteOptions() { + LEAK_DETECTOR.watch(this); + } @Nullable private final StackTraceElement[] elements = getStackTrace(); @Override - protected void finalize() throws Throwable { + public void check() { assertClosed(this, formatStackTrace(elements)); - super.finalize(); } } diff --git a/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java b/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java new file mode 100644 index 000000000000..33972d60c2e0 --- /dev/null +++ b/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java @@ -0,0 +1,58 @@ +package org.apache.hadoop.hdds.utils.db.managed; + +import org.apache.commons.lang3.RandomUtils; +import org.apache.hadoop.hdds.utils.NativeLibraryLoader; +import org.apache.ozone.test.tag.Native; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.Test; + +import java.util.function.Supplier; + +import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class TestRocksObjectLeakDetector { + + @Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME) + @Test + public void testLeakDetector() throws Exception { + Assumptions.assumeTrue(NativeLibraryLoader.getInstance().loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)); + + testLeakDetector(ManagedBloomFilter::new); + testLeakDetector(ManagedColumnFamilyOptions::new); + testLeakDetector(ManagedEnvOptions::new); + testLeakDetector(ManagedFlushOptions::new); + testLeakDetector(ManagedIngestExternalFileOptions::new); + testLeakDetector(() -> new ManagedLRUCache(1l)); + testLeakDetector(ManagedOptions::new); + testLeakDetector(ManagedReadOptions::new); + testLeakDetector(() -> new ManagedSlice(RandomUtils.nextBytes(10))); + testLeakDetector(ManagedStatistics::new); + testLeakDetector(ManagedWriteBatch::new); + testLeakDetector(ManagedWriteOptions::new); + } + + private void testLeakDetector(Supplier supplier) throws Exception { + // base metrics + long managedObjects = ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects(); + long leakObjects = ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects(); + + // Allocate and close. + allocate(ManagedBloomFilter::new, true); + System.gc(); + assertEquals(managedObjects + 1, ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects()); + assertEquals(leakObjects, ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects()); + + // Allocate and not close. + allocate(supplier, false); + assertEquals(managedObjects + 2, ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects()); + assertEquals(leakObjects + 1, ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects()); + } + + private void allocate(Supplier supplier, boolean close) throws Exception { + T object = supplier.get(); + if (close) { + object.close(); + } + } +} From a5bace9d18f7de6a97b3f49bf303dc5927ceca08 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Wed, 20 Dec 2023 10:34:23 -0800 Subject: [PATCH 02/18] Fix test. --- .../hdds/utils/db/managed/TestRocksObjectLeakDetector.java | 1 + 1 file changed, 1 insertion(+) diff --git a/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java b/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java index 33972d60c2e0..c6d78fc60cb7 100644 --- a/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java +++ b/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java @@ -45,6 +45,7 @@ private void testLeakDetector(Supplier supplier) th // Allocate and not close. allocate(supplier, false); + System.gc(); assertEquals(managedObjects + 2, ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects()); assertEquals(leakObjects + 1, ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects()); } From ba3c85b84c40b3425befe52892d57805fa90d639 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Thu, 21 Dec 2023 12:07:46 -0800 Subject: [PATCH 03/18] Correct approach. --- .../hadoop/hdds/resource/LeakDetector.java | 74 ++++++++++++++----- .../{Leakable.java => LeakTracker.java} | 33 ++++++--- .../db/TestRDBStoreByteArrayIterator.java | 24 ------ .../utils/db/managed/ManagedBloomFilter.java | 18 ++--- .../managed/ManagedColumnFamilyOptions.java | 17 ++--- .../managed/ManagedCompactRangeOptions.java | 18 ++--- .../utils/db/managed/ManagedDBOptions.java | 18 ++--- .../utils/db/managed/ManagedEnvOptions.java | 18 ++--- .../utils/db/managed/ManagedFlushOptions.java | 17 ++--- .../ManagedIngestExternalFileOptions.java | 19 ++--- .../utils/db/managed/ManagedLRUCache.java | 15 ++-- .../hdds/utils/db/managed/ManagedObject.java | 25 ++----- .../hdds/utils/db/managed/ManagedOptions.java | 21 ++---- .../utils/db/managed/ManagedReadOptions.java | 19 ++--- .../db/managed/ManagedRocksObjectUtils.java | 25 +++---- .../hdds/utils/db/managed/ManagedSlice.java | 24 +++--- .../db/managed/ManagedSstFileReader.java | 10 +-- .../managed/ManagedSstFileReaderIterator.java | 9 +-- .../db/managed/ManagedSstFileWriter.java | 16 ++-- .../utils/db/managed/ManagedStatistics.java | 17 ++--- .../utils/db/managed/ManagedWriteBatch.java | 17 ++--- .../utils/db/managed/ManagedWriteOptions.java | 17 ++--- .../hdds/utils/db/managed/package-info.java | 10 +-- .../src/test/java/TestRefQueue.java | 62 ++++++++++++++++ .../hadoop/ozone/MiniOzoneClusterImpl.java | 2 +- 25 files changed, 273 insertions(+), 272 deletions(-) rename hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/{Leakable.java => LeakTracker.java} (57%) create mode 100644 hadoop-ozone/integration-test/src/test/java/TestRefQueue.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index 54907bbfc031..e923087a8311 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -1,33 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ package org.apache.hadoop.hdds.resource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.lang.ref.ReferenceQueue; -import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; /** - * Simple general resource leak detector using {@link ReferenceQueue} to observe resource object life-cycle - * and perform assertions to verify proper resource closure after they are GCed. + * Simple general resource leak detector using {@link ReferenceQueue} and {@link java.lang.ref.WeakReference} to observe + * resource object life-cycle and assert proper resource closure before they are GCed. + * + *

+ * Example usage: * *

 {@code
- * class MyResource implements Leakable {
+ * class MyResource implements AutoClosable {
+ *   private final LeakTracker leakTracker;
  *   public MyResource() {
- *     ResourceLeakDetector.LEAK_DETECTOR.watch(this);
+ *     leakTracker = MyResourceLeakDetector.LEAK_DETECTOR.track(this, () -> {
+ *       // report leaks, don't refer to the original object (MyResource) here.
+ *       System.out.println("MyResource is not closed before being discarded.");
+ *     });
  *   }
- *   public void check() {
- *     // assertions to verify this resource closure.
+ *   @Override
+ *   public void close() {
+ *     // proper resources closure cleanup...
+ *     // inform tracker this object is closed properly.
+ *     leakTracker.close();
  *   }
  * }
- * class ResourceLeakDetector {
- *    public final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
+ * class MyResourceLeakDetector {
+ *    public static final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
  * }
  * }
- * @see Leakable */ public class LeakDetector implements Runnable { public static final Logger LOG = LoggerFactory.getLogger(LeakDetector.class); - private final ReferenceQueue queue = new ReferenceQueue<>(); + private final ReferenceQueue queue = new ReferenceQueue<>(); + private final Set allLeaks = Collections.newSetFromMap(new ConcurrentHashMap<>()); private final String name; public LeakDetector(String name) { @@ -39,6 +69,7 @@ private void start() { Thread t = new Thread(this); t.setName(LeakDetector.class.getSimpleName() + "-" + name); t.setDaemon(true); + LOG.info("Starting leak detector thread {}.", name); t.start(); } @@ -46,13 +77,14 @@ private void start() { public void run() { while (true) { try { - WeakReference ref = (WeakReference) queue.remove(); - Leakable leakable = ref.get(); - if (leakable != null) { - leakable.check(); + LeakTracker tracker = (LeakTracker) queue.remove(); + // Original resource already been GCed, if tracker is not closed yet, + // report a leak. + if (allLeaks.remove(tracker)) { + tracker.reportLeak(); } } catch (InterruptedException e) { - LOG.warn("Thread interrupted, exiting."); + LOG.warn("Thread interrupted, exiting.", e); break; } } @@ -60,8 +92,12 @@ public void run() { LOG.warn("Existing leak detector {}.", name); } - public void watch(Leakable leakable) { - // Intentionally ignored. - WeakReference ignored = new WeakReference(leakable, queue); + public LeakTracker track(Object leakable, Runnable reportLeak) { + // A rate filter can be put here to only track a subset of all objects, e.g. 5%, 10%, + // if we have proofs that leak tracking impacts performance. + // For now, it looks simple and effective enough. + LeakTracker tracker = new LeakTracker(leakable, queue, allLeaks, reportLeak); + allLeaks.add(tracker); + return tracker; } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java similarity index 57% rename from hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java rename to hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java index e835975a37de..50d80ba525f6 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/Leakable.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java @@ -18,20 +18,33 @@ */ package org.apache.hadoop.hdds.resource; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.Set; + /** - * Interface for a leakable resource. This interface should be implemented by the resource to monitor itself. - * - * Creating proxy objects, e.g. using lambda, like below does not work because we expect the resource object - * itself is monitored, not the immediate proxy object. - *
 {@code
- * LEAK_DETECTOR.watch(() -> { assertClose(resource) });
- * }
+ * A token to track resource closure. * * @see LeakDetector */ -public interface Leakable { +public class LeakTracker extends WeakReference { + private final Set allLeaks; + private final Runnable leakReporter; + LeakTracker(Object referent, ReferenceQueue referenceQueue, + Set allLeaks, Runnable leakReporter) { + super(referent, referenceQueue); + this.allLeaks = allLeaks; + this.leakReporter = leakReporter; + } + /** - * Perform assertions to verify proper resource closure. This is invoked after the resource object is GCed. + * Called by the track resource when it closes. */ - void check(); + public void close() { + allLeaks.remove(this); + } + + void reportLeak() { + leakReporter.run(); + } } diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java index 4efeb2c59093..e56f4ec03a01 100644 --- a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreByteArrayIterator.java @@ -304,28 +304,4 @@ public void testNormalPrefixedIterator() throws IOException { iter.close(); } - - @Test - public void testGetStackTrace() { - ManagedRocksIterator iterator = mock(ManagedRocksIterator.class); - RocksIterator mock = mock(RocksIterator.class); - when(iterator.get()).thenReturn(mock); - when(mock.isOwningHandle()).thenReturn(true); - ManagedRocksObjectUtils.assertClosed(iterator); - verify(iterator, times(1)).getStackTrace(); - - iterator = new ManagedRocksIterator(rocksDBIteratorMock); - - // construct the expected trace. - StackTraceElement[] traceElements = Thread.currentThread().getStackTrace(); - StringBuilder sb = new StringBuilder(); - // first 2 lines will differ. - for (int i = 2; i < traceElements.length; i++) { - sb.append(traceElements[i]); - sb.append("\n"); - } - String expectedTrace = sb.toString(); - String fromObjectInit = iterator.getStackTrace(); - assertTrue(fromObjectInit.contains(expectedTrace)); - } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java index 4814d950d7e2..e4bd244a743d 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java @@ -18,29 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.BloomFilter; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed BloomFilter. */ -public class ManagedBloomFilter extends BloomFilter implements Leakable { - public ManagedBloomFilter() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedBloomFilter extends BloomFilter { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java index 1f1043b458d8..911ba85371df 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java @@ -18,22 +18,19 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.TableFormatConfig; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed ColumnFamilyOptions. */ -public class ManagedColumnFamilyOptions extends ColumnFamilyOptions implements Leakable { - +public class ManagedColumnFamilyOptions extends ColumnFamilyOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); @@ -42,14 +39,13 @@ public class ManagedColumnFamilyOptions extends ColumnFamilyOptions implements L * instances. */ private boolean reused = false; + private final LeakTracker leakTracker = track(this, elements); public ManagedColumnFamilyOptions() { - LEAK_DETECTOR.watch(this); } public ManagedColumnFamilyOptions(ColumnFamilyOptions columnFamilyOptions) { super(columnFamilyOptions); - LEAK_DETECTOR.watch(this); } @Override @@ -88,8 +84,9 @@ public boolean isReused() { } @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } /** diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java index 4f8562ad42e4..e83f0596fee4 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java @@ -18,29 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.CompactRangeOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed CompactRangeOptions. */ -public class ManagedCompactRangeOptions extends CompactRangeOptions implements Leakable { - public ManagedCompactRangeOptions() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedCompactRangeOptions extends CompactRangeOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java index 4276d1377af4..d37cc76ebc28 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java @@ -18,29 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.DBOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed DBOptions. */ -public class ManagedDBOptions extends DBOptions implements Leakable { - public ManagedDBOptions() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedDBOptions extends DBOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java index ef8e015dd0a3..f5f2bc97959d 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java @@ -18,29 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.EnvOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed EnvOptions. */ -public class ManagedEnvOptions extends EnvOptions implements Leakable { - public ManagedEnvOptions() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedEnvOptions extends EnvOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java index f7ebf440a914..6e22ca50b4a0 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java @@ -18,28 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.FlushOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed FlushOptions. */ -public class ManagedFlushOptions extends FlushOptions implements Leakable { - public ManagedFlushOptions() { - LEAK_DETECTOR.watch(this); - } +public class ManagedFlushOptions extends FlushOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java index 2f5305cb0491..d7560be7feeb 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java @@ -18,30 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.IngestExternalFileOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed IngestExternalFileOptions. */ -public class ManagedIngestExternalFileOptions extends IngestExternalFileOptions implements - Leakable { - public ManagedIngestExternalFileOptions() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedIngestExternalFileOptions extends IngestExternalFileOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java index 3b250e2cead3..967a4ec6ce90 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java @@ -18,30 +18,29 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.LRUCache; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed LRUCache. */ -public class ManagedLRUCache extends LRUCache implements Leakable { +public class ManagedLRUCache extends LRUCache { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); public ManagedLRUCache(long capacity) { super(capacity); - LEAK_DETECTOR.watch(this); } @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java index 1ee7938d0263..3141dedc34dc 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java @@ -18,28 +18,26 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.RocksObject; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * General template for a managed RocksObject. * @param */ -class ManagedObject implements AutoCloseable, Leakable { +class ManagedObject implements AutoCloseable { private final T original; - @Nullable - private final StackTraceElement[] elements; + private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); ManagedObject(T original) { this.original = original; - this.elements = ManagedRocksObjectUtils.getStackTrace(); - LEAK_DETECTOR.watch(this); } public T get() { @@ -49,15 +47,6 @@ public T get() { @Override public void close() { original.close(); + leakTracker.close(); } - - @Override - public void check() { - ManagedRocksObjectUtils.assertClosed(this); - } - - public String getStackTrace() { - return formatStackTrace(elements); - } - } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java index 865af6856e8e..1107aa429e4c 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java @@ -18,29 +18,22 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Options; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Options. */ -public class ManagedOptions extends Options implements Leakable { - public ManagedOptions() { - LEAK_DETECTOR.watch(this); - } - - @Nullable +public class ManagedOptions extends Options { private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java index 7dcbd230a930..1428dd786664 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java @@ -18,30 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.ReadOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed {@link ReadOptions}. */ -public class ManagedReadOptions extends ReadOptions implements Leakable { - public ManagedReadOptions() { - LEAK_DETECTOR.watch(this); - } - +public class ManagedReadOptions extends ReadOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } - } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index a51dc50bf2ed..685dd671fe5f 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -18,8 +18,10 @@ */ package org.apache.hadoop.hdds.utils.db.managed; +import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.resource.LeakDetector; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.awaitility.Awaitility; import org.awaitility.core.ConditionTimeoutException; import org.rocksdb.RocksDB; @@ -43,34 +45,29 @@ private ManagedRocksObjectUtils() { public static final Logger LOG = LoggerFactory.getLogger(ManagedRocksObjectUtils.class); - static final LeakDetector LEAK_DETECTOR = new LeakDetector("ManagedRocksObject"); - private static final Duration POLL_DELAY_DURATION = Duration.ZERO; private static final Duration POLL_INTERVAL_DURATION = Duration.ofMillis(100); - public static void assertClosed(ManagedObject object) { - assertClosed(object.get(), object.getStackTrace()); - } - static void assertClosed(RocksObject rocksObject, String stackTrace) { + private static final LeakDetector LEAK_DETECTOR = new LeakDetector("ManagedRocksObject"); + + static LeakTracker track(AutoCloseable object, @Nullable StackTraceElement[] stackTrace) { ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject(); - if (rocksObject.isOwningHandle()) { - reportLeak(rocksObject, stackTrace); - } + final String name = object.getClass().getSimpleName(); + return LEAK_DETECTOR.track(object, () -> reportLeak(name, formatStackTrace(stackTrace))); } - static void reportLeak(Object object, String stackTrace) { + static void reportLeak(String name, String stackTrace) { ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject(); - String warning = String.format("%s is not closed properly", - object.getClass().getSimpleName()); + String warning = String.format("%s is not closed properly", name); if (stackTrace != null && LOG.isDebugEnabled()) { - String debugMessage = String - .format("%nStackTrace for unclosed instance: %s", stackTrace); + String debugMessage = String.format("%nStackTrace for unclosed instance: %s", stackTrace); warning = warning.concat(debugMessage); } LOG.warn(warning); } + static @Nullable StackTraceElement[] getStackTrace() { return HddsUtils.getStackTrace(LOG); } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java index 69be32936b01..d00dcbcb3b60 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java @@ -18,26 +18,23 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Slice; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Slice. */ -public class ManagedSlice extends Slice implements Leakable { - +public class ManagedSlice extends Slice { @Nullable - private final StackTraceElement[] elements; + private final StackTraceElement[] elements = ManagedRocksObjectUtils.getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); public ManagedSlice(byte[] data) { super(data); - this.elements = ManagedRocksObjectUtils.getStackTrace(); - LEAK_DETECTOR.watch(this); } @Override @@ -46,11 +43,10 @@ public synchronized long getNativeHandle() { } @Override - public void check() { - ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject(); - if (isOwningHandle()) { - ManagedRocksObjectUtils.reportLeak(this, formatStackTrace(elements)); - } + protected void disposeInternal() { + super.disposeInternal(); + // RocksMutableObject.close is final and can't be decorated. + // So, we decorate disposeInternal instead. + leakTracker.close(); } - } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReader.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReader.java index 7ba1001a432e..b49c6e7a9e49 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReader.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReader.java @@ -29,15 +29,7 @@ public class ManagedSstFileReader extends ManagedObject { super(original); } - public static ManagedSstFileReader managed( - SstFileReader reader) { + public static ManagedSstFileReader managed(SstFileReader reader) { return new ManagedSstFileReader(reader); } - - @Override - protected void finalize() throws Throwable { - ManagedRocksObjectUtils.assertClosed(this); - super.finalize(); - } - } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReaderIterator.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReaderIterator.java index 0916e89571b1..660a7c347aee 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReaderIterator.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileReaderIterator.java @@ -23,19 +23,12 @@ /** * Managed SstFileReaderIterator. */ -public class ManagedSstFileReaderIterator - extends ManagedObject { +public class ManagedSstFileReaderIterator extends ManagedObject { ManagedSstFileReaderIterator(SstFileReaderIterator original) { super(original); } - @Override - protected void finalize() throws Throwable { - ManagedRocksObjectUtils.assertClosed(this); - super.finalize(); - } - public static ManagedSstFileReaderIterator managed( SstFileReaderIterator iterator) { return new ManagedSstFileReaderIterator(iterator); diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java index 35bd7de86964..0c3d5dba1704 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java @@ -18,34 +18,32 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.EnvOptions; import org.rocksdb.Options; import org.rocksdb.SstFileWriter; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed SstFileWriter. */ -public class ManagedSstFileWriter extends SstFileWriter implements Leakable { - +public class ManagedSstFileWriter extends SstFileWriter { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); public ManagedSstFileWriter(EnvOptions envOptions, Options options) { super(envOptions, options); - LEAK_DETECTOR.watch(this); } @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java index 3eb8f12c3975..6a80a1a7a1ad 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java @@ -18,28 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Statistics; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Statistics. */ -public class ManagedStatistics extends Statistics implements Leakable { - public ManagedStatistics() { - LEAK_DETECTOR.watch(this); - } +public class ManagedStatistics extends Statistics { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java index a7775cb73309..bb741bad7e8f 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java @@ -18,35 +18,32 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.WriteBatch; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed WriteBatch. */ -public class ManagedWriteBatch extends WriteBatch implements Leakable { - +public class ManagedWriteBatch extends WriteBatch { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); public ManagedWriteBatch() { - LEAK_DETECTOR.watch(this); } public ManagedWriteBatch(byte[] data) { super(data); - LEAK_DETECTOR.watch(this); } @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java index 33bb74283365..319590e83910 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java @@ -18,28 +18,25 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.Leakable; +import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.WriteOptions; import javax.annotation.Nullable; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.LEAK_DETECTOR; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.assertClosed; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.formatStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; +import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed {@link WriteOptions}. */ -public class ManagedWriteOptions extends WriteOptions implements Leakable { - public ManagedWriteOptions() { - LEAK_DETECTOR.watch(this); - } +public class ManagedWriteOptions extends WriteOptions { @Nullable private final StackTraceElement[] elements = getStackTrace(); + private final LeakTracker leakTracker = track(this, elements); @Override - public void check() { - assertClosed(this, formatStackTrace(elements)); + public void close() { + super.close(); + leakTracker.close(); } } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/package-info.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/package-info.java index c60dd4c82ec9..ee214f8150ed 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/package-info.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/package-info.java @@ -18,18 +18,16 @@ */ /** - * RocksDB is deprecating the RocksObject's finalizer that cleans up native - * resources. In fact, the finalizer is removed in the new version of RocksDB - * as per https://github.com/facebook/rocksdb/commit/99d86252b. That poses a + * RocksDB has deprecated the RocksObject's finalizer that cleans up native + * resources, see https://github.com/facebook/rocksdb/commit/99d86252b. That poses a * requirement for RocksDb's applications to explicitly close RocksObject * instances themselves to avoid leaking native resources. The general approach * is to close RocksObjects with try-with-resource statement. * Yet, this is not always an easy option in Ozone we need a mechanism to * manage and detect leaks. * - * This package contains wrappers and utilities to catch RocksObject - * instantiates in Ozone, intercept their finalizers and assert if the created - * instances are closed properly before being GCed. + * This package contains RocksObject decorators and utilities to catch track RocksObject's + * lifecycle to ensure they're properly closed before being GCed. */ package org.apache.hadoop.hdds.utils.db.managed; diff --git a/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java b/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java new file mode 100644 index 000000000000..5cd7075a4593 --- /dev/null +++ b/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java @@ -0,0 +1,62 @@ +import org.jooq.meta.derby.sys.Sys; + +import java.io.IOException; +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; +import java.util.Collections; +import java.util.IdentityHashMap; +import java.util.Set; + +public class TestRefQueue { + static ReferenceQueue MY_QUEUE = new ReferenceQueue<>(); + static Set> refs = Collections.newSetFromMap(new IdentityHashMap<>()); + + public static void main(String[] args) throws IOException { + Thread t = new Thread(() -> { + while (true) { + MyReference r = null; + try { + r = (MyReference) MY_QUEUE.remove(); + refs.remove(r); + } catch (InterruptedException e) { + break; + } + System.out.println(r.name + " eligible for collection"); + } + System.out.println("Escaping checker"); + }); + t.setDaemon(true); + t.start(); + + new MyObject(); + new MyObject(); + new MyObject(); + + System.gc(); + System.in.read(); + + } + + static class MyObject { + MyObject() { + //normal init... + MyReference ref = new MyReference(this, MY_QUEUE); + refs.add(ref); + } + + @Override + protected void finalize() throws Throwable { + System.out.println("Finalize object"); + } + } + static class MyReference extends WeakReference { + public final String name; + + public MyReference(MyObject o, ReferenceQueue q) { + super(o, q); + name = o.toString(); + } + } + + +} diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index e2e737356964..fed0a3fd0bf4 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -448,13 +448,13 @@ public String getClusterId() { public void shutdown() { try { LOG.info("Shutting down the Mini Ozone Cluster"); + System.gc(); IOUtils.closeQuietly(clients); final File baseDir = new File(getBaseDir()); stop(); FileUtils.deleteDirectory(baseDir); ContainerCache.getInstance(conf).shutdownCache(); DefaultMetricsSystem.shutdown(); - ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks(); CodecTestUtil.gc(); } catch (Exception e) { From 6e6ca6c8756bd83196702ada8e96b36e24c5dcc3 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Thu, 21 Dec 2023 12:50:00 -0800 Subject: [PATCH 04/18] Testing leak detection. --- .../hadoop/hdds/resource/LeakDetector.java | 5 +- .../managed/TestRocksObjectLeakDetector.java | 52 ++++++++++++++++--- .../hadoop/ozone/MiniOzoneClusterImpl.java | 1 + 3 files changed, 48 insertions(+), 10 deletions(-) rename {hadoop-hdds/rocks-native => hadoop-ozone/integration-test}/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java (56%) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index e923087a8311..5a51f9f4713a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -94,8 +94,9 @@ public void run() { public LeakTracker track(Object leakable, Runnable reportLeak) { // A rate filter can be put here to only track a subset of all objects, e.g. 5%, 10%, - // if we have proofs that leak tracking impacts performance. - // For now, it looks simple and effective enough. + // if we have proofs that leak tracking impacts performance, or a single LeakDetector + // thread can't keep up with the pace of object allocation. + // For now, it looks effective enough and let keep it simple. LeakTracker tracker = new LeakTracker(leakable, queue, allLeaks, reportLeak); allLeaks.add(tracker); return tracker; diff --git a/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java similarity index 56% rename from hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java rename to hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java index c6d78fc60cb7..1888d4afc1c7 100644 --- a/hadoop-hdds/rocks-native/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java @@ -1,23 +1,56 @@ package org.apache.hadoop.hdds.utils.db.managed; import org.apache.commons.lang3.RandomUtils; -import org.apache.hadoop.hdds.utils.NativeLibraryLoader; -import org.apache.ozone.test.tag.Native; -import org.junit.jupiter.api.Assumptions; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.ozone.MiniOzoneCluster; +import org.apache.ozone.test.tag.Unhealthy; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import java.io.IOException; +import java.util.UUID; +import java.util.concurrent.TimeoutException; import java.util.function.Supplier; -import static org.apache.hadoop.hdds.utils.NativeConstants.ROCKS_TOOLS_NATIVE_LIBRARY_NAME; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +/** + * Test managed rocks object leak detection. + * This test creates garbage that will fail other tests and is intended for manual run only. + * It is flaky as well because of depending on background processes and environment. + */ +@Unhealthy public class TestRocksObjectLeakDetector { - @Native(ROCKS_TOOLS_NATIVE_LIBRARY_NAME) + private static MiniOzoneCluster cluster; + + @BeforeAll + static void setUp() throws IOException, InterruptedException, + TimeoutException { + OzoneConfiguration conf = new OzoneConfiguration(); + String clusterId = UUID.randomUUID().toString(); + String scmId = UUID.randomUUID().toString(); + String omServiceId = "omServiceId1"; + cluster = MiniOzoneCluster.newBuilder(conf) + .setClusterId(clusterId) + .setScmId(scmId) + .setOMServiceId(omServiceId) + .setNumOfOzoneManagers(1) + .build(); + cluster.waitForClusterToBeReady(); + } + + @AfterAll + static void cleanUp() { + if (cluster != null) { + assertThrows(AssertionError.class, () -> cluster.shutdown()); + } + } + @Test public void testLeakDetector() throws Exception { - Assumptions.assumeTrue(NativeLibraryLoader.getInstance().loadLibrary(ROCKS_TOOLS_NATIVE_LIBRARY_NAME)); - testLeakDetector(ManagedBloomFilter::new); testLeakDetector(ManagedColumnFamilyOptions::new); testLeakDetector(ManagedEnvOptions::new); @@ -38,14 +71,17 @@ private void testLeakDetector(Supplier supplier) th long leakObjects = ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects(); // Allocate and close. - allocate(ManagedBloomFilter::new, true); + allocate(supplier, true); System.gc(); + // it could take a while for leak detection to run in the background. + Thread.sleep(500); assertEquals(managedObjects + 1, ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects()); assertEquals(leakObjects, ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects()); // Allocate and not close. allocate(supplier, false); System.gc(); + Thread.sleep(500); assertEquals(managedObjects + 2, ManagedRocksObjectMetrics.INSTANCE.totalManagedObjects()); assertEquals(leakObjects + 1, ManagedRocksObjectMetrics.INSTANCE.totalLeakObjects()); } diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java index fed0a3fd0bf4..7be05902daf5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java @@ -448,6 +448,7 @@ public String getClusterId() { public void shutdown() { try { LOG.info("Shutting down the Mini Ozone Cluster"); + // Explicit gc to trigger leak detection. System.gc(); IOUtils.closeQuietly(clients); final File baseDir = new File(getBaseDir()); From 3045642106ebfcbf11a44fca14b3668476100333 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Thu, 21 Dec 2023 13:12:42 -0800 Subject: [PATCH 05/18] Cleanup. --- .../hadoop/hdds/resource/LeakDetector.java | 13 ++-- .../hadoop/hdds/resource/LeakTracker.java | 2 +- .../db/managed/ManagedRocksObjectUtils.java | 1 - .../hdds/utils/db/managed/ManagedSlice.java | 7 ++- .../src/test/java/TestRefQueue.java | 62 ------------------- .../managed/TestRocksObjectLeakDetector.java | 3 +- 6 files changed, 13 insertions(+), 75 deletions(-) delete mode 100644 hadoop-ozone/integration-test/src/test/java/TestRefQueue.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index 5a51f9f4713a..c260ec9a72c7 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -35,13 +35,11 @@ * *
 {@code
  * class MyResource implements AutoClosable {
- *   private final LeakTracker leakTracker;
- *   public MyResource() {
- *     leakTracker = MyResourceLeakDetector.LEAK_DETECTOR.track(this, () -> {
- *       // report leaks, don't refer to the original object (MyResource) here.
- *       System.out.println("MyResource is not closed before being discarded.");
- *     });
- *   }
+ *   private final LeakTracker MyResourceLeakDetector.LEAK_DETECTOR.track(this, () -> {
+ *      // report leaks, don't refer to the original object (MyResource) here.
+ *      System.out.println("MyResource is not closed before being discarded.");
+ *   });
+ *
  *   @Override
  *   public void close() {
  *     // proper resources closure cleanup...
@@ -49,6 +47,7 @@
  *     leakTracker.close();
  *   }
  * }
+ *
  * class MyResourceLeakDetector {
  *    public static final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
  * }
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java
index 50d80ba525f6..a08784176f22 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java
@@ -38,7 +38,7 @@ public class LeakTracker extends WeakReference {
   }
 
   /**
-   * Called by the track resource when it closes.
+   * Called by the tracked resource closing.
    */
   public void close() {
     allLeaks.remove(this);
diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
index 685dd671fe5f..7a381609ccf3 100644
--- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
+++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
@@ -67,7 +67,6 @@ static void reportLeak(String name, String stackTrace) {
     LOG.warn(warning);
   }
 
-
   static @Nullable StackTraceElement[] getStackTrace() {
     return HddsUtils.getStackTrace(LOG);
   }
diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java
index d00dcbcb3b60..c57c914fcd50 100644
--- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java
+++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java
@@ -23,6 +23,7 @@
 
 import javax.annotation.Nullable;
 
+import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace;
 import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track;
 
 /**
@@ -30,7 +31,7 @@
  */
 public class ManagedSlice extends Slice {
   @Nullable
-  private final StackTraceElement[] elements = ManagedRocksObjectUtils.getStackTrace();
+  private final StackTraceElement[] elements = getStackTrace();
   private final LeakTracker leakTracker = track(this, elements);
 
   public ManagedSlice(byte[] data) {
@@ -45,8 +46,8 @@ public synchronized long getNativeHandle() {
   @Override
   protected void disposeInternal() {
     super.disposeInternal();
-    // RocksMutableObject.close is final and can't be decorated.
-    // So, we decorate disposeInternal instead.
+    // RocksMutableObject.close is final thus can't be decorated.
+    // So, we decorate disposeInternal instead to track closure.
     leakTracker.close();
   }
 }
diff --git a/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java b/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java
deleted file mode 100644
index 5cd7075a4593..000000000000
--- a/hadoop-ozone/integration-test/src/test/java/TestRefQueue.java
+++ /dev/null
@@ -1,62 +0,0 @@
-import org.jooq.meta.derby.sys.Sys;
-
-import java.io.IOException;
-import java.lang.ref.ReferenceQueue;
-import java.lang.ref.WeakReference;
-import java.util.Collections;
-import java.util.IdentityHashMap;
-import java.util.Set;
-
-public class TestRefQueue {
-  static ReferenceQueue MY_QUEUE = new ReferenceQueue<>();
-  static Set> refs = Collections.newSetFromMap(new IdentityHashMap<>());
-
-  public static void main(String[] args) throws IOException {
-    Thread t = new Thread(() -> {
-      while (true) {
-        MyReference r = null;
-        try {
-          r = (MyReference) MY_QUEUE.remove();
-          refs.remove(r);
-        } catch (InterruptedException e) {
-          break;
-        }
-        System.out.println(r.name + " eligible for collection");
-      }
-      System.out.println("Escaping checker");
-    });
-    t.setDaemon(true);
-    t.start();
-
-    new MyObject();
-    new MyObject();
-    new MyObject();
-
-    System.gc();
-    System.in.read();
-
-  }
-
-  static class MyObject {
-    MyObject() {
-      //normal init...
-      MyReference ref = new MyReference(this, MY_QUEUE);
-      refs.add(ref);
-    }
-
-    @Override
-    protected void finalize() throws Throwable {
-      System.out.println("Finalize object");
-    }
-  }
-  static class MyReference extends WeakReference {
-    public final String name;
-
-    public MyReference(MyObject o, ReferenceQueue q) {
-      super(o, q);
-      name = o.toString();
-    }
-  }
-
-
-}
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
index 1888d4afc1c7..2e90fa37f8fc 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
@@ -19,7 +19,8 @@
 /**
  * Test managed rocks object leak detection.
  * This test creates garbage that will fail other tests and is intended for manual run only.
- * It is flaky as well because of depending on background processes and environment.
+ * It is also flaky because of depending on background processes and environment (other background tasks
+ * can create extra managed rocks objects and thus fails the counter assertions).
  */
 @Unhealthy
 public class TestRocksObjectLeakDetector {

From 65477d5dc3ae41f34a5efb2d46bcaf8a09bbb125 Mon Sep 17 00:00:00 2001
From: Duong Nguyen 
Date: Thu, 21 Dec 2023 13:15:39 -0800
Subject: [PATCH 06/18] Cleanup.

---
 .../main/java/org/apache/hadoop/hdds/resource/LeakDetector.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java
index c260ec9a72c7..642a26aa66c5 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java
@@ -42,7 +42,7 @@
  *
  *   @Override
  *   public void close() {
- *     // proper resources closure cleanup...
+ *     // proper resources cleanup...
  *     // inform tracker this object is closed properly.
  *     leakTracker.close();
  *   }

From e4c250087ebf60a63aec54417e588af8423a763a Mon Sep 17 00:00:00 2001
From: Duong Nguyen 
Date: Thu, 21 Dec 2023 13:19:58 -0800
Subject: [PATCH 07/18] Checksylte.

---
 .../hadoop/hdds/resource/package-info.java    |  2 +-
 .../db/managed/ManagedRocksObjectUtils.java   |  2 --
 .../managed/TestRocksObjectLeakDetector.java  | 20 ++++++++++++++++++-
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java
index 378c2304f5c7..762119c603ef 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java
@@ -19,4 +19,4 @@
 
 /**
  * Contain utilities for resource management.
- */
\ No newline at end of file
+ */
diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
index 7a381609ccf3..d0e047846d39 100644
--- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
+++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java
@@ -18,14 +18,12 @@
  */
 package org.apache.hadoop.hdds.utils.db.managed;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hdds.HddsUtils;
 import org.apache.hadoop.hdds.resource.LeakDetector;
 import org.apache.hadoop.hdds.resource.LeakTracker;
 import org.awaitility.Awaitility;
 import org.awaitility.core.ConditionTimeoutException;
 import org.rocksdb.RocksDB;
-import org.rocksdb.RocksObject;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
index 2e90fa37f8fc..87fbe23ac761 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/utils/db/managed/TestRocksObjectLeakDetector.java
@@ -1,3 +1,21 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
 package org.apache.hadoop.hdds.utils.db.managed;
 
 import org.apache.commons.lang3.RandomUtils;
@@ -57,7 +75,7 @@ public void testLeakDetector() throws Exception {
     testLeakDetector(ManagedEnvOptions::new);
     testLeakDetector(ManagedFlushOptions::new);
     testLeakDetector(ManagedIngestExternalFileOptions::new);
-    testLeakDetector(() -> new ManagedLRUCache(1l));
+    testLeakDetector(() -> new ManagedLRUCache(1L));
     testLeakDetector(ManagedOptions::new);
     testLeakDetector(ManagedReadOptions::new);
     testLeakDetector(() -> new ManagedSlice(RandomUtils.nextBytes(10)));

From 96f1e2d2e3f58c285e7b608f6e6d7916f9aceee4 Mon Sep 17 00:00:00 2001
From: Duong Nguyen 
Date: Thu, 21 Dec 2023 13:39:37 -0800
Subject: [PATCH 08/18] Use GC from CodecTestUtil.

---
 .../java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java    | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
index 7be05902daf5..50c89dd3e688 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java
@@ -448,8 +448,7 @@ public String getClusterId() {
   public void shutdown() {
     try {
       LOG.info("Shutting down the Mini Ozone Cluster");
-      // Explicit gc to trigger leak detection.
-      System.gc();
+      CodecTestUtil.gc();
       IOUtils.closeQuietly(clients);
       final File baseDir = new File(getBaseDir());
       stop();
@@ -457,7 +456,6 @@ public void shutdown() {
       ContainerCache.getInstance(conf).shutdownCache();
       DefaultMetricsSystem.shutdown();
       ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks();
-      CodecTestUtil.gc();
     } catch (Exception e) {
       LOG.error("Exception while shutting down the cluster.", e);
     }

From d87f173561ad1f20cdbb29ee6fa4af5795b8d5c5 Mon Sep 17 00:00:00 2001
From: Duong Nguyen 
Date: Thu, 21 Dec 2023 13:57:24 -0800
Subject: [PATCH 09/18] Unit test for LeakDetector.

---
 .../hdds/resource/TestLeakDetector.java       | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java

diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java
new file mode 100644
index 000000000000..768a4aa68f16
--- /dev/null
+++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * 

+ * http://www.apache.org/licenses/LICENSE-2.0 + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdds.resource; + +import org.junit.jupiter.api.Test; + +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Test LeakDetector. + */ +public class TestLeakDetector { + private static final LeakDetector LEAK_DETECTOR = new LeakDetector("test"); + private AtomicInteger leaks = new AtomicInteger(0); + + @Test + public void testLeakDetector() throws Exception { + // create and close resource => no leaks. + createResource(true); + System.gc(); + Thread.sleep(100); + assertEquals(0, leaks.get()); + + // create and not close => leaks. + createResource(false); + System.gc(); + Thread.sleep(100); + assertEquals(1, leaks.get()); + } + + private void createResource(boolean close) throws Exception { + MyResource resource = new MyResource(leaks); + if (close) { + resource.close(); + } + } + + private static class MyResource implements AutoCloseable { + private final LeakTracker leakTracker; + + private MyResource(final AtomicInteger leaks) { + leakTracker = LEAK_DETECTOR.track(this, () -> leaks.incrementAndGet());; + } + + @Override + public void close() throws Exception { + leakTracker.close(); + } + } +} From ceaf506aba2eaa2a4c4df2bf49860de720218145 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Thu, 21 Dec 2023 14:22:48 -0800 Subject: [PATCH 10/18] checkstyle. --- .../org/apache/hadoop/hdds/resource/TestLeakDetector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java index 768a4aa68f16..2b57440f90e9 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java @@ -52,11 +52,11 @@ private void createResource(boolean close) throws Exception { } } - private static class MyResource implements AutoCloseable { + private static final class MyResource implements AutoCloseable { private final LeakTracker leakTracker; private MyResource(final AtomicInteger leaks) { - leakTracker = LEAK_DETECTOR.track(this, () -> leaks.incrementAndGet());; + leakTracker = LEAK_DETECTOR.track(this, () -> leaks.incrementAndGet()); } @Override From 0eb40313c710a3d74f1ba5c73148055d5fef77dd Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Thu, 21 Dec 2023 19:12:25 -0800 Subject: [PATCH 11/18] Correct docs. --- .../apache/hadoop/hdds/resource/LeakDetector.java | 13 ++++++------- .../apache/hadoop/hdds/resource/LeakTracker.java | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index 642a26aa66c5..b39273627d8c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -27,15 +27,17 @@ import java.util.concurrent.ConcurrentHashMap; /** - * Simple general resource leak detector using {@link ReferenceQueue} and {@link java.lang.ref.WeakReference} to observe - * resource object life-cycle and assert proper resource closure before they are GCed. + * Simple general resource leak detector using {@link ReferenceQueue} and {@link java.lang.ref.WeakReference} to + * observe resource object life-cycle and assert proper resource closure before they are GCed. * *

* Example usage: * *

 {@code
  * class MyResource implements AutoClosable {
- *   private final LeakTracker MyResourceLeakDetector.LEAK_DETECTOR.track(this, () -> {
+ *   static final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
+ *
+ *   private final LeakTracker leakTracker = LEAK_DETECTOR.track(this, () -> {
  *      // report leaks, don't refer to the original object (MyResource) here.
  *      System.out.println("MyResource is not closed before being discarded.");
  *   });
@@ -43,14 +45,11 @@
  *   @Override
  *   public void close() {
  *     // proper resources cleanup...
- *     // inform tracker this object is closed properly.
+ *     // inform tracker that this object is closed properly.
  *     leakTracker.close();
  *   }
  * }
  *
- * class MyResourceLeakDetector {
- *    public static final LeakDetector LEAK_DETECTOR = new LeakDetector("MyResource");
- * }
  * }
*/ public class LeakDetector implements Runnable { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java index a08784176f22..ad3b665ae6d0 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java @@ -38,7 +38,7 @@ public class LeakTracker extends WeakReference { } /** - * Called by the tracked resource closing. + * Called by the tracked resource when closing. */ public void close() { allLeaks.remove(this); From 79b0ea681795b253d1aeba844027d278a6d6da9e Mon Sep 17 00:00:00 2001 From: "Doroszlai, Attila" <6454655+adoroszlai@users.noreply.github.com> Date: Fri, 22 Dec 2023 16:15:37 +0100 Subject: [PATCH 12/18] Fix typo and dangling javadoc --- .../java/org/apache/hadoop/hdds/resource/LeakDetector.java | 2 +- .../java/org/apache/hadoop/hdds/resource/LeakTracker.java | 2 +- .../java/org/apache/hadoop/hdds/resource/package-info.java | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index b39273627d8c..74c9ea4d4ab9 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -87,7 +87,7 @@ public void run() { } } - LOG.warn("Existing leak detector {}.", name); + LOG.warn("Exiting leak detector {}.", name); } public LeakTracker track(Object leakable, Runnable reportLeak) { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java index ad3b665ae6d0..f767eca8d3be 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java @@ -30,7 +30,7 @@ public class LeakTracker extends WeakReference { private final Set allLeaks; private final Runnable leakReporter; - LeakTracker(Object referent, ReferenceQueue referenceQueue, + LeakTracker(Object referent, ReferenceQueue referenceQueue, Set allLeaks, Runnable leakReporter) { super(referent, referenceQueue); this.allLeaks = allLeaks; diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java index 762119c603ef..86f1370f609d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java @@ -1,4 +1,4 @@ -/** +/* * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -15,8 +15,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.hadoop.hdds.resource; /** - * Contain utilities for resource management. + * Contains utilities for resource management. */ +package org.apache.hadoop.hdds.resource; From 333e6a1dcf1ab287d226225ce765c4988042957b Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Fri, 22 Dec 2023 17:46:38 -0800 Subject: [PATCH 13/18] Reduce code duplications. --- .../org/apache/hadoop/hdds/resource/LeakDetector.java | 7 +++---- .../hadoop/hdds/utils/db/managed/ManagedBloomFilter.java | 7 +------ .../hdds/utils/db/managed/ManagedColumnFamilyOptions.java | 8 +------- .../hdds/utils/db/managed/ManagedCompactRangeOptions.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedDBOptions.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedEnvOptions.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedFlushOptions.java | 7 +------ .../db/managed/ManagedIngestExternalFileOptions.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedLRUCache.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedObject.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedOptions.java | 4 +--- .../hadoop/hdds/utils/db/managed/ManagedReadOptions.java | 7 +------ .../hdds/utils/db/managed/ManagedRocksObjectUtils.java | 5 +++-- .../apache/hadoop/hdds/utils/db/managed/ManagedSlice.java | 7 +------ .../hdds/utils/db/managed/ManagedSstFileWriter.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedStatistics.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedWriteBatch.java | 7 +------ .../hadoop/hdds/utils/db/managed/ManagedWriteOptions.java | 7 +------ 18 files changed, 22 insertions(+), 100 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java index 74c9ea4d4ab9..bbc54ca4fa24 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java @@ -52,7 +52,7 @@ * * } */ -public class LeakDetector implements Runnable { +public class LeakDetector { public static final Logger LOG = LoggerFactory.getLogger(LeakDetector.class); private final ReferenceQueue queue = new ReferenceQueue<>(); private final Set allLeaks = Collections.newSetFromMap(new ConcurrentHashMap<>()); @@ -64,15 +64,14 @@ public LeakDetector(String name) { } private void start() { - Thread t = new Thread(this); + Thread t = new Thread(this::run); t.setName(LeakDetector.class.getSimpleName() + "-" + name); t.setDaemon(true); LOG.info("Starting leak detector thread {}.", name); t.start(); } - @Override - public void run() { + private void run() { while (true) { try { LeakTracker tracker = (LeakTracker) queue.remove(); diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java index e4bd244a743d..7a168ec57619 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.BloomFilter; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed BloomFilter. */ public class ManagedBloomFilter extends BloomFilter { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java index 911ba85371df..d5339252bfa4 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java @@ -22,24 +22,18 @@ import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.TableFormatConfig; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed ColumnFamilyOptions. */ public class ManagedColumnFamilyOptions extends ColumnFamilyOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - /** * Indicate if this ColumnFamilyOptions is intentionally used across RockDB * instances. */ private boolean reused = false; - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); public ManagedColumnFamilyOptions() { } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java index e83f0596fee4..9d3d8a1105ee 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.CompactRangeOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed CompactRangeOptions. */ public class ManagedCompactRangeOptions extends CompactRangeOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java index d37cc76ebc28..0f5e5ebe2d1f 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.DBOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed DBOptions. */ public class ManagedDBOptions extends DBOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java index f5f2bc97959d..804c8885e5ee 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.EnvOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed EnvOptions. */ public class ManagedEnvOptions extends EnvOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java index 6e22ca50b4a0..16fe2a8f8084 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.FlushOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed FlushOptions. */ public class ManagedFlushOptions extends FlushOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java index d7560be7feeb..5bbc631ddbb2 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.IngestExternalFileOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed IngestExternalFileOptions. */ public class ManagedIngestExternalFileOptions extends IngestExternalFileOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java index 967a4ec6ce90..be203ada35ae 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.LRUCache; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed LRUCache. */ public class ManagedLRUCache extends LRUCache { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); public ManagedLRUCache(long capacity) { super(capacity); diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java index 3141dedc34dc..52b8eb322ae9 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java @@ -21,9 +21,6 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.RocksObject; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** @@ -32,9 +29,7 @@ */ class ManagedObject implements AutoCloseable { private final T original; - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); ManagedObject(T original) { this.original = original; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java index 1107aa429e4c..d5e99310b318 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java @@ -21,15 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Options; -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Options. */ public class ManagedOptions extends Options { - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java index 1428dd786664..66b6229d473d 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.ReadOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed {@link ReadOptions}. */ public class ManagedReadOptions extends ReadOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index d0e047846d39..4a1ffb659156 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -49,9 +49,10 @@ private ManagedRocksObjectUtils() { private static final LeakDetector LEAK_DETECTOR = new LeakDetector("ManagedRocksObject"); - static LeakTracker track(AutoCloseable object, @Nullable StackTraceElement[] stackTrace) { + static LeakTracker track(AutoCloseable object) { ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject(); final String name = object.getClass().getSimpleName(); + final StackTraceElement[] stackTrace = getStackTrace(); return LEAK_DETECTOR.track(object, () -> reportLeak(name, formatStackTrace(stackTrace))); } @@ -65,7 +66,7 @@ static void reportLeak(String name, String stackTrace) { LOG.warn(warning); } - static @Nullable StackTraceElement[] getStackTrace() { + private static @Nullable StackTraceElement[] getStackTrace() { return HddsUtils.getStackTrace(LOG); } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java index c57c914fcd50..689a1bc1fe33 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Slice; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Slice. */ public class ManagedSlice extends Slice { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); public ManagedSlice(byte[] data) { super(data); diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java index 0c3d5dba1704..77386106d521 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java @@ -23,18 +23,13 @@ import org.rocksdb.Options; import org.rocksdb.SstFileWriter; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed SstFileWriter. */ public class ManagedSstFileWriter extends SstFileWriter { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); public ManagedSstFileWriter(EnvOptions envOptions, Options options) { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java index 6a80a1a7a1ad..4240b2f742f7 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.Statistics; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed Statistics. */ public class ManagedStatistics extends Statistics { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java index bb741bad7e8f..013ad7087574 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.WriteBatch; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed WriteBatch. */ public class ManagedWriteBatch extends WriteBatch { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); public ManagedWriteBatch() { } diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java index 319590e83910..b1e8e154cb0a 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java @@ -21,18 +21,13 @@ import org.apache.hadoop.hdds.resource.LeakTracker; import org.rocksdb.WriteOptions; -import javax.annotation.Nullable; - -import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.getStackTrace; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; /** * Managed {@link WriteOptions}. */ public class ManagedWriteOptions extends WriteOptions { - @Nullable - private final StackTraceElement[] elements = getStackTrace(); - private final LeakTracker leakTracker = track(this, elements); + private final LeakTracker leakTracker = track(this); @Override public void close() { From 69bf9d55fad520cbf0ca8851f8c5428461367c04 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Sat, 23 Dec 2023 15:00:00 -0800 Subject: [PATCH 14/18] Update startIndex. --- .../hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index 4a1ffb659156..980131d2bcd5 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -71,7 +71,7 @@ static void reportLeak(String name, String stackTrace) { } static String formatStackTrace(@Nullable StackTraceElement[] elements) { - return HddsUtils.formatStackTrace(elements, 3); + return HddsUtils.formatStackTrace(elements, 4); } /** From 8e19927f23a3190f74f96697aa9178ceb12b7dc1 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Tue, 2 Jan 2024 18:45:51 -0800 Subject: [PATCH 15/18] Move LeakDetector to utils package. --- .../org/apache/hadoop/hdds/resource/TestLeakDetector.java | 2 ++ .../hadoop/hdds/utils/db/managed/ManagedBloomFilter.java | 2 +- .../hdds/utils/db/managed/ManagedColumnFamilyOptions.java | 2 +- .../hdds/utils/db/managed/ManagedCompactRangeOptions.java | 2 +- .../apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedEnvOptions.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedFlushOptions.java | 2 +- .../utils/db/managed/ManagedIngestExternalFileOptions.java | 2 +- .../apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java | 2 +- .../apache/hadoop/hdds/utils/db/managed/ManagedObject.java | 2 +- .../apache/hadoop/hdds/utils/db/managed/ManagedOptions.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedReadOptions.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java | 4 ++-- .../org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedStatistics.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedWriteBatch.java | 2 +- .../hadoop/hdds/utils/db/managed/ManagedWriteOptions.java | 2 +- 18 files changed, 20 insertions(+), 18 deletions(-) diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java index 2b57440f90e9..4a60fcc8a4d5 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/resource/TestLeakDetector.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hdds.resource; +import org.apache.hadoop.hdds.utils.LeakDetector; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.junit.jupiter.api.Test; import java.util.concurrent.atomic.AtomicInteger; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java index 7a168ec57619..ffee7c1f5519 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedBloomFilter.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.BloomFilter; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java index d5339252bfa4..7b1da6a16923 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedColumnFamilyOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.ColumnFamilyOptions; import org.rocksdb.TableFormatConfig; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java index 9d3d8a1105ee..0e397ed0e9b6 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedCompactRangeOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.CompactRangeOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java index 0f5e5ebe2d1f..fa01e2e1018b 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedDBOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.DBOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java index 804c8885e5ee..baad1ad7f4ca 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedEnvOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.EnvOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java index 16fe2a8f8084..126f5336ba90 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedFlushOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.FlushOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java index 5bbc631ddbb2..1783a34587cf 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedIngestExternalFileOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.IngestExternalFileOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java index be203ada35ae..5244863a5a17 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedLRUCache.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.LRUCache; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java index 52b8eb322ae9..1e4068a7a800 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedObject.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.RocksObject; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java index d5e99310b318..e438068e3a70 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.Options; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java index 66b6229d473d..48c2238ec4a0 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedReadOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.ReadOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index 980131d2bcd5..7117d8eeb1c9 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -19,8 +19,8 @@ package org.apache.hadoop.hdds.utils.db.managed; import org.apache.hadoop.hdds.HddsUtils; -import org.apache.hadoop.hdds.resource.LeakDetector; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakDetector; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.awaitility.Awaitility; import org.awaitility.core.ConditionTimeoutException; import org.rocksdb.RocksDB; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java index 689a1bc1fe33..8c366bdaa423 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSlice.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.Slice; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java index 77386106d521..de7e9d526634 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedSstFileWriter.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.EnvOptions; import org.rocksdb.Options; import org.rocksdb.SstFileWriter; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java index 4240b2f742f7..75af8b881355 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedStatistics.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.Statistics; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java index 013ad7087574..b1411b09a49a 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteBatch.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.WriteBatch; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java index b1e8e154cb0a..5d32a290b5e2 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedWriteOptions.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hdds.utils.db.managed; -import org.apache.hadoop.hdds.resource.LeakTracker; +import org.apache.hadoop.hdds.utils.LeakTracker; import org.rocksdb.WriteOptions; import static org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectUtils.track; From 7c9ddf866f1bbe3f76b5abee63d5c59bb0314880 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Tue, 2 Jan 2024 18:48:43 -0800 Subject: [PATCH 16/18] Move LeakDetector to utils package. --- .../apache/hadoop/hdds/{resource => utils}/LeakDetector.java | 2 +- .../org/apache/hadoop/hdds/{resource => utils}/LeakTracker.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/{resource => utils}/LeakDetector.java (98%) rename hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/{resource => utils}/LeakTracker.java (97%) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakDetector.java similarity index 98% rename from hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java rename to hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakDetector.java index bbc54ca4fa24..67f5c2f2bbc5 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakDetector.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakDetector.java @@ -16,7 +16,7 @@ * limitations under the License. * */ -package org.apache.hadoop.hdds.resource; +package org.apache.hadoop.hdds.utils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakTracker.java similarity index 97% rename from hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java rename to hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakTracker.java index f767eca8d3be..6103d520ca8a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/LeakTracker.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LeakTracker.java @@ -16,7 +16,7 @@ * limitations under the License. * */ -package org.apache.hadoop.hdds.resource; +package org.apache.hadoop.hdds.utils; import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; From 263fb651c37a122bd9fe5953579c36d2188c47ba Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Tue, 2 Jan 2024 22:26:53 -0800 Subject: [PATCH 17/18] Avoid calling getSimpleName for each ManagedObject creation. --- .../hdds/utils/db/managed/ManagedRocksObjectUtils.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java index 7117d8eeb1c9..7ae7001ccd39 100644 --- a/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java +++ b/hadoop-hdds/managed-rocksdb/src/main/java/org/apache/hadoop/hdds/utils/db/managed/ManagedRocksObjectUtils.java @@ -51,14 +51,14 @@ private ManagedRocksObjectUtils() { static LeakTracker track(AutoCloseable object) { ManagedRocksObjectMetrics.INSTANCE.increaseManagedObject(); - final String name = object.getClass().getSimpleName(); + final Class clazz = object.getClass(); final StackTraceElement[] stackTrace = getStackTrace(); - return LEAK_DETECTOR.track(object, () -> reportLeak(name, formatStackTrace(stackTrace))); + return LEAK_DETECTOR.track(object, () -> reportLeak(clazz, formatStackTrace(stackTrace))); } - static void reportLeak(String name, String stackTrace) { + static void reportLeak(Class clazz, String stackTrace) { ManagedRocksObjectMetrics.INSTANCE.increaseLeakObject(); - String warning = String.format("%s is not closed properly", name); + String warning = String.format("%s is not closed properly", clazz.getSimpleName()); if (stackTrace != null && LOG.isDebugEnabled()) { String debugMessage = String.format("%nStackTrace for unclosed instance: %s", stackTrace); warning = warning.concat(debugMessage); From 63304d005a9dfc45bab1c138254730570b3b2e45 Mon Sep 17 00:00:00 2001 From: Duong Nguyen Date: Tue, 2 Jan 2024 22:28:05 -0800 Subject: [PATCH 18/18] Remove package-info. --- .../hadoop/hdds/resource/package-info.java | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java deleted file mode 100644 index 86f1370f609d..000000000000 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/resource/package-info.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * Contains utilities for resource management. - */ -package org.apache.hadoop.hdds.resource;