HDDS-16423. Fix intermittent failure in testContainerReconciliationFailureContainerScan

[smengcl]

Generated-by: Claude Code (Opus 4.8)

What changes were proposed in this pull request?

TestContainerReconciliationWithMockDatanodes fails intermittently in testContainerReconciliationFailureContainerScan:

[ERROR] testContainerReconciliationFailureContainerScan  Time elapsed: 0.005 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at ...TestContainerReconciliationWithMockDatanodes$MockDatanode.scanContainer(TestContainerReconciliationWithMockDatanodes.java:561)
	at ...TestContainerReconciliationWithMockDatanodes.testContainerReconciliationFailureContainerScan(TestContainerReconciliationWithMockDatanodes.java:316)

The container replicas and the OnDemandContainerScanner are created once in @BeforeAll and shared by every test method. The MockDatanode.scanContainer helper asserts that OnDemandContainerScanner.scanContainerWithoutGap(...) returns a present Future. That method returns an empty Optional when the container is already registered as in progress in containerRescheduleCheckSet; the entry is only removed by the scan executor task after the scan finishes.

KeyValueHandler.reconcileContainer schedules a fire-and-forget scan via scanContainerWithoutGap in a finally block, so every reconciliation test leaves one scan in flight. Those tests wait via waitForExpectedScanCount(...), which polls the numContainersScanned metric that is incremented inside the scan, before the executor thread runs removeContainerFromScheduledContainers. When the next test's synchronous scanContainer runs before that removal, no scan is scheduled, the Optional is empty, and the assertion fails. The withoutGap helper uses minScanGap = 0, so the scan-gap path cannot cause the empty Optional, and this test introduces no corruption, so the volume-failed path is not exercised either; the only cause is the still-registered prior scan.

The fix makes MockDatanode.scanContainer wait until scanContainerWithoutGap returns a scheduled Future before blocking on it, instead of asserting the first call always schedules one. The single-thread scan executor is guaranteed to drain the prior scan, so this is deterministic by construction and keeps the per-call scan count at exactly one, preserving the existing waitForExpectedScanCount assertions.

What is the link to the Apache JIRA

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

How was this patch tested?

Ran the full test class (which runs the reconciliation tests that leave a scan in flight before the failing test) offline:

mvn -pl :hdds-container-service test \
  -Dtest='TestContainerReconciliationWithMockDatanodes' \
  -DskipShade -DskipRecon -DskipDocs -Dsurefire.failIfNoSpecifiedTests=false

A single green run cannot prove the absence of a race, so the fix is deterministic by construction: the only transient reason scanContainerWithoutGap returns an empty Optional here is a prior scan still registered in containerRescheduleCheckSet, and the single-thread scan executor is guaranteed to drain it, after which the retry schedules exactly one scan.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No unresolved review comments; the test helper robustly handles in-flight scans while preserving existing assertions.

Pull request overview

Fixes an intermittent race in mock datanode reconciliation tests caused by in-flight asynchronous scans.

Changes:

• Retry scan scheduling until a Future is returned.
• Preserve synchronous completion and scan-count assertions.

File summaries

File	Summary
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestContainerReconciliationWithMockDatanodes.java	Makes mock datanode scans resilient to in-progress scanner registration.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.