HDDS-10306. Speed up TestSnapshotBackgroundServices by hevinhsu · Pull Request #9721 · apache/ozone

hevinhsu · 2026-02-06T09:15:33Z

What changes were proposed in this pull request?

Speed up TestSnapshotBackgroundServices by reusing a shared MiniOzoneHACluster instead of creating a new cluster for each test case.

Please describe your PR in detail:

Refactor TestSnapshotBackgroundServices to use a shared MiniOzoneHACluster to reduce repeated cluster initialization cost.
Introduce a small test-only API to apply updated configurations by restarting OM, allowing configuration changes without recreating the entire cluster.

What is the link to the Apache JIRA?

https://issues.apache.org/jira/browse/HDDS-10306

How was this patch tested?

https://github.com/hevinhsu/ozone/actions/runs/21741639312

Additionally, the following test was executed locally to verify the improvement:

mvn clean test -Dtest=TestSnapshotBackgroundServices -Dsurefire.skipAfterFailureCount=1 -pl hadoop-ozone/integration-test -am

before:

[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 240.2 s -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotBackgroundServices

after:

[INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 155.6 s -- in org.apache.hadoop.ozone.om.snapshot.TestSnapshotBackgroundServices

adoroszlai

Thanks @hevinhsu for the patch.

...n-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java

Copilot

Pull request overview

This PR speeds up the TestSnapshotBackgroundServices integration test by reusing a single shared MiniOzoneHACluster across test cases and adding a helper API to restart OMs with updated configuration, avoiding full cluster re-creation.

Changes:

Refactored TestSnapshotBackgroundServices to use a per-class shared HA cluster and per-test recovery/setup.
Added MiniOzoneHAClusterImpl#getInactiveOM() and a helper to restart OMs with a config customizer.
Adjusted OM restart behavior to ensure an inactive OM is re-activated before restart.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
`hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java`	Adds inactive OM access and a restart-with-config helper to support faster test reconfiguration.
`hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java`	Reworks the test lifecycle to reuse the HA cluster and selectively restart/recover OMs between test cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T07:38:29Z

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

+    for (OzoneManager om : toRestart) {
+      if (!om.stop()) {
+        continue;
+      }
+      om.join();
+      om.restart();
+      GenericTestUtils.waitFor(om::isRunning, 1000, 30000);
+    }


restartOzoneManagersWithConfigCustomizer silently skips restarting an OM when om.stop() returns false (even though the OM was in the toRestart list). This can mask a failed stop and leave the cluster partially restarted with a mix of old/new config. Consider failing fast (throw) or at least logging and still attempting a restart / waiting for a consistent state.

Copilot · 2026-02-11T07:38:29Z

...n-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java

+  @BeforeEach
+  public void setupTest() throws IOException, InterruptedException, TimeoutException {
+    recoverCluster();
+    stopFollowerOM(cluster.getOMLeader());


setupTest passes cluster.getOMLeader() into stopFollowerOM(...), but getOMLeader() can legally return null transiently (eg during leader transitions) and would cause the helper to stop an arbitrary OM. Prefer using the result of cluster.waitForLeaderOM() (or assert non-null) to avoid test flakiness.

Suggested change

stopFollowerOM(cluster.getOMLeader());

OzoneManager leader = cluster.waitForLeaderOM();

stopFollowerOM(leader);

...n-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java

Copilot · 2026-02-11T07:38:30Z

...n-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java

    assertNotNull(files);
    int numberOfSstFiles = files.length;

+    assertEquals(cluster.getOMLeader(), newLeaderOM);


assertEquals(cluster.getOMLeader(), newLeaderOM) can be flaky because getOMLeader() explicitly returns null when no leader is ready or when multiple leaders appear ready. If the intent is to verify leadership, use cluster.waitForLeaderOM() (and compare), or assert newLeaderOM.isLeaderReady().

Suggested change

assertEquals(cluster.getOMLeader(), newLeaderOM);

assertEquals(cluster.waitForLeaderOM(), newLeaderOM);

Copilot · 2026-02-11T07:38:30Z

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

+      if (configCustomizer != null) {
+        configCustomizer.accept(configuration);
+      }
+      om.setConfiguration(configuration);


Access of element annotated with VisibleForTesting found in production code.

Copilot · 2026-02-11T07:38:30Z

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

+        configCustomizer.accept(configuration);
+      }
+      om.setConfiguration(configuration);
+      if (om.isRunning()) {


Access of element annotated with VisibleForTesting found in production code.

Copilot · 2026-02-11T07:38:31Z

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

+      }
+      om.join();
+      om.restart();
+      GenericTestUtils.waitFor(om::isRunning, 1000, 30000);


Access of element annotated with VisibleForTesting found in production code.

Suggested change

GenericTestUtils.waitFor(om::isRunning, 1000, 30000);

long startTime = System.currentTimeMillis();

while (!om.isRunning()) {

if (System.currentTimeMillis() - startTime > 30000) {

throw new TimeoutException("Timed out waiting for OzoneManager to start.");

}

Thread.sleep(1000);

}

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java

smengcl

lgtm

feat: use shared cluster to improve test speed

9a64c82

adoroszlai requested a review from jojochuang February 6, 2026 13:12

adoroszlai reviewed Feb 6, 2026

View reviewed changes

...n-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotBackgroundServices.java Show resolved Hide resolved

adoroszlai added the test label Feb 7, 2026

hevinhsu added 3 commits February 10, 2026 09:51

fix: address comments

66c82f4

Merge remote-tracking branch 'origin/master' into HDDS-10306

fe69dfc

chore: revert unrelated changes

2b49fd0

hevinhsu force-pushed the HDDS-10306 branch from 97587c7 to 2b49fd0 Compare February 10, 2026 01:59

adoroszlai requested a review from smengcl February 10, 2026 10:32

Merge remote-tracking branch 'origin/master' into HDDS-10306

faccac1

smengcl requested review from Copilot and removed request for jojochuang and smengcl February 11, 2026 07:30

Copilot started reviewing on behalf of smengcl February 11, 2026 07:30 View session

smengcl self-requested a review February 11, 2026 07:30

Copilot AI reviewed Feb 11, 2026

View reviewed changes

smengcl reviewed Feb 11, 2026

View reviewed changes

hadoop-ozone/mini-cluster/src/main/java/org/apache/hadoop/ozone/MiniOzoneHAClusterImpl.java Outdated Show resolved Hide resolved

hevinhsu added 4 commits February 11, 2026 16:46

fix: move active API call after OM restart to avoid inconsistent state

593b07d

Merge remote-tracking branch 'origin/master' into HDDS-10306

97607e9

fix: remove unused parameter

7176929

Merge remote-tracking branch 'origin/master' into HDDS-10306

49137b5

adoroszlai requested a review from smengcl February 12, 2026 10:48

smengcl approved these changes Feb 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-10306. Speed up TestSnapshotBackgroundServices#9721

HDDS-10306. Speed up TestSnapshotBackgroundServices#9721
hevinhsu wants to merge 9 commits intoapache:masterfrom
hevinhsu:HDDS-10306

hevinhsu commented Feb 6, 2026

Uh oh!

adoroszlai left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Uh oh!

smengcl left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	stopFollowerOM(cluster.getOMLeader());
	OzoneManager leader = cluster.waitForLeaderOM();
	stopFollowerOM(leader);

	assertEquals(cluster.getOMLeader(), newLeaderOM);
	assertEquals(cluster.waitForLeaderOM(), newLeaderOM);

-      GenericTestUtils.waitFor(om::isRunning, 1000, 30000);
+      long startTime = System.currentTimeMillis();
+      while (!om.isRunning()) {
+        if (System.currentTimeMillis() - startTime > 30000) {
+          throw new TimeoutException("Timed out waiting for OzoneManager to start.");
+        }
+        Thread.sleep(1000);
+      }

Conversation

hevinhsu commented Feb 6, 2026

What changes were proposed in this pull request?

Please describe your PR in detail:

What is the link to the Apache JIRA?

How was this patch tested?

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants