From 2dcd2ea765e23d71eae0125bcae2acd65cb7a79b Mon Sep 17 00:00:00 2001 From: horizonzy Date: Mon, 12 Jun 2023 17:10:02 +0800 Subject: [PATCH 01/10] Allocator support exitOnOutOfMemory config. --- .../allocator/ByteBufAllocatorBuilder.java | 2 + .../impl/ByteBufAllocatorBuilderImpl.java | 10 ++- .../allocator/impl/ByteBufAllocatorImpl.java | 29 +++++-- .../bookkeeper/common/util/ShutdownUtil.java | 87 +++++++++++++++++++ .../impl/ByteBufAllocatorBuilderTest.java | 15 ++++ .../bookkeeper/bookie/BookieResources.java | 1 + .../apache/bookkeeper/client/BookKeeper.java | 1 + .../conf/AbstractConfiguration.java | 10 +++ .../conf/AbstractConfigurationTest.java | 10 +++ .../tools/perf/journal/JournalWriter.java | 1 + 10 files changed, 158 insertions(+), 8 deletions(-) create mode 100644 bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/ByteBufAllocatorBuilder.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/ByteBufAllocatorBuilder.java index 3e36a23d170..cb244140b35 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/ByteBufAllocatorBuilder.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/ByteBufAllocatorBuilder.java @@ -92,4 +92,6 @@ static ByteBufAllocatorBuilder create() { *

Default is {@link LeakDetectionPolicy#Disabled} */ ByteBufAllocatorBuilder leakDetectionPolicy(LeakDetectionPolicy leakDetectionPolicy); + + ByteBufAllocatorBuilder exitOnOutOfMemory(boolean exitOnOutOfMemory); } diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java index 69c57232aff..34d1c6b750a 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java @@ -18,6 +18,7 @@ package org.apache.bookkeeper.common.allocator.impl; import io.netty.buffer.ByteBufAllocator; + import java.util.function.Consumer; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorBuilder; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorWithOomHandler; @@ -37,11 +38,12 @@ public class ByteBufAllocatorBuilderImpl implements ByteBufAllocatorBuilder { OutOfMemoryPolicy outOfMemoryPolicy = OutOfMemoryPolicy.FallbackToHeap; Consumer outOfMemoryListener = null; LeakDetectionPolicy leakDetectionPolicy = LeakDetectionPolicy.Disabled; + boolean exitOnOutOfMemory = false; @Override public ByteBufAllocatorWithOomHandler build() { return new ByteBufAllocatorImpl(pooledAllocator, unpooledAllocator, poolingPolicy, poolingConcurrency, - outOfMemoryPolicy, outOfMemoryListener, leakDetectionPolicy); + outOfMemoryPolicy, outOfMemoryListener, leakDetectionPolicy, exitOnOutOfMemory); } @Override @@ -86,4 +88,10 @@ public ByteBufAllocatorBuilder leakDetectionPolicy(LeakDetectionPolicy leakDetec return this; } + @Override + public ByteBufAllocatorBuilder exitOnOutOfMemory(boolean exitOnOutOfMemory) { + this.exitOnOutOfMemory = exitOnOutOfMemory; + return this; + } + } diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java index 87582cca92c..5489e110904 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java @@ -24,11 +24,13 @@ import io.netty.buffer.UnpooledByteBufAllocator; import io.netty.util.ResourceLeakDetector; import io.netty.util.ResourceLeakDetector.Level; + import java.util.function.Consumer; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorWithOomHandler; import org.apache.bookkeeper.common.allocator.LeakDetectionPolicy; import org.apache.bookkeeper.common.allocator.OutOfMemoryPolicy; import org.apache.bookkeeper.common.allocator.PoolingPolicy; +import org.apache.bookkeeper.common.util.ShutdownUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,15 +50,16 @@ public class ByteBufAllocatorImpl extends AbstractByteBufAllocator implements By private final PoolingPolicy poolingPolicy; private final OutOfMemoryPolicy outOfMemoryPolicy; private Consumer outOfMemoryListener; + private final boolean exitOnOutOfMemory; ByteBufAllocatorImpl(ByteBufAllocator pooledAllocator, ByteBufAllocator unpooledAllocator, PoolingPolicy poolingPolicy, int poolingConcurrency, OutOfMemoryPolicy outOfMemoryPolicy, Consumer outOfMemoryListener, - LeakDetectionPolicy leakDetectionPolicy) { + LeakDetectionPolicy leakDetectionPolicy, boolean exitOnOutOfMemory) { super(poolingPolicy == PoolingPolicy.PooledDirect /* preferDirect */); - this.poolingPolicy = poolingPolicy; this.outOfMemoryPolicy = outOfMemoryPolicy; + this.exitOnOutOfMemory = exitOnOutOfMemory; if (outOfMemoryListener == null) { this.outOfMemoryListener = (v) -> { log.error("Unable to allocate memory", v); @@ -146,7 +149,7 @@ protected ByteBuf newHeapBuffer(int initialCapacity, int maxCapacity) { : unpooledAllocator; return alloc.heapBuffer(initialCapacity, maxCapacity); } catch (OutOfMemoryError e) { - outOfMemoryListener.accept(e); + consumeOOMError(e); throw e; } } @@ -166,12 +169,12 @@ private ByteBuf newDirectBuffer(int initialCapacity, int maxCapacity, boolean ca try { return unpooledAllocator.heapBuffer(initialCapacity, maxCapacity); } catch (OutOfMemoryError e2) { - outOfMemoryListener.accept(e2); + consumeOOMError(e); throw e2; } } else { // ThrowException - outOfMemoryListener.accept(e); + consumeOOMError(e); throw e; } } @@ -181,12 +184,24 @@ private ByteBuf newDirectBuffer(int initialCapacity, int maxCapacity, boolean ca try { return unpooledAllocator.directBuffer(initialCapacity, maxCapacity); } catch (OutOfMemoryError e) { - outOfMemoryListener.accept(e); - throw e; + consumeOOMError(e); + throw e; } } } + private void consumeOOMError(OutOfMemoryError outOfMemoryError) { + try { + outOfMemoryListener.accept(outOfMemoryError); + } catch (Throwable e) { + log.warn("Consume outOfMemory error failed.", e); + } + if (exitOnOutOfMemory) { + log.info("Exiting JVM process for OOM error: {}", outOfMemoryError.getMessage(), outOfMemoryError); + ShutdownUtil.triggerImmediateForcefulShutdown(); + } + } + @Override public boolean isDirectBufferPooled() { return pooledAllocator != null && pooledAllocator.isDirectBufferPooled(); diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java new file mode 100644 index 00000000000..4f7dab1a2b1 --- /dev/null +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java @@ -0,0 +1,87 @@ +/* + * 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.bookkeeper.common.util; + +import lombok.extern.slf4j.Slf4j; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +/** + * Forked from Pulsar. + */ +@Slf4j +public class ShutdownUtil { + private static final Method log4j2ShutdownMethod; + + static { + // use reflection to find org.apache.logging.log4j.LogManager.shutdown method + Method shutdownMethod = null; + try { + shutdownMethod = Class.forName("org.apache.logging.log4j.LogManager") + .getMethod("shutdown"); + } catch (ClassNotFoundException | NoSuchMethodException e) { + // ignore when Log4j2 isn't found, log at debug level + log.debug("Cannot find org.apache.logging.log4j.LogManager.shutdown method", e); + } + log4j2ShutdownMethod = shutdownMethod; + } + + /** + * Triggers an immediate forceful shutdown of the current process. + * + * @param status Termination status. By convention, a nonzero status code indicates abnormal termination. + * @see Runtime#halt(int) + */ + public static void triggerImmediateForcefulShutdown(int status) { + triggerImmediateForcefulShutdown(status, true); + } + public static void triggerImmediateForcefulShutdown(int status, boolean logging) { + try { + if (status != 0 && logging) { + log.warn("Triggering immediate shutdown of current process with status {}", status, + new Exception("Stacktrace for immediate shutdown")); + } + shutdownLogging(); + } finally { + Runtime.getRuntime().halt(status); + } + } + + private static void shutdownLogging() { + // flush log buffers and shutdown log4j2 logging to prevent log truncation + if (log4j2ShutdownMethod != null) { + try { + // use reflection to call org.apache.logging.log4j.LogManager.shutdown() + log4j2ShutdownMethod.invoke(null); + } catch (IllegalAccessException | InvocationTargetException e) { + log.error("Unable to call org.apache.logging.log4j.LogManager.shutdown using reflection.", e); + } + } + } + + /** + * Triggers an immediate forceful shutdown of the current process using 1 as the status code. + * + * @see Runtime#halt(int) + */ + public static void triggerImmediateForcefulShutdown() { + triggerImmediateForcefulShutdown(1); + } +} \ No newline at end of file diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 6f2538d6c81..6192a2de0ba 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -86,6 +86,21 @@ public void testOomWithException() { // Ensure the notification was triggered even when exception is thrown assertEquals(outOfDirectMemException, receivedException.get()); } + + @Test + public void testOomExit() { + ByteBufAllocator baseAlloc = mock(ByteBufAllocator.class); + when(baseAlloc.directBuffer(anyInt(), anyInt())).thenThrow(outOfDirectMemException); + + ByteBufAllocator alloc = ByteBufAllocatorBuilder.create() + .pooledAllocator(baseAlloc) + .outOfMemoryPolicy(OutOfMemoryPolicy.ThrowException) + .exitOnOutOfMemory(true) + .build(); + + alloc.buffer(); + fail("JVM should exit, shouldn't be there."); + } @Test public void testOomWithFallback() { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieResources.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieResources.java index c9b71b9968d..755efd5be02 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieResources.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieResources.java @@ -70,6 +70,7 @@ public static ByteBufAllocatorWithOomHandler createAllocator(ServerConfiguration .poolingConcurrency(conf.getAllocatorPoolingConcurrency()) .outOfMemoryPolicy(conf.getAllocatorOutOfMemoryPolicy()) .leakDetectionPolicy(conf.getAllocatorLeakDetectionPolicy()) + .exitOnOutOfMemory(conf.exitOnOutOfMemory()) .build(); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java index d7043dc8c9a..0362aadcbaa 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeper.java @@ -478,6 +478,7 @@ public BookKeeper(ClientConfiguration conf, ZooKeeper zk, EventLoopGroup eventLo .poolingConcurrency(conf.getAllocatorPoolingConcurrency()) .outOfMemoryPolicy(conf.getAllocatorOutOfMemoryPolicy()) .leakDetectionPolicy(conf.getAllocatorLeakDetectionPolicy()) + .exitOnOutOfMemory(conf.exitOnOutOfMemory()) .build(); } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java index 438dc40983e..153b8a3db9d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java @@ -183,6 +183,7 @@ public abstract class AbstractConfiguration protected static final String ALLOCATOR_POOLING_CONCURRENCY = "allocatorPoolingConcurrency"; protected static final String ALLOCATOR_OOM_POLICY = "allocatorOutOfMemoryPolicy"; protected static final String ALLOCATOR_LEAK_DETECTION_POLICY = "allocatorLeakDetectionPolicy"; + protected static final String ALLOCATOR_EXIT_ON_OUT_OF_MEMORY = "allocatorExitOnOutOfMemory"; // option to limit stats logging public static final String LIMIT_STATS_LOGGING = "limitStatsLogging"; @@ -1155,6 +1156,15 @@ public T setAllocatorLeakDetectionPolicy(LeakDetectionPolicy leakDetectionPolicy this.setProperty(ALLOCATOR_LEAK_DETECTION_POLICY, leakDetectionPolicy.toString()); return getThis(); } + + public T setExitOnOutOfMemory(boolean exitOnOutOfMemory) { + this.setProperty(ALLOCATOR_EXIT_ON_OUT_OF_MEMORY, exitOnOutOfMemory); + return getThis(); + } + + public boolean exitOnOutOfMemory() { + return getBoolean(ALLOCATOR_EXIT_ON_OUT_OF_MEMORY, false); + } /** * Return whether the busy-wait is enabled for BookKeeper and Netty IO threads. diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java index a6333a47d32..5234d110fe2 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java @@ -19,6 +19,8 @@ package org.apache.bookkeeper.conf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.CALLS_REAL_METHODS; import static org.mockito.Mockito.mock; @@ -179,4 +181,12 @@ public void testAllocatorLeakDetectionPolicy() { System.getProperties().put(nettyLevelKey, nettyLevelStr); } } + + @Test + public void testExitOnOutOfMemory() { + assertFalse(conf.exitOnOutOfMemory()); + conf.setExitOnOutOfMemory(true); + assertTrue(conf.exitOnOutOfMemory()); + } + } diff --git a/tools/perf/src/main/java/org/apache/bookkeeper/tools/perf/journal/JournalWriter.java b/tools/perf/src/main/java/org/apache/bookkeeper/tools/perf/journal/JournalWriter.java index 383ccfb9825..e287fbd94af 100644 --- a/tools/perf/src/main/java/org/apache/bookkeeper/tools/perf/journal/JournalWriter.java +++ b/tools/perf/src/main/java/org/apache/bookkeeper/tools/perf/journal/JournalWriter.java @@ -495,6 +495,7 @@ private static ByteBufAllocator getAllocator(ServerConfiguration conf) { log.error("Unable to allocate memory, exiting bookie", ex); }) .leakDetectionPolicy(conf.getAllocatorLeakDetectionPolicy()) + .exitOnOutOfMemory(conf.exitOnOutOfMemory()) .build(); } From d833b9d703ad1b28bde80122eae686135b18cc47 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 13 Jun 2023 08:53:55 +0800 Subject: [PATCH 02/10] fix checkstyle. --- .../bookkeeper/common/util/package-info.java | 21 +++++++++++++++++++ .../impl/ByteBufAllocatorBuilderTest.java | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/package-info.java diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/package-info.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/package-info.java new file mode 100644 index 00000000000..55031dd8f8d --- /dev/null +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/package-info.java @@ -0,0 +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. + */ +/** + * defines the utilities for allocator used across the project. + */ +package org.apache.bookkeeper.common.util; \ No newline at end of file diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 6192a2de0ba..6ce28060439 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -86,7 +86,7 @@ public void testOomWithException() { // Ensure the notification was triggered even when exception is thrown assertEquals(outOfDirectMemException, receivedException.get()); } - + @Test public void testOomExit() { ByteBufAllocator baseAlloc = mock(ByteBufAllocator.class); From aaca547eac4ebebb67b210fd431ecded45f90b0c Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 13 Jun 2023 09:01:12 +0800 Subject: [PATCH 03/10] Fix checkstyle. --- .../common/allocator/impl/ByteBufAllocatorBuilderImpl.java | 1 - .../bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java | 1 - .../java/org/apache/bookkeeper/common/util/ShutdownUtil.java | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java index 34d1c6b750a..4b5469a3f7e 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderImpl.java @@ -18,7 +18,6 @@ package org.apache.bookkeeper.common.allocator.impl; import io.netty.buffer.ByteBufAllocator; - import java.util.function.Consumer; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorBuilder; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorWithOomHandler; diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java index 5489e110904..93f19d5f7ca 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java @@ -24,7 +24,6 @@ import io.netty.buffer.UnpooledByteBufAllocator; import io.netty.util.ResourceLeakDetector; import io.netty.util.ResourceLeakDetector.Level; - import java.util.function.Consumer; import org.apache.bookkeeper.common.allocator.ByteBufAllocatorWithOomHandler; import org.apache.bookkeeper.common.allocator.LeakDetectionPolicy; diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java index 4f7dab1a2b1..a398b57fe74 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/util/ShutdownUtil.java @@ -18,10 +18,9 @@ package org.apache.bookkeeper.common.util; -import lombok.extern.slf4j.Slf4j; - import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import lombok.extern.slf4j.Slf4j; /** * Forked from Pulsar. From 6368ad102b49ecb77ce095ad177f988493fda9dc Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 13 Jun 2023 09:09:29 +0800 Subject: [PATCH 04/10] Fix checkstyle. --- .../org/apache/bookkeeper/conf/AbstractConfiguration.java | 4 ++-- .../org/apache/bookkeeper/conf/AbstractConfigurationTest.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java index 153b8a3db9d..6d2a82bb551 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java @@ -1156,12 +1156,12 @@ public T setAllocatorLeakDetectionPolicy(LeakDetectionPolicy leakDetectionPolicy this.setProperty(ALLOCATOR_LEAK_DETECTION_POLICY, leakDetectionPolicy.toString()); return getThis(); } - + public T setExitOnOutOfMemory(boolean exitOnOutOfMemory) { this.setProperty(ALLOCATOR_EXIT_ON_OUT_OF_MEMORY, exitOnOutOfMemory); return getThis(); } - + public boolean exitOnOutOfMemory() { return getBoolean(ALLOCATOR_EXIT_ON_OUT_OF_MEMORY, false); } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java index 5234d110fe2..194ab2c68d0 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java @@ -181,12 +181,12 @@ public void testAllocatorLeakDetectionPolicy() { System.getProperties().put(nettyLevelKey, nettyLevelStr); } } - + @Test public void testExitOnOutOfMemory() { assertFalse(conf.exitOnOutOfMemory()); conf.setExitOnOutOfMemory(true); assertTrue(conf.exitOnOutOfMemory()); } - + } From 3fe25b643de7c47ea08da78751b1346296dbcfdb Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 13 Jun 2023 11:55:59 +0800 Subject: [PATCH 05/10] Fix ci. --- bookkeeper-common-allocator/pom.xml | 5 ++++ .../allocator/impl/ByteBufAllocatorImpl.java | 2 +- .../impl/ByteBufAllocatorBuilderTest.java | 27 ++++++++++++++++--- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/bookkeeper-common-allocator/pom.xml b/bookkeeper-common-allocator/pom.xml index 7c6b4654165..71bbbc5f638 100644 --- a/bookkeeper-common-allocator/pom.xml +++ b/bookkeeper-common-allocator/pom.xml @@ -29,6 +29,11 @@ io.netty netty-buffer + + org.mockito + mockito-core + test + diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java index 93f19d5f7ca..31fd70767a7 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java @@ -168,7 +168,7 @@ private ByteBuf newDirectBuffer(int initialCapacity, int maxCapacity, boolean ca try { return unpooledAllocator.heapBuffer(initialCapacity, maxCapacity); } catch (OutOfMemoryError e2) { - consumeOOMError(e); + consumeOOMError(e2); throw e2; } } else { diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 6ce28060439..7da945ca95d 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -24,6 +24,9 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.powermock.api.mockito.PowerMockito.doNothing; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.verifyStatic; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; @@ -35,11 +38,17 @@ import org.apache.bookkeeper.common.allocator.ByteBufAllocatorBuilder; import org.apache.bookkeeper.common.allocator.OutOfMemoryPolicy; import org.apache.bookkeeper.common.allocator.PoolingPolicy; +import org.apache.bookkeeper.common.util.ShutdownUtil; import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; /** * Tests for {@link ByteBufAllocatorBuilderImpl}. */ +@RunWith(PowerMockRunner.class) +@PrepareForTest(ShutdownUtil.class) public class ByteBufAllocatorBuilderTest { private static final OutOfMemoryError outOfDirectMemException; @@ -87,7 +96,7 @@ public void testOomWithException() { assertEquals(outOfDirectMemException, receivedException.get()); } - @Test + @Test() public void testOomExit() { ByteBufAllocator baseAlloc = mock(ByteBufAllocator.class); when(baseAlloc.directBuffer(anyInt(), anyInt())).thenThrow(outOfDirectMemException); @@ -98,8 +107,20 @@ public void testOomExit() { .exitOnOutOfMemory(true) .build(); - alloc.buffer(); - fail("JVM should exit, shouldn't be there."); + mockStatic(ShutdownUtil.class); + doNothing().when(ShutdownUtil.class); + ShutdownUtil.triggerImmediateForcefulShutdown(); + + try { + alloc.buffer(); + fail("Should have thrown exception"); + } catch (OutOfMemoryError e) { + // Expected + assertEquals(outOfDirectMemException, e); + } + + verifyStatic(ShutdownUtil.class); + ShutdownUtil.triggerImmediateForcefulShutdown(); } @Test From ad92acffb3ce13973a9e46ab68a8894923106b1e Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 13 Jun 2023 12:02:29 +0800 Subject: [PATCH 06/10] Fix checkstyle. --- .../common/allocator/impl/ByteBufAllocatorBuilderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 7da945ca95d..6c892e8dc82 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -118,7 +118,7 @@ public void testOomExit() { // Expected assertEquals(outOfDirectMemException, e); } - + verifyStatic(ShutdownUtil.class); ShutdownUtil.triggerImmediateForcefulShutdown(); } From 31c5403bd792dc71b56c00ba4a79f886d63a9735 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 27 Jun 2023 11:20:46 +0800 Subject: [PATCH 07/10] For compatible. --- .../common/allocator/impl/ByteBufAllocatorImpl.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java index 31fd70767a7..3d5103de82c 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java @@ -50,6 +50,14 @@ public class ByteBufAllocatorImpl extends AbstractByteBufAllocator implements By private final OutOfMemoryPolicy outOfMemoryPolicy; private Consumer outOfMemoryListener; private final boolean exitOnOutOfMemory; + + ByteBufAllocatorImpl(ByteBufAllocator pooledAllocator, ByteBufAllocator unpooledAllocator, + PoolingPolicy poolingPolicy, int poolingConcurrency, OutOfMemoryPolicy outOfMemoryPolicy, + Consumer outOfMemoryListener, + LeakDetectionPolicy leakDetectionPolicy) { + this(pooledAllocator, unpooledAllocator, poolingPolicy, poolingConcurrency, outOfMemoryPolicy, + outOfMemoryListener, leakDetectionPolicy, false); + } ByteBufAllocatorImpl(ByteBufAllocator pooledAllocator, ByteBufAllocator unpooledAllocator, PoolingPolicy poolingPolicy, int poolingConcurrency, OutOfMemoryPolicy outOfMemoryPolicy, From 4a7b02a2203a9ddcb9aee1db56e3c8a4deb449ea Mon Sep 17 00:00:00 2001 From: horizonzy Date: Thu, 29 Jun 2023 23:59:14 +0800 Subject: [PATCH 08/10] Fix checkstyle. --- .../bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java index 3d5103de82c..3bc06f8e7ea 100644 --- a/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java +++ b/bookkeeper-common-allocator/src/main/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorImpl.java @@ -50,7 +50,7 @@ public class ByteBufAllocatorImpl extends AbstractByteBufAllocator implements By private final OutOfMemoryPolicy outOfMemoryPolicy; private Consumer outOfMemoryListener; private final boolean exitOnOutOfMemory; - + ByteBufAllocatorImpl(ByteBufAllocator pooledAllocator, ByteBufAllocator unpooledAllocator, PoolingPolicy poolingPolicy, int poolingConcurrency, OutOfMemoryPolicy outOfMemoryPolicy, Consumer outOfMemoryListener, From 2934bc66e73459ca73b71be97e44babd28af5940 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Sat, 13 Jul 2024 15:06:13 +0800 Subject: [PATCH 09/10] fix ci. --- .../impl/ByteBufAllocatorBuilderTest.java | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 6c892e8dc82..733522a3982 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -23,10 +23,8 @@ import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; -import static org.powermock.api.mockito.PowerMockito.doNothing; -import static org.powermock.api.mockito.PowerMockito.mockStatic; -import static org.powermock.api.mockito.PowerMockito.verifyStatic; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufAllocator; @@ -40,15 +38,12 @@ import org.apache.bookkeeper.common.allocator.PoolingPolicy; import org.apache.bookkeeper.common.util.ShutdownUtil; import org.junit.Test; -import org.junit.runner.RunWith; -import org.powermock.core.classloader.annotations.PrepareForTest; -import org.powermock.modules.junit4.PowerMockRunner; +import org.mockito.MockedStatic; +import org.mockito.Mockito; /** * Tests for {@link ByteBufAllocatorBuilderImpl}. */ -@RunWith(PowerMockRunner.class) -@PrepareForTest(ShutdownUtil.class) public class ByteBufAllocatorBuilderTest { private static final OutOfMemoryError outOfDirectMemException; @@ -106,10 +101,8 @@ public void testOomExit() { .outOfMemoryPolicy(OutOfMemoryPolicy.ThrowException) .exitOnOutOfMemory(true) .build(); - - mockStatic(ShutdownUtil.class); - doNothing().when(ShutdownUtil.class); - ShutdownUtil.triggerImmediateForcefulShutdown(); + + MockedStatic mockedStatic = mockStatic(ShutdownUtil.class); try { alloc.buffer(); @@ -118,9 +111,8 @@ public void testOomExit() { // Expected assertEquals(outOfDirectMemException, e); } - - verifyStatic(ShutdownUtil.class); - ShutdownUtil.triggerImmediateForcefulShutdown(); + + mockedStatic.verify(() -> ShutdownUtil.triggerImmediateForcefulShutdown(), Mockito.times(1)); } @Test From 7995b51af7e49b6ad0220e4992701ed9ed594993 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Mon, 15 Jul 2024 11:29:17 +0800 Subject: [PATCH 10/10] code clean. --- .../common/allocator/impl/ByteBufAllocatorBuilderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java index 733522a3982..40c41fa65bc 100644 --- a/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java +++ b/bookkeeper-common-allocator/src/test/java/org/apache/bookkeeper/common/allocator/impl/ByteBufAllocatorBuilderTest.java @@ -101,7 +101,7 @@ public void testOomExit() { .outOfMemoryPolicy(OutOfMemoryPolicy.ThrowException) .exitOnOutOfMemory(true) .build(); - + MockedStatic mockedStatic = mockStatic(ShutdownUtil.class); try { @@ -111,7 +111,7 @@ public void testOomExit() { // Expected assertEquals(outOfDirectMemException, e); } - + mockedStatic.verify(() -> ShutdownUtil.triggerImmediateForcefulShutdown(), Mockito.times(1)); }