Apache9 commented on code in PR #5507:
URL: https://github.com/apache/hbase/pull/5507#discussion_r1390536061
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestLogRolling.java:
##########
@@ -158,6 +169,138 @@ private void startAndWriteData() throws IOException,
InterruptedException {
}
}
+ public static void setSyncLatencyMillis(int latency) {
+ syncLatencyMillis = latency;
+ }
+
+ public static int getSyncLatencyMillis() {
+ return syncLatencyMillis;
+ }
+
+ @Test
+ public void testSlowSyncLogRolling() throws Exception {
+ // Create the test table
+ TableDescriptor desc =
TableDescriptorBuilder.newBuilder(TableName.valueOf(getName()))
+
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(HConstants.CATALOG_FAMILY)).build();
+ admin.createTable(desc);
+ Table table = TEST_UTIL.getConnection().getTable(desc.getTableName());
+ int row = 1;
+ try {
+ server = TEST_UTIL.getRSForFirstRegionInTable(desc.getTableName());
+ RegionInfo region =
server.getRegions(desc.getTableName()).get(0).getRegionInfo();
+ // Get a reference to the wal.
+ final AbstractFSWAL log = (AbstractFSWAL) server.getWAL(region);
+
+ final AtomicBoolean slowSyncHookCalled = new AtomicBoolean();
+ // Register a WALActionsListener to observe if a SLOW_SYNC roll is
requested
+ log.registerWALActionsListener(new WALActionsListener() {
+ @Override
+ public void logRollRequested(WALActionsListener.RollRequestReason
reason) {
+ switch (reason) {
+ case SLOW_SYNC:
+ slowSyncHookCalled.lazySet(true);
+ break;
+ default:
+ break;
+ }
+ }
+ });
+
+ // Write some data
+ for (int i = 0; i < 10; i++) {
+ writeData(table, row++);
+ }
+
+ assertFalse("Should not have triggered log roll due to SLOW_SYNC",
slowSyncHookCalled.get());
+
+ // Only test for FSHLog.
+ if
("filesystem".equals(TEST_UTIL.getConfiguration().get(WALFactory.WAL_PROVIDER)))
{
+
+ // Adds 200 ms of latency to any sync on the hlog. This should be more
than sufficient to
+ // trigger slow sync warnings.
+ setSyncLatencyMillis(200);
+ setSlowLogWriter(log.conf);
+ log.rollWriter(true);
+
+ // Set up for test
+ slowSyncHookCalled.set(false);
+
+ final WALProvider.WriterBase oldWriter1 = log.getWriter();
+
+ // Write some data.
+ // We need to write at least 5 times, but double it. We should only
request
+ // a SLOW_SYNC roll once in the current interval.
+ for (int i = 0; i < 10; i++) {
+ writeData(table, row++);
+ }
+
+ // Wait for our wait injecting writer to get rolled out, as needed.
+ TEST_UTIL.waitFor(10000, 100, new
Waiter.ExplainingPredicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return log.getWriter() != oldWriter1;
+ }
+
+ @Override
+ public String explainFailure() throws Exception {
+ return "Waited too long for our test writer to get rolled out";
+ }
+ });
+
+ assertTrue("Should have triggered log roll due to SLOW_SYNC",
slowSyncHookCalled.get());
+ }
+
+ // Adds 5000 ms of latency to any sync on the hlog. This will trip the
other threshold.
+ setSyncLatencyMillis(5000);
+ setSlowLogWriter(log.conf);
+ log.rollWriter(true);
+
+ // Set up for test
+ slowSyncHookCalled.set(false);
+
+ final WALProvider.WriterBase oldWriter2 = log.getWriter();
+
+ // Write some data. Should only take one sync.
+ writeData(table, row++);
+
+ // Wait for our wait injecting writer to get rolled out, as needed.
+ TEST_UTIL.waitFor(10000, 100, new
Waiter.ExplainingPredicate<Exception>() {
+ @Override
+ public boolean evaluate() throws Exception {
+ return log.getWriter() != oldWriter2;
+ }
+
+ @Override
+ public String explainFailure() throws Exception {
+ return "Waited too long for our test writer to get rolled out";
+ }
+ });
+
+ assertTrue("Should have triggered log roll due to SLOW_SYNC",
slowSyncHookCalled.get());
+
+ // Set default log writer, no additional latency to any sync on the hlog.
+ setDefaultLogWriter(log.conf);
+ log.rollWriter(true);
+
+ // Set up for test
+ slowSyncHookCalled.set(false);
+
+ // Write some data
+ for (int i = 0; i < 10; i++) {
+ writeData(table, row++);
+ }
+
+ assertFalse("Should not have triggered log roll due to SLOW_SYNC",
slowSyncHookCalled.get());
+
+ } finally {
+ table.close();
Review Comment:
Just use try-with-resource?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestLogRolling.java:
##########
@@ -74,6 +78,7 @@ public abstract class AbstractTestLogRolling {
protected static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
@Rule
public final TestName name = new TestName();
+ private static int syncLatencyMillis;
Review Comment:
Just making it `protected static` is enough?
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/AbstractTestLogRolling.java:
##########
@@ -158,6 +169,138 @@ private void startAndWriteData() throws IOException,
InterruptedException {
}
}
+ public static void setSyncLatencyMillis(int latency) {
Review Comment:
Why these methods need to be public?
--
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: issues-unsubscr...@hbase.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
