Copilot commented on code in PR #8283:
URL: https://github.com/apache/hbase/pull/8283#discussion_r3322182656
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingMemStore.java:
##########
@@ -56,49 +55,35 @@
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
import org.apache.hadoop.hbase.util.Threads;
import org.apache.hadoop.hbase.wal.WAL;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.ClassRule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
* compacted memstore test case
*/
-@Category({ RegionServerTests.class, MediumTests.class })
+@Tag(RegionServerTests.TAG)
+@Tag(MediumTests.TAG)
public class TestCompactingMemStore extends TestDefaultMemStore {
- @ClassRule
- public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestCompactingMemStore.class);
-
private static final Logger LOG =
LoggerFactory.getLogger(TestCompactingMemStore.class);
protected static ChunkCreator chunkCreator;
protected HRegion region;
protected RegionServicesForStores regionServicesForStores;
protected HStore store;
+ private Configuration conf;
- @After
- public void tearDown() throws Exception {
+ @Override
+ protected void internalTearDown() throws Exception {
chunkCreator.clearChunksInPool();
- super.tearDown();
}
Review Comment:
internalTearDown only clears the ChunkCreator pool but never closes the
HRegion/WAL created in internalSetUp. This can leak filesystem handles and
background resources across tests. Close the region (which also closes its WAL)
during teardown.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompaction.java:
##########
@@ -744,7 +738,7 @@ public List<Path> compact(ThroughputController
throughputController, User user)
this.wait();
}
} catch (InterruptedException e) {
- Assume.assumeNoException(e);
+ Assumptions.abort(e.getMessage());
}
Review Comment:
The InterruptedException handler aborts the test but does not restore the
thread's interrupted status. This can mask interrupts and cause later code in
the same thread to behave incorrectly (or fail to stop promptly).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactingMemStore.java:
##########
@@ -113,7 +98,10 @@ protected void compactingSetUp() throws Exception {
ThreadPoolExecutor pool = (ThreadPoolExecutor)
Executors.newFixedThreadPool(1);
Mockito.when(regionServicesForStores.getInMemoryCompactionPool()).thenReturn(pool);
Review Comment:
This test overrides RegionServicesForStores' in-memory compaction pool with
a newFixedThreadPool whose threads are non-daemon by default. If any task is
submitted, this can leave non-daemon threads running after the test finishes
and potentially hang the test JVM. Use a daemon thread factory here (or avoid
overriding the pool).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
