This is an automated email from the ASF dual-hosted git repository.
Apache9 pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push:
new 4528f5a35f8 Revert "HBASE-30073 Test fixes for some flappers and a
reproducible error (#8057) (#8082)"
4528f5a35f8 is described below
commit 4528f5a35f8008db5623dd832df6ca258737f630
Author: Duo Zhang <[email protected]>
AuthorDate: Mon Apr 20 15:41:52 2026 +0800
Revert "HBASE-30073 Test fixes for some flappers and a reproducible error
(#8057) (#8082)"
This reverts commit 76358a16c8a5f38a46263feca3422bf82c063350.
---
.../apache/hadoop/hbase/HBaseJupiterExtension.java | 2 +-
hbase-compression/pom.xml | 5 -
.../master/balancer/StochasticLoadBalancer.java | 5 +-
.../apache/hadoop/hbase/HBaseTestingUtility.java | 10 +-
.../org/apache/hadoop/hbase/TestZooKeeper.java | 1 +
.../hbase/client/AbstractTestAsyncTableScan.java | 18 +---
.../hfile/bucket/TestPrefetchWithBucketCache.java | 22 ++--
.../hbase/master/assignment/TestRollbackSCP.java | 5 -
.../hbase/quotas/TestBlockBytesScannedQuota.java | 2 +-
.../hadoop/hbase/regionserver/TestHRegion.java | 119 ++++-----------------
.../hbase/replication/TestReplicationBase.java | 4 -
11 files changed, 39 insertions(+), 154 deletions(-)
diff --git
a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseJupiterExtension.java
b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseJupiterExtension.java
index 057c4642ffa..9d4ea87e0ec 100644
---
a/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseJupiterExtension.java
+++
b/hbase-common/src/test/java/org/apache/hadoop/hbase/HBaseJupiterExtension.java
@@ -84,7 +84,7 @@ public class HBaseJupiterExtension implements
InvocationInterceptor, BeforeAllCa
private static final Map<String, Duration> TAG_TO_TIMEOUT =
ImmutableMap.of(SmallTests.TAG, Duration.ofMinutes(3), MediumTests.TAG,
Duration.ofMinutes(6),
- LargeTests.TAG, Duration.ofMinutes(20), IntegrationTests.TAG,
Duration.ZERO);
+ LargeTests.TAG, Duration.ofMinutes(13), IntegrationTests.TAG,
Duration.ZERO);
private static final String EXECUTOR = "executor";
diff --git a/hbase-compression/pom.xml b/hbase-compression/pom.xml
index f829c174a04..c2e4633b398 100644
--- a/hbase-compression/pom.xml
+++ b/hbase-compression/pom.xml
@@ -45,11 +45,6 @@
<artifactId>hbase-resource-bundle</artifactId>
<optional>true</optional>
</dependency>
- <dependency>
- <groupId>org.junit.jupiter</groupId>
- <artifactId>junit-jupiter-api</artifactId>
- <scope>test</scope>
- </dependency>
</dependencies>
<build>
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
index 7503195e273..821825ce0ce 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java
@@ -628,6 +628,7 @@ public class StochasticLoadBalancer extends
BaseLoadBalancer {
rackManager, regionCacheRatioOnOldServerMap);
long startTime = EnvironmentEdgeManager.currentTime();
+ cluster.setStopRequestedAt(startTime + maxRunningTime);
initCosts(cluster);
balancerConditionals.loadClusterState(cluster);
@@ -678,10 +679,6 @@ public class StochasticLoadBalancer extends
BaseLoadBalancer {
currentCost / sumMultiplier, functionCost(), computedMaxSteps);
final String initFunctionTotalCosts = totalCostsPerFunc();
- long searchStartTime = EnvironmentEdgeManager.currentTime();
- // Budget maxRunningTime for the stochastic walk only; initialization
(cluster costs, etc.)
- // can be substantial on busy hosts and must not consume the search
deadline.
- cluster.setStopRequestedAt(searchStartTime + maxRunningTime);
// Perform a stochastic walk to see if we can get a good fit.
long step;
boolean planImprovedConditionals = false;
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
index 17eb3160452..68456b7d341 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
@@ -489,13 +489,7 @@ public class HBaseTestingUtility extends
HBaseZKTestingUtility {
String sysValue = System.getProperty(propertyName);
- // Check if directory sharing should be disabled for this test.
- // Tests that run with high parallelism and don't need shared directories
can set this
- // to avoid race conditions where one test's tearDown() deletes
directories another test
- // is still using.
- boolean disableSharing =
conf.getBoolean("hbase.test.disable-directory-sharing", false);
-
- if (sysValue != null && !disableSharing) {
+ if (sysValue != null) {
// There is already a value set. So we do nothing but hope
// that there will be no conflicts
LOG.info("System.getProperty(\"" + propertyName + "\") already set to: "
+ sysValue
@@ -508,7 +502,7 @@ public class HBaseTestingUtility extends
HBaseZKTestingUtility {
}
conf.set(propertyName, sysValue);
} else {
- // Ok, it's not set (or sharing is disabled), so we create it as a
subdirectory
+ // Ok, it's not set, so we create it as a subdirectory
createSubDir(propertyName, parent, subDirName);
System.setProperty(propertyName, conf.get(propertyName));
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
index 39eca489515..0b0454e42eb 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
@@ -77,6 +77,7 @@ public class TestZooKeeper {
conf.setInt(HConstants.ZK_SESSION_TIMEOUT, 1000);
conf.setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS,
MockLoadBalancer.class,
LoadBalancer.class);
+ TEST_UTIL.startMiniDFSCluster(2);
}
@AfterAll
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestAsyncTableScan.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestAsyncTableScan.java
index 158db897587..6f36be61c65 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestAsyncTableScan.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/AbstractTestAsyncTableScan.java
@@ -46,7 +46,6 @@ import java.util.stream.IntStream;
import java.util.stream.Stream;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.ConnectionRule;
-import org.apache.hadoop.hbase.HBaseConfiguration;
import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.MatcherPredicate;
import org.apache.hadoop.hbase.MiniClusterRule;
@@ -74,21 +73,8 @@ import org.junit.rules.TestRule;
public abstract class AbstractTestAsyncTableScan {
protected static final OpenTelemetryClassRule OTEL_CLASS_RULE =
OpenTelemetryClassRule.create();
-
- private static Configuration createConfiguration() {
- // Use HBaseConfiguration.create() instead of new Configuration() to
properly load
- // hbase-default.xml which contains required default values (e.g. for log
cleaner plugins)
- Configuration conf = HBaseConfiguration.create();
- // Disable directory sharing to prevent race conditions when tests run in
parallel.
- // Each test instance gets its own isolated directories to avoid one
test's tearDown()
- // deleting directories another parallel test is still using.
- conf.setBoolean("hbase.test.disable-directory-sharing", true);
- return conf;
- }
-
- protected static final MiniClusterRule MINI_CLUSTER_RULE =
- MiniClusterRule.newBuilder().setConfiguration(createConfiguration())
-
.setMiniClusterOption(StartMiniClusterOption.builder().numWorkers(3).build()).build();
+ protected static final MiniClusterRule MINI_CLUSTER_RULE =
MiniClusterRule.newBuilder()
+
.setMiniClusterOption(StartMiniClusterOption.builder().numWorkers(3).build()).build();
protected static final ConnectionRule CONN_RULE =
ConnectionRule.createAsyncConnectionRule(MINI_CLUSTER_RULE::createAsyncConnection);
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestPrefetchWithBucketCache.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestPrefetchWithBucketCache.java
index 385d498e809..6ff9f755154 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestPrefetchWithBucketCache.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestPrefetchWithBucketCache.java
@@ -22,6 +22,7 @@ import static
org.apache.hadoop.hbase.HConstants.BUCKET_CACHE_SIZE_KEY;
import static
org.apache.hadoop.hbase.io.hfile.BlockCacheFactory.BUCKET_CACHE_BUCKETS_KEY;
import static
org.apache.hadoop.hbase.io.hfile.bucket.BucketCache.QUEUE_ADDITION_WAIT_TIME;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -327,8 +328,7 @@ public class TestPrefetchWithBucketCache {
conf.setLong(QUEUE_ADDITION_WAIT_TIME, 0);
blockCache = BlockCacheFactory.createBlockCache(conf);
cacheConf = new CacheConfig(conf, blockCache);
- // Use 15000 KVs to ensure file reliably exceeds 1MB cache capacity even
with size variance
- Path storeFile = writeStoreFile("testPrefetchRunTriggersEvictions", 15000);
+ Path storeFile = writeStoreFile("testPrefetchInterruptOnCapacity", 10000);
// Prefetches the file blocks
createReaderAndWaitForPrefetchInterruption(storeFile);
Waiter.waitFor(conf, (PrefetchExecutor.getPrefetchDelay() + 1000),
@@ -343,16 +343,14 @@ public class TestPrefetchWithBucketCache {
}
return true;
});
- // With no wait time configuration, prefetch will either trigger evictions
when reaching
- // cache capacity, or have failed inserts when the writer queue fills
faster than it drains.
- // Both outcomes are valid - test should only fail if NEITHER happens,
which would indicate
- // a problem with the capacity management logic.
- long evictions = bc.getStats().getEvictedCount();
- long failedInserts = bc.getStats().getFailedInserts();
- assertTrue(
- "Expected either evictions or failed inserts to demonstrate capacity
management, "
- + "but got evictions=" + evictions + ", failedInserts=" +
failedInserts,
- evictions > 0 || failedInserts > 0);
+ if (bc.getStats().getFailedInserts() == 0) {
+ // With no wait time configuration, prefetch should trigger evictions
once it reaches
+ // cache capacity
+ assertNotEquals(0, bc.getStats().getEvictedCount());
+ } else {
+ LOG.info("We had {} cache insert failures, which may cause cache usage "
+ + "to never reach capacity.", bc.getStats().getFailedInserts());
+ }
}
@Test
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRollbackSCP.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRollbackSCP.java
index 364057c06b1..25f2e582068 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRollbackSCP.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRollbackSCP.java
@@ -136,11 +136,6 @@ public class TestRollbackSCP {
@Before
public void setUp() throws IOException {
UTIL.ensureSomeNonStoppedRegionServersAvailable(2);
- // Surefire reruns failed tests in the same JVM without re-running
@BeforeClass. Reset injection
- // state so compareAndSet in persistToMeta can succeed again and
kill-before-store flags clear.
- INJECTED.set(false);
- ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdateInRollback(
- UTIL.getMiniHBaseCluster().getMaster().getMasterProcedureExecutor(),
false);
}
private ServerCrashProcedure getSCPForServer(ServerName serverName) throws
IOException {
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestBlockBytesScannedQuota.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestBlockBytesScannedQuota.java
index a4c522447ec..543f138da73 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestBlockBytesScannedQuota.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/quotas/TestBlockBytesScannedQuota.java
@@ -260,7 +260,7 @@ public class TestBlockBytesScannedQuota {
private void testTraffic(Callable<Long> trafficCallable, long
expectedSuccess, long marginOfError)
throws Exception {
- TEST_UTIL.waitFor(30_000, () -> {
+ TEST_UTIL.waitFor(5_000, () -> {
long actualSuccess;
try {
actualSuccess = trafficCallable.call();
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
index 640d34d1df8..9032792ee36 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
@@ -263,10 +263,6 @@ public class TestHRegion {
TEST_UTIL = HBaseTestingUtility.createLocalHTU();
FILESYSTEM = TEST_UTIL.getTestFileSystem();
CONF = TEST_UTIL.getConfiguration();
- // Disable directory sharing to prevent race conditions when tests run in
parallel.
- // Each test instance gets its own isolated directories to avoid one
test's tearDown()
- // deleting directories another parallel test is still using.
- CONF.setBoolean("hbase.test.disable-directory-sharing", true);
NettyAsyncFSWALConfigHelper.setEventLoopConfig(CONF, GROUP,
NioSocketChannel.class);
dir = TEST_UTIL.getDataTestDir("TestHRegion").toString();
method = name.getMethodName();
@@ -1353,23 +1349,18 @@ public class TestHRegion {
threads[i].start();
}
} finally {
- done.set(true);
- for (GetTillDoneOrException t : threads) {
- if (t != null) {
- try {
- t.join(5000);
- } catch (InterruptedException e) {
- e.printStackTrace();
- }
- }
- }
if (this.region != null) {
HBaseTestingUtility.closeRegionAndWAL(this.region);
this.region = null;
}
}
- // Check for errors after threads have been stopped
+ done.set(true);
for (GetTillDoneOrException t : threads) {
+ try {
+ t.join();
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
if (t.e != null) {
LOG.info("Exception=" + t.e);
assertFalse("Found a NPE in " + t.getName(), t.e instanceof
NullPointerException);
@@ -1397,7 +1388,7 @@ public class TestHRegion {
@Override
public void run() {
- while (!this.done.get() && !Thread.currentThread().isInterrupted()) {
+ while (!this.done.get()) {
try {
assertTrue(region.get(g).size() > 0);
this.count.incrementAndGet();
@@ -4577,7 +4568,7 @@ public class TestHRegion {
@Override
public void run() {
done = false;
- while (!done && !Thread.currentThread().isInterrupted()) {
+ while (!done) {
synchronized (this) {
try {
wait();
@@ -4790,7 +4781,7 @@ public class TestHRegion {
@Override
public void run() {
done = false;
- while (!done && !Thread.currentThread().isInterrupted()) {
+ while (!done) {
try {
for (int r = 0; r < numRows; r++) {
byte[] row = Bytes.toBytes("row" + r);
@@ -5324,7 +5315,7 @@ public class TestHRegion {
Runnable flusher = new Runnable() {
@Override
public void run() {
- while (!incrementDone.get() &&
!Thread.currentThread().isInterrupted()) {
+ while (!incrementDone.get()) {
try {
region.flush(true);
} catch (Exception e) {
@@ -5340,38 +5331,13 @@ public class TestHRegion {
long expected = (long) threadNum * incCounter;
Thread[] incrementers = new Thread[threadNum];
Thread flushThread = new Thread(flusher);
- flushThread.setName("FlushThread-" + method);
for (int i = 0; i < threadNum; i++) {
incrementers[i] = new Thread(new Incrementer(this.region, incCounter));
incrementers[i].start();
}
flushThread.start();
- try {
- for (int i = 0; i < threadNum; i++) {
- incrementers[i].join();
- }
-
- incrementDone.set(true);
- flushThread.join();
-
- Get get = new Get(Incrementer.incRow);
- get.addColumn(Incrementer.family, Incrementer.qualifier);
- get.readVersions(1);
- Result res = this.region.get(get);
- List<Cell> kvs = res.getColumnCells(Incrementer.family,
Incrementer.qualifier);
-
- // we just got the latest version
- assertEquals(1, kvs.size());
- Cell kv = kvs.get(0);
- assertEquals(expected, Bytes.toLong(kv.getValueArray(),
kv.getValueOffset()));
- } finally {
- // Ensure flush thread is stopped even if test fails or times out
- incrementDone.set(true);
- flushThread.interrupt();
- flushThread.join(5000); // Wait up to 5 seconds for thread to stop
- if (flushThread.isAlive()) {
- LOG.warn("Flush thread did not stop within timeout for test " +
method);
- }
+ for (int i = 0; i < threadNum; i++) {
+ incrementers[i].join();
}
incrementDone.set(true);
@@ -5434,7 +5400,7 @@ public class TestHRegion {
Runnable flusher = new Runnable() {
@Override
public void run() {
- while (!appendDone.get() && !Thread.currentThread().isInterrupted()) {
+ while (!appendDone.get()) {
try {
region.flush(true);
} catch (Exception e) {
@@ -5454,41 +5420,13 @@ public class TestHRegion {
}
Thread[] appenders = new Thread[threadNum];
Thread flushThread = new Thread(flusher);
- flushThread.setName("FlushThread-" + method);
for (int i = 0; i < threadNum; i++) {
appenders[i] = new Thread(new Appender(this.region, appendCounter));
appenders[i].start();
}
flushThread.start();
- try {
- for (int i = 0; i < threadNum; i++) {
- appenders[i].join();
- }
-
- appendDone.set(true);
- flushThread.join();
-
- Get get = new Get(Appender.appendRow);
- get.addColumn(Appender.family, Appender.qualifier);
- get.readVersions(1);
- Result res = this.region.get(get);
- List<Cell> kvs = res.getColumnCells(Appender.family, Appender.qualifier);
-
- // we just got the latest version
- assertEquals(1, kvs.size());
- Cell kv = kvs.get(0);
- byte[] appendResult = new byte[kv.getValueLength()];
- System.arraycopy(kv.getValueArray(), kv.getValueOffset(), appendResult,
0,
- kv.getValueLength());
- assertArrayEquals(expected, appendResult);
- } finally {
- // Ensure flush thread is stopped even if test fails or times out
- appendDone.set(true);
- flushThread.interrupt();
- flushThread.join(5000); // Wait up to 5 seconds for thread to stop
- if (flushThread.isAlive()) {
- LOG.warn("Flush thread did not stop within timeout for test " +
method);
- }
+ for (int i = 0; i < threadNum; i++) {
+ appenders[i].join();
}
appendDone.set(true);
@@ -7525,7 +7463,7 @@ public class TestHRegion {
// Writer thread
Thread writerThread = new Thread(() -> {
try {
- while (!Thread.currentThread().isInterrupted()) {
+ while (true) {
// If all the reader threads finish, then stop the writer thread
if (latch.await(0, TimeUnit.MILLISECONDS)) {
return;
@@ -7550,19 +7488,15 @@ public class TestHRegion {
.addColumn(fam1, q3, tsIncrement + 1, Bytes.toBytes(1L))
.addColumn(fam1, q4, tsAppend + 1, Bytes.toBytes("a")) });
}
- } catch (InterruptedException e) {
- // Test interrupted, exit gracefully
- Thread.currentThread().interrupt();
} catch (Exception e) {
assertionError.set(new AssertionError(e));
}
});
- writerThread.setName("WriterThread-" + method);
writerThread.start();
// Reader threads
for (int i = 0; i < numReaderThreads; i++) {
- Thread readerThread = new Thread(() -> {
+ new Thread(() -> {
try {
for (int j = 0; j < 10000; j++) {
// Verify the values
@@ -7591,24 +7525,13 @@ public class TestHRegion {
}
latch.countDown();
- });
- readerThread.setName("ReaderThread-" + i + "-" + method);
- readerThread.start();
+ }).start();
}
- try {
- writerThread.join();
+ writerThread.join();
- if (assertionError.get() != null) {
- throw assertionError.get();
- }
- } finally {
- // Ensure writer thread is stopped on test timeout
- writerThread.interrupt();
- writerThread.join(5000);
- if (writerThread.isAlive()) {
- LOG.warn("Writer thread did not stop within timeout for test " +
method);
- }
+ if (assertionError.get() != null) {
+ throw assertionError.get();
}
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationBase.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationBase.java
index 9f74acb58b8..e87e13d151b 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationBase.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationBase.java
@@ -183,10 +183,6 @@ public class TestReplicationBase {
protected static void setupConfig(HBaseTestingUtility util, String
znodeParent) {
Configuration conf = util.getConfiguration();
conf.set(HConstants.ZOOKEEPER_ZNODE_PARENT, znodeParent);
- // Disable directory sharing to prevent race conditions when tests run in
parallel.
- // Each test instance gets its own isolated directories to avoid one
test's tearDown()
- // deleting directories another parallel test is still using.
- conf.setBoolean("hbase.test.disable-directory-sharing", true);
// We don't want too many edits per batch sent to the ReplicationEndpoint
to trigger
// sufficient number of events. But we don't want to go too low because
// HBaseInterClusterReplicationEndpoint partitions entries into batches
and we want
