This is an automated email from the ASF dual-hosted git repository.
ritesh pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 2dd528c9e2 HDDS-9295. Avoid overriding finalize() in CodecBuffer.
(#5302)
2dd528c9e2 is described below
commit 2dd528c9e2c51e831a76f987d1eef332e3135bad
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Wed Sep 20 23:34:12 2023 -0700
HDDS-9295. Avoid overriding finalize() in CodecBuffer. (#5302)
---
.../apache/hadoop/hdds/utils/db/CodecBuffer.java | 54 +++++++++++++++++++---
.../apache/hadoop/hdds/utils/db/CodecTestUtil.java | 2 +-
.../hadoop/hdds/utils/db/TestLeakDetector.java | 43 +++++++++++++++++
.../container/common/TestBlockDeletingService.java | 2 +
.../container/keyvalue/TestKeyValueContainer.java | 2 +
.../hadoop/hdds/utils/TestRDBSnapshotProvider.java | 2 +
.../org/apache/hadoop/hdds/utils/db/TestCodec.java | 4 ++
.../apache/hadoop/hdds/utils/db/TestRDBStore.java | 2 +
.../utils/db/TestRDBStoreCodecBufferIterator.java | 1 +
.../hdds/utils/db/TestTypedRDBTableStore.java | 2 +
.../apache/hadoop/ozone/MiniOzoneClusterImpl.java | 9 +++-
11 files changed, 114 insertions(+), 9 deletions(-)
diff --git
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java
index 6a404c3e75..7d16abb816 100644
---
a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java
+++
b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/db/CodecBuffer.java
@@ -37,6 +37,7 @@ import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
import java.util.function.IntFunction;
import java.util.function.ToIntFunction;
@@ -47,9 +48,43 @@ import static org.apache.hadoop.hdds.HddsUtils.getStackTrace;
* A buffer used by {@link Codec}
* for supporting RocksDB direct {@link ByteBuffer} APIs.
*/
-public final class CodecBuffer implements AutoCloseable {
+public class CodecBuffer implements AutoCloseable {
public static final Logger LOG = LoggerFactory.getLogger(CodecBuffer.class);
+ /** To create {@link CodecBuffer} instances. */
+ private static class Factory {
+ private static volatile Function<ByteBuf, CodecBuffer> constructor
+ = CodecBuffer::new;
+ static void set(Function<ByteBuf, CodecBuffer> f) {
+ constructor = f;
+ LOG.info("Successfully set constructor to " + f);
+ }
+
+ static CodecBuffer newCodecBuffer(ByteBuf buf) {
+ return constructor.apply(buf);
+ }
+ }
+
+ /** To detect buffer leak. */
+ private static class LeakDetector {
+ static CodecBuffer newCodecBuffer(ByteBuf buf) {
+ return new CodecBuffer(buf) {
+ @Override
+ protected void finalize() {
+ detectLeaks();
+ }
+ };
+ }
+ }
+
+ /**
+ * Detect buffer leak in runtime.
+ * Note that there is a severe performance penalty for leak detection.
+ */
+ public static void enableLeakDetection() {
+ Factory.set(LeakDetector::newCodecBuffer);
+ }
+
/** The size of a buffer. */
public static class Capacity {
private final Object name;
@@ -106,7 +141,7 @@ public final class CodecBuffer implements AutoCloseable {
* the buffer's capacity can be increased if necessary.
*/
static CodecBuffer allocate(int capacity, IntFunction<ByteBuf> allocator) {
- return new CodecBuffer(allocator.apply(capacity));
+ return Factory.newCodecBuffer(allocator.apply(capacity));
}
/**
@@ -127,7 +162,7 @@ public final class CodecBuffer implements AutoCloseable {
/** Wrap the given array. */
public static CodecBuffer wrap(byte[] array) {
- return new CodecBuffer(Unpooled.wrappedBuffer(array));
+ return Factory.newCodecBuffer(Unpooled.wrappedBuffer(array));
}
private static final AtomicInteger LEAK_COUNT = new AtomicInteger();
@@ -153,8 +188,16 @@ public final class CodecBuffer implements AutoCloseable {
Preconditions.assertSame(expected, buf.refCnt(), "refCnt");
}
- @Override
- protected void finalize() throws Throwable {
+ /**
+ * Detect buffer leak by asserting that the underlying buffer is released
+ * when this object is garbage collected.
+ * This method may be invoked inside the {@link #finalize()} method
+ * or using a {@link java.lang.ref.ReferenceQueue}.
+ * For performance reason, this class does not override {@link #finalize()}.
+ *
+ * @see #enableLeakDetection()
+ */
+ void detectLeaks() {
// leak detection
final int capacity = buf.capacity();
if (!released.isDone() && capacity > 0) {
@@ -169,7 +212,6 @@ public final class CodecBuffer implements AutoCloseable {
buf.release(refCnt);
}
}
- super.finalize();
}
@Override
diff --git
a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/CodecTestUtil.java
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/CodecTestUtil.java
index 785737ff57..d139a4a72e 100644
---
a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/CodecTestUtil.java
+++
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/CodecTestUtil.java
@@ -36,7 +36,7 @@ public final class CodecTestUtil {
/**
* Force gc to check leakage.
*/
- static void gc() throws InterruptedException {
+ public static void gc() throws InterruptedException {
// use WeakReference to detect gc
Object obj = new Object();
final WeakReference<Object> weakRef = new WeakReference<>(obj);
diff --git
a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/TestLeakDetector.java
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/TestLeakDetector.java
new file mode 100644
index 0000000000..42422d0dff
--- /dev/null
+++
b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/utils/db/TestLeakDetector.java
@@ -0,0 +1,43 @@
+/*
+ * 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;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test {@link CodecBuffer.LeakDetector}.
+ */
+public final class TestLeakDetector {
+ @Test
+ public void test() throws Exception {
+ CodecBuffer.enableLeakDetection();
+ // allocate a buffer and then release it.
+ CodecBuffer.allocateHeap(2).release();
+ // no leak at this point.
+ CodecTestUtil.gc();
+
+ // allocate a buffer but NOT release it.
+ CodecBuffer.allocateHeap(3);
+ // It should detect a buffer leak.
+ final AssertionError e = Assertions.assertThrows(
+ AssertionError.class, CodecTestUtil::gc);
+ e.printStackTrace(System.out);
+ Assertions.assertTrue(e.getMessage().startsWith("Found 1 leak"));
+ }
+}
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
index b4efd6a27c..080300bcb8 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
@@ -143,6 +143,8 @@ public class TestBlockDeletingService {
@Before
public void init() throws IOException {
+ CodecBuffer.enableLeakDetection();
+
testRoot = GenericTestUtils
.getTestDir(TestBlockDeletingService.class.getSimpleName());
if (testRoot.exists()) {
diff --git
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
index 6dd5b1411f..5c241e7cc5 100644
---
a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
+++
b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueContainer.java
@@ -145,6 +145,8 @@ public class TestKeyValueContainer {
@Before
public void setUp() throws Exception {
+ CodecBuffer.enableLeakDetection();
+
DatanodeConfiguration dc = CONF.getObject(DatanodeConfiguration.class);
dc.setAutoCompactionSmallSstFileNum(100);
dc.setRocksdbDeleteObsoleteFilesPeriod(5000);
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java
index 585c635d5a..7a1ebbdb7e 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java
@@ -88,6 +88,8 @@ public class TestRDBSnapshotProvider {
@BeforeEach
public void init(@TempDir File tempDir) throws Exception {
+ CodecBuffer.enableLeakDetection();
+
options = getNewDBOptions();
configSet = new HashSet<>();
for (String name : families) {
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestCodec.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestCodec.java
index cdcef15a86..1a0559d94f 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestCodec.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestCodec.java
@@ -43,6 +43,10 @@ public final class TestCodec {
static final Logger LOG = LoggerFactory.getLogger(TestCodec.class);
static final int NUM_LOOPS = 10;
+ static {
+ CodecBuffer.enableLeakDetection();
+ }
+
@Test
public void testShortCodec() throws Exception {
runTestShortCodec((short)0);
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java
index 7831e03efd..b0a71cea91 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStore.java
@@ -75,6 +75,8 @@ public class TestRDBStore {
@BeforeEach
public void setUp(@TempDir File tempDir) throws Exception {
+ CodecBuffer.enableLeakDetection();
+
options = new ManagedDBOptions();
options.setCreateIfMissing(true);
options.setCreateMissingColumnFamilies(true);
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
index f9806b47d2..5069da4a71 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestRDBStoreCodecBufferIterator.java
@@ -62,6 +62,7 @@ public class TestRDBStoreCodecBufferIterator {
@BeforeEach
public void setup() {
+ CodecBuffer.enableLeakDetection();
rocksIteratorMock = mock(RocksIterator.class);
managedRocksIterator = newManagedRocksIterator();
rdbTableMock = mock(RDBTable.class);
diff --git
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestTypedRDBTableStore.java
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestTypedRDBTableStore.java
index cc249a57b7..5352c14e55 100644
---
a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestTypedRDBTableStore.java
+++
b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/TestTypedRDBTableStore.java
@@ -65,6 +65,8 @@ public class TestTypedRDBTableStore {
@BeforeEach
public void setUp(@TempDir File tempDir) throws Exception {
+ CodecBuffer.enableLeakDetection();
+
options = new ManagedDBOptions();
options.setCreateIfMissing(true);
options.setCreateMissingColumnFamilies(true);
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 42b5402256..cd21063156 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
@@ -66,6 +66,7 @@ import
org.apache.hadoop.hdds.security.symmetric.SecretKeyClient;
import
org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient;
import org.apache.hadoop.hdds.utils.IOUtils;
import org.apache.hadoop.hdds.utils.db.CodecBuffer;
+import org.apache.hadoop.hdds.utils.db.CodecTestUtil;
import org.apache.hadoop.hdds.utils.db.managed.ManagedRocksObjectMetrics;
import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
import org.apache.hadoop.ozone.client.OzoneClient;
@@ -120,6 +121,10 @@ public class MiniOzoneClusterImpl implements
MiniOzoneCluster {
private static final Logger LOG =
LoggerFactory.getLogger(MiniOzoneClusterImpl.class);
+ static {
+ CodecBuffer.enableLeakDetection();
+ }
+
private OzoneConfiguration conf;
private final SCMConfigurator scmConfigurator;
private StorageContainerManager scm;
@@ -449,8 +454,8 @@ public class MiniOzoneClusterImpl implements
MiniOzoneCluster {
DefaultMetricsSystem.shutdown();
ManagedRocksObjectMetrics.INSTANCE.assertNoLeaks();
- CodecBuffer.assertNoLeaks();
- } catch (IOException e) {
+ CodecTestUtil.gc();
+ } catch (Exception e) {
LOG.error("Exception while shutting down the cluster.", e);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
