Skip to content

HDFS-17957. Shut down leaked mini-cluster instances in hadoop-hdfs-project tests. - #8659

Draft
joseluisll wants to merge 2 commits into
apache:trunkfrom
joseluisll:HDFS-17957-cluster-leak
Draft

HDFS-17957. Shut down leaked mini-cluster instances in hadoop-hdfs-project tests.#8659
joseluisll wants to merge 2 commits into
apache:trunkfrom
joseluisll:HDFS-17957-cluster-leak

Conversation

@joseluisll

@joseluisll joseluisll commented Aug 4, 2026

Copy link
Copy Markdown

Description of PR

Shuts down 67 leaked mini-cluster instances across 42 test files in
hadoop-hdfs-project, and fixes an assertion race in TestFsVolumeList.

A leaked cluster keeps its NameNode and DataNode threads, heap and ports alive
under the remaining tests of the class, so one real failure turns into several
bogus ones. The three categories are cluster never shut down (22), shut down
only on the happy path so it leaks precisely when the test fails (36), and held
in a field with no teardown (9). The fixes are try-with-resources, shutdown()
moved into finally, or an @AfterEach.

Two things a reviewer should look at:

TestFsVolumeList also gets an unrelated fix, without which it stays red. Its
testAddRplicaProcessorForAddingReplicaInMap compares the configured pool size
against BlockPoolSlice#getAddReplicaForkPoolSize(), which returned
ForkJoinPool#getPoolSize() — threads currently started, which the pool grows
lazily — giving its expected: <5> but was: <4> failures. The
@VisibleForTesting getter now returns getParallelism(). This is the only
src/main change in the PR and the method has no production callers.

TestNNWithQJM#testNewNamenodeTakesOverWriter had its shutdown commented out
on purpose: the test leaves its NameNode fenced behind a second cluster holding
the journal quorum, so closing the edit log terminates the JVM with "Could not
sync enough journals to persistent storage". It is now shut down inside a
try/catch that tolerates that, so the rest of the class no longer runs
underneath a live fenced NameNode. This is the only judgement call in the diff.

TestFetchImage is the one survey case deliberately left alone — same fenced
shutdown problem, but with a single @Test there is no later test to protect,
so adding teardown would only break a class that passes today.

The remaining survey candidates are itemised in the JIRA: 4 need no fix by
design, 4 were scan false positives, and 11 are outside hadoop-hdfs-project and
tracked separately.

How was this patch tested?

Every touched test class was run on Ubuntu 24.04 with JDK
17.0.19+10-1-24.04.2-Ubuntu, the same JDK and OS as the precommit agents:

Module Classes Tests Result
hadoop-hdfs 36 367 green
hadoop-hdfs-rbf 4 22 green
hadoop-hdfs-nfs 1 1 green

For code changes:

  • Does the title of this PR start with the corresponding JIRA issue id?
  • Object storage: N/A
  • If adding new dependencies … — no new dependencies
  • If applicable, have you updated the LICENSE… — N/A

AI Tooling

Contains content generated by Claude Code.

joseluisll and others added 2 commits August 4, 2026 08:23
… race in TestFsVolumeList.

getAddReplicaForkPoolSize() now returns getParallelism() instead of the
lazily grown getPoolSize(). It is @VisibleForTesting with no production
callers.

Contains content generated by Claude Code.
Generated-by: Claude Code (Opus 5)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…oject tests.

65 cases in 41 files: hadoop-hdfs (36), hadoop-hdfs-rbf (4),
hadoop-hdfs-nfs (1). Test-only.

Contains content generated by Claude Code.
Generated-by: Claude Code (Opus 5)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@shfshihuafeng shfshihuafeng left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I accidentally approved this PR by mistake.

@joseluisll

Copy link
Copy Markdown
Author

It is almost finished. I left in draft because I would like to give it a second review before putting it as ready and get back to you.🏗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants