HBASE-30073 Test fixes for some flappers and a reproducible error

[haridsv]

Jira: HBASE-30073

• Fixed TestZstdDictionarySplitMerge by adding a missing test dependency
• Fixes for TestBlockBytesScannedQuota, TestHRegionWithInMemoryFlush, TestZooKeeper, TestStochasticLoadBalancerRegionReplicaSameHosts and TestRollbackSCP was provided by AI and verified by running 20 times each in a loop.

Here are the AI summaries:

---

TestStochasticLoadBalancerRegionReplicaSameHosts

Root cause

StochasticLoadBalancer.balanceTable() set cluster.setStopRequestedAt(startTime + maxRunningTime) at the very beginning of the method, before build ing costs (initCosts), the first computeCost, needsBalance, and other setup.

So the entire maxRunningTime budget (e.g. 250 ms in StochasticBalancerTestBase) was counting initialization + search, not just the stochastic walk.

On a slow or busy CI host, setup alone could take ~250 ms or more. Then when the walk started, isStopRequested() was already true (or became true af ter a single rejected move). The loop could exit with step still 0, no accepted moves, and balanceCluster returning null even though region repl icas were still colocated on the same host—so TestStochasticLoadBalancerRegionReplicaSameHosts failed. Locally, setup is usually only tens of ms, so the bug rarely shows up.

Fix

Set setStopRequestedAt only after initialization is done, immediately before the stochastic for loop, using a new timestamp (searchStartTime + maxRunningTime). That way maxRunningTime applies to the search phase only, which matches the intended meaning of the limit and avoids “no search time left” on loaded agents.

TestRollbackSCP

Root cause

Surefire reruns failed tests in the same JVM without running @BeforeClass again. The test used a static INJECTED flag with compareAndSet(false, t rue) so fault injection (and setKillAndToggleBeforeStoreUpdateInRollback) only ran once per JVM.

After the first failure (e.g. IllegalArgumentException: scheduler queue not empty at restartMasterProcedureExecutor), INJECTED stayed true. On reruns, compareAndSet did nothing, so the executor was never killed in rollback and procExec.isRunning() stayed true, which made waitFor(30000, () -> !procExec.isRunning()) always time out.

So the “three different failures” were really: first attempt hit the real error at line 180; attempts 2–3 were misleading timeouts at line 177 caused by broken rerun state, not the original bug.

Fix

In @Before (which does run before each attempt, including reruns), reset test-only state:

1. INJECTED.set(false) so injection can arm again on each run.
2. ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdateInRollback(procExec, false) so procedure-executor kill flags from the previous attempt are cleared.

That makes each Surefire attempt behave like a fresh test with respect to
injection, so reruns can pass when the underlying issue is fixed or flaky.

TestZooKeeper

Problem Identified:

• Line 78 had a duplicate call to TEST_UTIL.startMiniDFSCluster(2)
• This violated the "can only create one" constraint and caused HDFS namespace conflicts when tests ran in parallel

Why it was flaky:

• ✅ Local/isolated runs: Worked because no leftover HDFS state
• ❌ CI/parallel runs: Failed when other ZooKeeper tests ran first, leaving behind HDFS data with incompatible namespaceID

Fix Applied:
Removed the duplicate call on line 78. The test now properly:

1. Starts Mini DFS Cluster (line 71)
2. Starts Mini ZK Cluster (line 72)
3. Configures settings (lines 73-77)
4. Done! (no duplicate cluster start)

This matches the pattern used in other similar tests like TestClusterBootOrder and aligns with the comment's intent: "Test we can first start the ZK cluster by itself."

The test should now be stable across both local and CI environments. You can verify by running the test suite again to ensure TestZooKeeper no longer fails with InconsistentFSStateException.

TestHRegionWithInMemoryFlush

Root Cause Analysis

The TestHRegionWithInMemoryFlush test is flapping due to zombie background threads that continue running after tests timeout, causing directory race conditions with subsequent tests.

What's Happening

1. testParallelIncrementWithMemStoreFlush and testParallelAppendWithMemStoreFlush create background flush threads
2. These threads run in a busy loop: while (!incrementDone.get())
3. When the test times out (after ~4 minutes in CI), an InterruptedException is thrown during thread.join()
4. The code never reaches the lines that set the done flag and stop the flush thread
5. The zombie flush thread keeps running, busy-looping trying to flush
6. Subsequent tests' tearDown() methods clean up directories
7. The zombie thread tries to access deleted directories → race condition errors

Evidence

• Bad run shows only 5 tests completed (vs 136 in good run)
• Thread-4940 logs "Skipping flush because closing" repeatedly for over a minute
• 6 flaky errors about missing directories in unrelated tests like testGet_Basic and testScanner_JoinedScanners
• Error messages: "Parent directory doesn't exist", "chmod: cannot access"

The Fix

I've fixed both problematic tests in /Users/hdara/src/apache/hbase/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java:

Changes made:

1. Added interrupt check to flush thread loop: while (!done.get() && !Thread.currentThread().isInterrupted())
2. Wrapped test logic in try-finally to ensure cleanup even on timeout/failure
3. In finally block:

• Set done flag to true
• Interrupt the flush thread
• Wait up to 5 seconds for thread to stop
• Log warning if thread doesn't stop

Files modified:

• TestHRegion.java:5322-5368 - testParallelIncrementWithMemStoreFlush
• TestHRegion.java:5407-5459 - testParallelAppendWithMemStoreFlush

Why This Only Happened in CI

CI environments are typically slower and more heavily loaded, causing tests to hit the 13-minute timeout threshold. Locally, the tests complete faster, so the threads finish normally before cleanup.

The fix ensures that background threads are always properly stopped, preventing zombie threads from interfering with subsequent tests.

TestBlockBytesScannedQuota

The test is flapping due to a timing/race condition in the quota system:

1. 5-second timeout too short: The testTraffic method only waited 5 seconds for quotas to take effect
2. Quota cache not fully propagated: On slower systems (like CI), the quota cache refresh can be asynchronous and may not fully propagate in time
3. Quotas bypassed: When cache isn't refreshed, the logs show "bypass expected false, actual true", meaning all requests succeed instead of being throttled
4. Insufficient retries: Each iteration takes ~1.3 seconds, so only 3-4 retries fit in 5 seconds, not enough for the quota system to stabilize

Bad run pattern:

• Test expects 1 successful request but gets 5 (all succeed because quotas not enforced)
• Retries every ~1.3 seconds for 4 attempts
• Times out after 5 seconds with "Waiting timed out after [5,000] msec"

Good run pattern:

• Quotas enforced immediately
• Tests pass quickly (36.97s total vs 63.14s for failed run)

Increased the timeout in testTraffic() from 5,000ms to 30,000ms (line 263). This gives the quota system sufficient time to:

• Complete cache refresh
• Propagate quota settings across all components
• Handle slower CI environments

This is a conservative fix that maintains the retry logic while allowing adequate time for the distributed quota system to stabilize. The 30-second timeout is still reasonable for a test and should handle the asynchronous nature of quota enforcement.

TestBlockBytesScannedQuota

The test is flapping due to a timing/race condition in the quota system:

1. 5-second timeout too short: The testTraffic method only waited 5 seconds for quotas to take effect
2. Quota cache not fully propagated: On slower systems (like CI), the quota cache refresh can be asynchronous and may not fully propagate in time
3. Quotas bypassed: When cache isn't refreshed, the logs show "bypass expected false, actual true", meaning all requests succeed instead of being throttled
4. Insufficient retries: Each iteration takes ~1.3 seconds, so only 3-4 retries fit in 5 seconds, not enough for the quota system to stabilize

Bad run pattern:

• Test expects 1 successful request but gets 5 (all succeed because quotas not enforced)
• Retries every ~1.3 seconds for 4 attempts
• Times out after 5 seconds with "Waiting timed out after [5,000] msec"

Good run pattern:

• Quotas enforced immediately
• Tests pass quickly (36.97s total vs 63.14s for failed run)

Increased the timeout in testTraffic() from 5,000ms to 30,000ms (line 263). This gives the quota system sufficient time to:

• Complete cache refresh
• Propagate quota settings across all components
• Handle slower CI environments

This is a conservative fix that maintains the retry logic while allowing adequate time for the distributed quota system to stabilize. The 30-second timeout is still reasonable for a test and should handle the asynchronous nature of quota enforcement.