Skip to content

Commit b09d290

Browse files
authored
[fix][test] Restore NETTY_LEAK_DETECTION for Gradle and CI tests (#26605)
1 parent 4e9866a commit b09d290

8 files changed

Lines changed: 224 additions & 27 deletions

File tree

.github/workflows/pulsar-ci.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,8 @@ jobs:
493493
JOB_NAME: CI - Integration - ${{ matrix.name }}
494494
PULSAR_TEST_IMAGE_NAME: apachepulsar/java-test-image:latest
495495
CI_JDK_MAJOR_VERSION: ${{ needs.preconditions.outputs.jdk_major_version }}
496+
NETTY_LEAK_DETECTION: "${{ needs.preconditions.outputs.netty_leak_detection }}"
497+
NETTY_LEAK_DUMP_DIR: ${{ github.workspace }}/build/netty-leak-dumps
496498
strategy:
497499
fail-fast: false
498500
matrix:
@@ -617,6 +619,10 @@ jobs:
617619
if: ${{ always() }}
618620
uses: ./.github/actions/copy-test-reports
619621

622+
- name: Report detected Netty leaks
623+
if: ${{ always() && env.NETTY_LEAK_DETECTION != 'off' }}
624+
run: $GITHUB_WORKSPACE/pulsar-build/pulsar_ci_tool.sh report_netty_leaks
625+
620626
- name: Upload test reports
621627
uses: actions/upload-artifact@v7
622628
if: ${{ !success() }}
@@ -638,6 +644,7 @@ jobs:
638644
**/hs_err_*.log
639645
**/core.*
640646
build/threaddumps/
647+
${{ env.NETTY_LEAK_DUMP_DIR }}/*
641648
retention-days: 7
642649
if-no-files-found: ignore
643650

@@ -754,6 +761,8 @@ jobs:
754761
JOB_NAME: CI - System - ${{ matrix.name }}
755762
PULSAR_TEST_IMAGE_NAME: apachepulsar/pulsar-test-latest-version:latest
756763
CI_JDK_MAJOR_VERSION: ${{ needs.preconditions.outputs.jdk_major_version }}
764+
NETTY_LEAK_DETECTION: "${{ needs.preconditions.outputs.netty_leak_detection }}"
765+
NETTY_LEAK_DUMP_DIR: ${{ github.workspace }}/build/netty-leak-dumps
757766
strategy:
758767
fail-fast: false
759768
matrix:
@@ -833,6 +842,10 @@ jobs:
833842
if: ${{ always() }}
834843
uses: ./.github/actions/copy-test-reports
835844

845+
- name: Report detected Netty leaks
846+
if: ${{ always() && env.NETTY_LEAK_DETECTION != 'off' }}
847+
run: $GITHUB_WORKSPACE/pulsar-build/pulsar_ci_tool.sh report_netty_leaks
848+
836849
- name: Upload test reports
837850
uses: actions/upload-artifact@v7
838851
if: ${{ !success() }}
@@ -854,6 +867,7 @@ jobs:
854867
**/hs_err_*.log
855868
**/core.*
856869
build/threaddumps/
870+
${{ env.NETTY_LEAK_DUMP_DIR }}/*
857871
retention-days: 7
858872
if-no-files-found: ignore
859873

CONTRIBUTING.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,28 @@ Failed tests are retried once by default (`testRetryCount=1`; `0` when running i
131131
running tests locally, prefer **`-PtestRetryCount=0`** to catch failures (including flakiness) early
132132
instead of having retries mask them.
133133

134+
### Netty buffer leak detection
135+
136+
Tests enable Netty's `paranoid` leak detection by default, using `ExtendedNettyLeakDetector`
137+
to include test names in leak reports and write `netty_leak_*.txt` files. Set
138+
`NETTY_LEAK_DUMP_DIR` to choose the output directory (the default is the JVM's temporary directory).
139+
140+
`NETTY_LEAK_DETECTION=report` (the default) reports leaks without failing tests.
141+
In CI, `NETTY_LEAK_DETECTION=fail_on_leak` makes the leak-reporting step fail the job when dumps
142+
are found. Unit, integration, and system test jobs collect dumps from both the test JVMs and
143+
Pulsar Docker containers, including reports generated during container shutdown. For local tests, use `-PtestExitJvmOnLeak=true` to fail the test JVM on a detected leak:
144+
145+
```bash
146+
NETTY_LEAK_DUMP_DIR=/tmp/pulsar-netty-leaks ./gradlew :pulsar-client-original:test \
147+
--tests "ConsumerBuilderImplTest" -PtestExitJvmOnLeak=true -PtestRetryCount=0
148+
```
149+
150+
Set `NETTY_LEAK_DETECTION=off` to disable detection, or use
151+
`-PtestLeakDetectionLevel=simple|advanced|paranoid|disabled` to change its level.
152+
`-PtestExitJvmOnLeakDelayMillis=1000` controls the delay before exiting on a leak.
153+
Detection is disabled automatically for `-PtestAsyncProfiler` and `profilingIntegrationTest`
154+
to avoid distorting profiles.
155+
134156
### Micro benchmarks (JMH)
135157

136158
For a **micro**-level question — what a single method, data structure or codec costs — write a

build-logic/conventions/src/main/kotlin/pulsar.java-conventions.gradle.kts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,9 @@ val javaToolchains = extensions.getByType<JavaToolchainService>()
205205
// Effective Java major version used to run tests: the -PtestJavaVersion override when set,
206206
// otherwise the JVM running Gradle.
207207
val testJavaMajorVersion = testJavaVersion.orNull ?: JavaVersion.current().majorVersion.toInt()
208+
val asyncProfilerEnabled = providers.gradleProperty("testAsyncProfiler")
209+
.map { it.isBlank() || it.toBoolean() }
210+
.getOrElse(false)
208211

209212
tasks.withType<Test>().configureEach {
210213
testJavaVersion.orNull?.let { version ->
@@ -248,6 +251,27 @@ tasks.withType<Test>().configureEach {
248251
val defaultTestRetryCount = if (ideaActive) "0" else "1"
249252
systemProperty("testRetryCount", providers.gradleProperty("testRetryCount").getOrElse(defaultTestRetryCount))
250253
systemProperty("testFailFast", failFastValue.toString())
254+
// Restore the test leak detector defaults from the Maven build. CI's report_netty_leaks
255+
// step handles report vs. fail_on_leak after collecting the dumps from all test JVMs.
256+
val nettyLeakDetectionEnabled =
257+
providers.environmentVariable("NETTY_LEAK_DETECTION").getOrElse("report") != "off" && !asyncProfilerEnabled
258+
if (nettyLeakDetectionEnabled) {
259+
systemProperty("io.netty.customResourceLeakDetector", "org.apache.pulsar.tests.ExtendedNettyLeakDetector")
260+
systemProperty("org.apache.pulsar.tests.ExtendedNettyLeakDetector.exitJvmOnLeak",
261+
providers.gradleProperty("testExitJvmOnLeak").getOrElse("false"))
262+
systemProperty("org.apache.pulsar.tests.ExtendedNettyLeakDetector.exitJvmDelayMillis",
263+
providers.gradleProperty("testExitJvmOnLeakDelayMillis").getOrElse("1000"))
264+
systemProperty("io.netty.leakDetection.level",
265+
providers.gradleProperty("testLeakDetectionLevel").getOrElse("paranoid"))
266+
// Track every allocation with less overhead by recording only acquire/release operations.
267+
systemProperty("io.netty.leakDetection.targetRecords", "16")
268+
systemProperty("io.netty.leakDetection.acquireAndReleaseOnly", "true")
269+
systemProperty("io.netty.leakDetection.samplingInterval", "32")
270+
// Process weak references promptly when the test listener triggers leak detection.
271+
jvmArgs("-XX:+UnlockExperimentalVMOptions", "-XX:ReferencesPerThread=0", "-XX:+ParallelRefProcEnabled")
272+
} else {
273+
systemProperty("io.netty.leakDetection.level", "disabled")
274+
}
251275
jvmArgs(
252276
"-XX:+HeapDumpOnOutOfMemoryError",
253277
"-XX:HeapDumpPath=${providers.gradleProperty("testHeapDumpPath").getOrElse("/tmp")}",
@@ -297,9 +321,6 @@ tasks.withType<Test>().configureEach {
297321
// the names of the `testAsyncProfiler` Maven profile that the 4.x branches use. This is a second
298322
// `configureEach` block so that it overrides the settings above, and so that the environment
299323
// variable and JDK lookups it does stay out of the configuration cache inputs when profiling is off.
300-
val asyncProfilerEnabled = providers.gradleProperty("testAsyncProfiler")
301-
.map { it.isBlank() || it.toBoolean() }
302-
.getOrElse(false)
303324
if (asyncProfilerEnabled) {
304325
// Locate the agent library: an explicit -Ptest.asyncprofiler.libpath wins, then the
305326
// LIBASYNCPROFILER_PATH environment variable (the variable microbench/README.md already uses for

pulsar-build/pulsar_ci_tool.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,12 +182,12 @@ ci_report_netty_leaks() {
182182
fi
183183

184184
# check if there are any netty_leak_*.txt files in the container logs
185-
local container_logs_dir="tests/integration/target/container-logs"
185+
local container_logs_dir="tests/integration/build/container-logs"
186186
if [ -d "$container_logs_dir" ]; then
187187
local container_netty_leak_dump_dir="$NETTY_LEAK_DUMP_DIR/container-logs"
188188
mkdir -p "$container_netty_leak_dump_dir"
189189
while read -r file; do
190-
# example file name "tests/integration/target/container-logs/ltnizrzm-standalone/var-log-pulsar.tar.gz"
190+
# example file name "tests/integration/build/container-logs/ltnizrzm-standalone/var-log-pulsar.tar.gz"
191191
# take ltnizrzm-standalone part
192192
container_name=$(basename "$(dirname "$file")")
193193
target_dir="$container_netty_leak_dump_dir/$container_name"

tests/integration/build.gradle.kts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,8 @@ tasks.register<Test>("profilingIntegrationTest") {
259259
"${dockerOrganization}/java-test-image:${dockerTag}-asyncprofiler")
260260
// Leak detection is paranoid by default and would distort the allocation profile.
261261
environment("NETTY_LEAK_DETECTION", "off")
262+
systemProperties.remove("io.netty.customResourceLeakDetector")
263+
systemProperty("io.netty.leakDetection.level", "disabled")
262264

263265
// A retried test would profile the cluster twice into the same run.
264266
systemProperty("testRetryCount", "0")
Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.pulsar.tests.integration.containers;
20+
21+
import static java.nio.charset.StandardCharsets.UTF_8;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import io.netty.buffer.ByteBufAllocator;
24+
import java.nio.file.Files;
25+
import java.nio.file.Path;
26+
import java.time.Duration;
27+
import java.util.UUID;
28+
import java.util.concurrent.CountDownLatch;
29+
import java.util.zip.GZIPInputStream;
30+
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
31+
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
32+
import org.apache.commons.io.FileUtils;
33+
import org.apache.pulsar.tests.ExtendedNettyLeakDetector;
34+
import org.testcontainers.containers.wait.strategy.Wait;
35+
import org.testcontainers.images.builder.Transferable;
36+
import org.testcontainers.utility.MountableFile;
37+
import org.testng.SkipException;
38+
import org.testng.annotations.DataProvider;
39+
import org.testng.annotations.Test;
40+
41+
public class NettyLeakDetectionTest {
42+
@DataProvider
43+
public Object[][] shutdownModes() {
44+
return new Object[][] {{false}, {true}};
45+
}
46+
47+
@Test(dataProvider = "shutdownModes")
48+
public void collectsLeaksReportedDuringShutdown(boolean supervised) throws Exception {
49+
if (!ExtendedNettyLeakDetector.isExtendedNettyLeakDetectorEnabled()
50+
|| !"paranoid".equals(System.getProperty("io.netty.leakDetection.level"))) {
51+
throw new SkipException("Requires the default paranoid test leak detector");
52+
}
53+
var container = new LeakProbeContainer(supervised);
54+
Path logs = Path.of(System.getProperty("buildDirectory", "build"),
55+
"container-logs", container.getContainerName());
56+
try (container) {
57+
container.start();
58+
assertThat(container.execCmd("sh", "-c", "ls /var/log/pulsar/netty_leak_*.txt 2>/dev/null || true")
59+
.getStdout()).as("No leak report before JVM shutdown").isEmpty();
60+
container.stop();
61+
Path archive = logs.resolve("var-log-pulsar.tar.gz");
62+
boolean foundLeak = false;
63+
try (var tar = new TarArchiveInputStream(new GZIPInputStream(Files.newInputStream(archive)))) {
64+
TarArchiveEntry entry;
65+
while ((entry = tar.getNextEntry()) != null) {
66+
if (entry.isFile() && entry.getName().contains("netty_leak_")) {
67+
String report = new String(tar.readAllBytes(), UTF_8);
68+
assertThat(report).contains("Traced leak detected ByteBuf", "container-shutdown-leak");
69+
foundLeak = true;
70+
}
71+
}
72+
}
73+
assertThat(foundLeak).as("Shutdown leak included in the collected container logs").isTrue();
74+
} finally {
75+
// This test deliberately leaks in a separate JVM. Do not report its expected leak in CI.
76+
FileUtils.deleteDirectory(logs.toFile());
77+
}
78+
}
79+
80+
private static class LeakProbeContainer extends PulsarContainer<LeakProbeContainer> {
81+
LeakProbeContainer(boolean supervised) {
82+
super("leak-test-" + UUID.randomUUID(), "probe", "probe",
83+
supervised ? "/usr/bin/supervisord" : "bin/pulsar", INVALID_PORT, INVALID_PORT);
84+
String className = LeakProbe.class.getName();
85+
String resource = className.replace('.', '/') + ".class";
86+
withCopyFileToContainer(MountableFile.forClasspathResource(resource), "/tmp/" + resource);
87+
String script = "#!/bin/sh\nexec java $PULSAR_EXTRA_OPTS -cp '/pulsar/lib/*:/tmp' '"
88+
+ className + "'\n";
89+
if (supervised) {
90+
withCopyToContainer(Transferable.of(script, 0755), "/tmp/leak-probe.sh");
91+
withCopyToContainer(Transferable.of("""
92+
[program:leak-probe]
93+
command=/tmp/leak-probe.sh
94+
autostart=true
95+
autorestart=false
96+
stopwaitsecs=15
97+
"""), "/etc/supervisord/conf.d/leak-probe.conf");
98+
withCommand("-c", "/etc/supervisord.conf");
99+
} else {
100+
// Use the standalone shutdown path with a small JVM instead of starting a broker.
101+
withCopyToContainer(Transferable.of(script, 0755), "/pulsar/bin/pulsar");
102+
withCommand();
103+
}
104+
waitingFor(Wait.forSuccessfulCommand("test -f /tmp/leak-probe-ready")
105+
.withStartupTimeout(Duration.ofSeconds(60)));
106+
}
107+
108+
@Override
109+
protected void passNettyLeakDetectionSystemProperties() {
110+
super.passNettyLeakDetectionSystemProperties();
111+
// Keep the deliberate leak alive until shutdown even when local tests fail on leaks.
112+
appendToEnv("PULSAR_EXTRA_OPTS",
113+
"-D" + ExtendedNettyLeakDetector.EXIT_JVM_ON_LEAK_SYSTEM_PROPERTY_NAME + "=false");
114+
}
115+
}
116+
117+
public static class LeakProbe {
118+
public static void main(String[] args) throws Exception {
119+
ExtendedNettyLeakDetector.setInitialHint("container-shutdown-leak");
120+
leakBuffer();
121+
Files.writeString(Path.of("/tmp/leak-probe-ready"), "ready");
122+
// Only the detector's shutdown hook will force collection and report the leaked buffer.
123+
new CountDownLatch(1).await();
124+
}
125+
126+
private static void leakBuffer() {
127+
ByteBufAllocator.DEFAULT.directBuffer(16).writeLong(42);
128+
}
129+
}
130+
}

tests/integration/src/test/java/org/apache/pulsar/tests/integration/containers/PulsarContainer.java

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,6 @@ public static void configureLeaveContainerRunning(
172172
protected void beforeStop() {
173173
super.beforeStop();
174174
if (null != getContainerId()) {
175-
DockerUtils.dumpContainerDirToTargetCompressed(
176-
getDockerClient(),
177-
getContainerId(),
178-
"/var/log/pulsar"
179-
);
180175
try {
181176
// stop the "tail -f ..." commands started in afterStart method
182177
// so that shutdown output doesn't clutter logs
@@ -199,26 +194,38 @@ public void stop() {
199194

200195
@Override
201196
protected void doStop() {
202-
if (getContainerId() != null) {
203-
if (serviceEntryPoint.equals("bin/pulsar")) {
204-
// attempt graceful shutdown using "docker stop"
205-
dockerClient.stopContainerCmd(getContainerId())
206-
.withTimeout(15)
207-
.exec();
208-
} else {
209-
// use "supervisorctl stop all" for graceful shutdown
210-
try {
211-
ContainerExecResult result = execCmd("/usr/bin/supervisorctl", "stop", "all");
212-
log.info().attr("exitCode", result.getExitCode())
213-
.attr("stdout", result.getStdout())
214-
.attr("stderr", result.getStderr())
215-
.log("Stopped supervisor services");
216-
} catch (Exception e) {
217-
log.error().exception(e).log("Cannot run 'supervisorctl stop all'");
197+
try {
198+
if (getContainerId() != null) {
199+
if (serviceEntryPoint.equals("bin/pulsar")) {
200+
// attempt graceful shutdown using "docker stop"
201+
dockerClient.stopContainerCmd(getContainerId())
202+
.withTimeout(15)
203+
.exec();
204+
} else {
205+
// use "supervisorctl stop all" for graceful shutdown
206+
try {
207+
ContainerExecResult result = execCmd("/usr/bin/supervisorctl", "stop", "all");
208+
log.info().attr("exitCode", result.getExitCode())
209+
.attr("stdout", result.getStdout())
210+
.attr("stderr", result.getStderr())
211+
.log("Stopped supervisor services");
212+
} catch (Exception e) {
213+
log.error().exception(e).log("Cannot run 'supervisorctl stop all'");
214+
}
215+
}
216+
}
217+
} finally {
218+
try {
219+
if (getContainerId() != null) {
220+
// The leak detector's JVM shutdown hook can produce additional reports. Copy them
221+
// after stopping the services, while the container still exists.
222+
DockerUtils.dumpContainerDirToTargetCompressed(
223+
getDockerClient(), getContainerId(), "/var/log/pulsar");
218224
}
225+
} finally {
226+
super.doStop();
219227
}
220228
}
221-
super.doStop();
222229
}
223230

224231
@Override

tests/integration/src/test/resources/pulsar-standalone.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
<suite name="Pulsar Standalone Tests" verbose="2" annotations="JDK">
2323
<test name="pulsar-standalone-suite" preserve-order="true" >
2424
<classes>
25+
<class name="org.apache.pulsar.tests.integration.containers.NettyLeakDetectionTest" />
2526
<class name="org.apache.pulsar.tests.integration.standalone.SmokeTest" />
2627
</classes>
2728
</test>

0 commit comments

Comments
 (0)