This is an automated email from the ASF dual-hosted git repository.
zhangduo 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 7a07dfe HBASE-25254 Rewrite TestMultiLogThreshold to remove the
LogDelegate in RSRpcServices (#2631)
7a07dfe is described below
commit 7a07dfe0597f5107d8e59b56372e58ad02efff85
Author: Duo Zhang <[email protected]>
AuthorDate: Sun Nov 8 21:47:18 2020 +0800
HBASE-25254 Rewrite TestMultiLogThreshold to remove the LogDelegate in
RSRpcServices (#2631)
Signed-off-by: Guanghao Zhang <[email protected]>
---
.../hadoop/hbase/regionserver/RSRpcServices.java | 38 ++-----
.../hbase/regionserver/TestMultiLogThreshold.java | 121 +++++++++++++--------
2 files changed, 81 insertions(+), 78 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index c952b4c..94279e4 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -1108,34 +1108,9 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
}
}
- // Exposed for testing
- interface LogDelegate {
- void logBatchWarning(String firstRegionName, int sum, int
rowSizeWarnThreshold);
- }
-
- private static LogDelegate DEFAULT_LOG_DELEGATE = new LogDelegate() {
- @Override
- public void logBatchWarning(String firstRegionName, int sum, int
rowSizeWarnThreshold) {
- if (LOG.isWarnEnabled()) {
- LOG.warn("Large batch operation detected (greater than " +
rowSizeWarnThreshold
- + ") (HBASE-18023)." + " Requested Number of Rows: " + sum + "
Client: "
- + RpcServer.getRequestUserName().orElse(null) + "/"
- + RpcServer.getRemoteAddress().orElse(null)
- + " first region in multi=" + firstRegionName);
- }
- }
- };
-
- private final LogDelegate ld;
-
- public RSRpcServices(final HRegionServer rs) throws IOException {
- this(rs, DEFAULT_LOG_DELEGATE);
- }
-
// Directly invoked only for testing
- RSRpcServices(final HRegionServer rs, final LogDelegate ld) throws
IOException {
+ public RSRpcServices(final HRegionServer rs) throws IOException {
final Configuration conf = rs.getConfiguration();
- this.ld = ld;
regionServer = rs;
rowSizeWarnThreshold = conf.getInt(
HConstants.BATCH_ROWS_THRESHOLD_NAME,
HConstants.BATCH_ROWS_THRESHOLD_DEFAULT);
@@ -2600,12 +2575,15 @@ public class RSRpcServices implements
HBaseRPCErrorHandler,
sum += regionAction.getActionCount();
}
if (sum > rowSizeWarnThreshold) {
- ld.logBatchWarning(firstRegionName, sum, rowSizeWarnThreshold);
+ LOG.warn("Large batch operation detected (greater than " +
rowSizeWarnThreshold +
+ ") (HBASE-18023)." + " Requested Number of Rows: " + sum + " Client: "
+
+ RpcServer.getRequestUserName().orElse(null) + "/" +
+ RpcServer.getRemoteAddress().orElse(null) + " first region in multi="
+ firstRegionName);
if (rejectRowsWithSizeOverThreshold) {
throw new ServiceException(
- "Rejecting large batch operation for current batch with
firstRegionName: "
- + firstRegionName + " , Requested Number of Rows: " + sum + " ,
Size Threshold: "
- + rowSizeWarnThreshold);
+ "Rejecting large batch operation for current batch with
firstRegionName: " +
+ firstRegionName + " , Requested Number of Rows: " + sum + " , Size
Threshold: " +
+ rowSizeWarnThreshold);
}
}
}
diff --git
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiLogThreshold.java
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiLogThreshold.java
index c58ee17..e2dcac0 100644
---
a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiLogThreshold.java
+++
b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMultiLogThreshold.java
@@ -17,6 +17,14 @@
*/
package org.apache.hadoop.hbase.regionserver;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.verify;
+
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
@@ -26,16 +34,20 @@ import org.apache.hadoop.hbase.HBaseTestingUtility;
import org.apache.hadoop.hbase.HConstants;
import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.ipc.HBaseRpcController;
-import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.log4j.Appender;
+import org.apache.log4j.Level;
+import org.apache.log4j.LogManager;
+import org.apache.log4j.spi.LoggingEvent;
import org.junit.After;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.apache.hbase.thirdparty.com.google.protobuf.RpcController;
@@ -52,21 +64,23 @@ import
org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
* via "Multi" commands) so classified as MediumTests
*/
@RunWith(Parameterized.class)
-@Category(LargeTests.class)
+@Category(MediumTests.class)
public class TestMultiLogThreshold {
@ClassRule
public static final HBaseClassTestRule CLASS_RULE =
- HBaseClassTestRule.forClass(TestMultiLogThreshold.class);
-
- private static RSRpcServices SERVICES;
+ HBaseClassTestRule.forClass(TestMultiLogThreshold.class);
- private static HBaseTestingUtility TEST_UTIL;
- private static Configuration CONF;
+ private static final TableName NAME = TableName.valueOf("tableName");
private static final byte[] TEST_FAM = Bytes.toBytes("fam");
- private static RSRpcServices.LogDelegate LD;
- private static HRegionServer RS;
- private static int THRESHOLD;
+
+ private HBaseTestingUtility util;
+ private Configuration conf;
+ private int threshold;
+ private HRegionServer rs;
+ private RSRpcServices services;
+
+ private Appender appender;
@Parameterized.Parameter
public static boolean rejectLargeBatchOp;
@@ -78,20 +92,22 @@ public class TestMultiLogThreshold {
@Before
public void setupTest() throws Exception {
- final TableName tableName = TableName.valueOf("tableName");
- TEST_UTIL = HBaseTestingUtility.createLocalHTU();
- CONF = TEST_UTIL.getConfiguration();
- THRESHOLD = CONF.getInt(HConstants.BATCH_ROWS_THRESHOLD_NAME,
- HConstants.BATCH_ROWS_THRESHOLD_DEFAULT);
- CONF.setBoolean("hbase.rpc.rows.size.threshold.reject",
rejectLargeBatchOp);
- TEST_UTIL.startMiniCluster();
- TEST_UTIL.createTable(tableName, TEST_FAM);
- RS = TEST_UTIL.getRSForFirstRegionInTable(tableName);
+ util = new HBaseTestingUtility();
+ conf = util.getConfiguration();
+ threshold =
+ conf.getInt(HConstants.BATCH_ROWS_THRESHOLD_NAME,
HConstants.BATCH_ROWS_THRESHOLD_DEFAULT);
+ conf.setBoolean("hbase.rpc.rows.size.threshold.reject",
rejectLargeBatchOp);
+ util.startMiniCluster();
+ util.createTable(NAME, TEST_FAM);
+ rs = util.getRSForFirstRegionInTable(NAME);
+ appender = mock(Appender.class);
+ LogManager.getLogger(RSRpcServices.class).addAppender(appender);
}
@After
public void tearDown() throws Exception {
- TEST_UTIL.shutdownMiniCluster();
+ LogManager.getLogger(RSRpcServices.class).removeAppender(appender);
+ util.shutdownMiniCluster();
}
private enum ActionType {
@@ -104,18 +120,18 @@ public class TestMultiLogThreshold {
* Actions
*/
private void sendMultiRequest(int rows, ActionType actionType)
- throws ServiceException, IOException {
+ throws ServiceException, IOException {
RpcController rpcc = Mockito.mock(HBaseRpcController.class);
MultiRequest.Builder builder = MultiRequest.newBuilder();
int numRAs = 1;
int numAs = 1;
switch (actionType) {
- case REGION_ACTIONS:
- numRAs = rows;
- break;
- case ACTIONS:
- numAs = rows;
- break;
+ case REGION_ACTIONS:
+ numRAs = rows;
+ break;
+ case ACTIONS:
+ numAs = rows;
+ break;
}
for (int i = 0; i < numRAs; i++) {
RegionAction.Builder rab = RegionAction.newBuilder();
@@ -128,38 +144,47 @@ public class TestMultiLogThreshold {
}
builder.addRegionAction(rab.build());
}
- LD = Mockito.mock(RSRpcServices.LogDelegate.class);
- SERVICES = new RSRpcServices(RS, LD);
- SERVICES.multi(rpcc, builder.build());
+ services = new RSRpcServices(rs);
+ services.multi(rpcc, builder.build());
+ }
+
+ private void assertLogBatchWarnings(boolean expected) {
+ ArgumentCaptor<LoggingEvent> captor =
ArgumentCaptor.forClass(LoggingEvent.class);
+ verify(appender, atLeastOnce()).doAppend(captor.capture());
+ boolean actual = false;
+ for (LoggingEvent event : captor.getAllValues()) {
+ if (event.getLevel() == Level.WARN &&
+ event.getRenderedMessage().contains("Large batch operation detected"))
{
+ actual = true;
+ break;
+ }
+ }
+ reset(appender);
+ assertEquals(expected, actual);
}
@Test
public void testMultiLogThresholdRegionActions() throws ServiceException,
IOException {
try {
- sendMultiRequest(THRESHOLD + 1, ActionType.REGION_ACTIONS);
- Assert.assertFalse(rejectLargeBatchOp);
+ sendMultiRequest(threshold + 1, ActionType.REGION_ACTIONS);
+ assertFalse(rejectLargeBatchOp);
} catch (ServiceException e) {
- Assert.assertTrue(rejectLargeBatchOp);
+ assertTrue(rejectLargeBatchOp);
}
- Mockito.verify(LD, Mockito.times(1))
- .logBatchWarning(Mockito.anyString(), Mockito.anyInt(),
Mockito.anyInt());
+ assertLogBatchWarnings(true);
- sendMultiRequest(THRESHOLD, ActionType.REGION_ACTIONS);
- Mockito.verify(LD, Mockito.never())
- .logBatchWarning(Mockito.anyString(), Mockito.anyInt(),
Mockito.anyInt());
+ sendMultiRequest(threshold, ActionType.REGION_ACTIONS);
+ assertLogBatchWarnings(false);
try {
- sendMultiRequest(THRESHOLD + 1, ActionType.ACTIONS);
- Assert.assertFalse(rejectLargeBatchOp);
+ sendMultiRequest(threshold + 1, ActionType.ACTIONS);
+ assertFalse(rejectLargeBatchOp);
} catch (ServiceException e) {
- Assert.assertTrue(rejectLargeBatchOp);
+ assertTrue(rejectLargeBatchOp);
}
- Mockito.verify(LD, Mockito.times(1))
- .logBatchWarning(Mockito.anyString(), Mockito.anyInt(),
Mockito.anyInt());
+ assertLogBatchWarnings(true);
- sendMultiRequest(THRESHOLD, ActionType.ACTIONS);
- Mockito.verify(LD, Mockito.never())
- .logBatchWarning(Mockito.anyString(), Mockito.anyInt(),
Mockito.anyInt());
+ sendMultiRequest(threshold, ActionType.ACTIONS);
+ assertLogBatchWarnings(false);
}
-
}
