You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2021/08/07 15:24:16 UTC

[hbase] branch branch-2 updated (4f0f519 -> 611ffa7)

This is an automated email from the ASF dual-hosted git repository.

zhangduo pushed a change to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git.


    from 4f0f519  HBASE-26142 NullPointerException when set 'hbase.hregion.memstore.mslab.indexchunksize.percent' to zero (#3531)
     new 61fbe6f  HBASE-26170 handleTooBigRequest in NettyRpcServer didn't skip enough bytes (#3564)
     new 611ffa7  HBASE-26179 TestRequestTooBigException spends too much time to finish (#3571)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../hadoop/hbase/ipc/NettyRpcFrameDecoder.java     | 12 +++--
 .../org/apache/hadoop/hbase/ipc/RpcServer.java     |  7 +--
 .../hbase/ipc/SimpleServerRpcConnection.java       | 12 +++--
 .../hbase/client/TestRequestTooBigException.java   | 55 ++++++++++------------
 4 files changed, 42 insertions(+), 44 deletions(-)

[hbase] 02/02: HBASE-26179 TestRequestTooBigException spends too much time to finish (#3571)

Posted by zh...@apache.org.
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

commit 611ffa7da990caf8b9ef7db97609242a6dbc4c7c
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Sat Aug 7 23:01:55 2021 +0800

    HBASE-26179 TestRequestTooBigException spends too much time to finish (#3571)
    
    Signed-off-by: Yulin Niu <ni...@apache.org>
---
 .../hadoop/hbase/ipc/NettyRpcFrameDecoder.java     |  9 ++--
 .../org/apache/hadoop/hbase/ipc/RpcServer.java     |  7 +--
 .../hbase/ipc/SimpleServerRpcConnection.java       | 12 +++--
 .../hbase/client/TestRequestTooBigException.java   | 57 ++++++++++------------
 4 files changed, 40 insertions(+), 45 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
index 6891243..01cf9b5 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
@@ -130,16 +130,17 @@ public class NettyRpcFrameDecoder extends ByteToMessageDecoder {
     NettyServerCall reqTooBig = connection.createCall(header.getCallId(), connection.service, null,
       null, null, null, 0, connection.addr, 0, null);
 
-    connection.rpcServer.metrics.exception(SimpleRpcServer.REQUEST_TOO_BIG_EXCEPTION);
+    RequestTooBigException reqTooBigEx = new RequestTooBigException(requestTooBigMessage);
+    connection.rpcServer.metrics.exception(reqTooBigEx);
 
     // Make sure the client recognizes the underlying exception
     // Otherwise, throw a DoNotRetryIOException.
     if (VersionInfoUtil.hasMinimumVersion(connection.connectionHeader.getVersionInfo(),
       RequestTooBigException.MAJOR_VERSION, RequestTooBigException.MINOR_VERSION)) {
-      reqTooBig.setResponse(null, null,
-        SimpleRpcServer.REQUEST_TOO_BIG_EXCEPTION, requestTooBigMessage);
+      reqTooBig.setResponse(null, null, reqTooBigEx, requestTooBigMessage);
     } else {
-      reqTooBig.setResponse(null, null, new DoNotRetryIOException(), requestTooBigMessage);
+      reqTooBig.setResponse(null, null, new DoNotRetryIOException(requestTooBigMessage),
+        requestTooBigMessage);
     }
 
     // To guarantee that the message is written and flushed before closing the channel,
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
index 4ebb639..b41619a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/RpcServer.java
@@ -41,13 +41,12 @@ import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.Server;
 import org.apache.hadoop.hbase.conf.ConfigurationObserver;
-import org.apache.hadoop.hbase.exceptions.RequestTooBigException;
 import org.apache.hadoop.hbase.io.ByteBuffAllocator;
 import org.apache.hadoop.hbase.monitoring.MonitoredRPCHandler;
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
-import org.apache.hadoop.hbase.regionserver.RSRpcServices;
-import org.apache.hadoop.hbase.namequeues.RpcLogDetails;
 import org.apache.hadoop.hbase.namequeues.NamedQueueRecorder;
+import org.apache.hadoop.hbase.namequeues.RpcLogDetails;
+import org.apache.hadoop.hbase.regionserver.RSRpcServices;
 import org.apache.hadoop.hbase.security.HBasePolicyProvider;
 import org.apache.hadoop.hbase.security.SaslUtil;
 import org.apache.hadoop.hbase.security.SaslUtil.QualityOfProtection;
@@ -177,8 +176,6 @@ public abstract class RpcServer implements RpcServerInterface,
   protected HBaseRPCErrorHandler errorHandler = null;
 
   public static final String MAX_REQUEST_SIZE = "hbase.ipc.max.request.size";
-  protected static final RequestTooBigException REQUEST_TOO_BIG_EXCEPTION =
-      new RequestTooBigException();
 
   protected static final String WARN_RESPONSE_TIME = "hbase.ipc.warn.response.time";
   protected static final String WARN_RESPONSE_SIZE = "hbase.ipc.warn.response.size";
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
index 0c7057a..622e67a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/SimpleServerRpcConnection.java
@@ -28,19 +28,20 @@ import java.util.concurrent.ConcurrentLinkedDeque;
 import java.util.concurrent.atomic.LongAdder;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
-
 import org.apache.hadoop.hbase.CellScanner;
 import org.apache.hadoop.hbase.DoNotRetryIOException;
-import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.hadoop.hbase.client.VersionInfoUtil;
 import org.apache.hadoop.hbase.exceptions.RequestTooBigException;
 import org.apache.hadoop.hbase.ipc.RpcServer.CallCleanup;
 import org.apache.hadoop.hbase.nio.ByteBuff;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
+import org.apache.yetus.audience.InterfaceAudience;
+
 import org.apache.hbase.thirdparty.com.google.protobuf.BlockingService;
 import org.apache.hbase.thirdparty.com.google.protobuf.CodedInputStream;
 import org.apache.hbase.thirdparty.com.google.protobuf.Descriptors.MethodDescriptor;
 import org.apache.hbase.thirdparty.com.google.protobuf.Message;
+
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.RPCProtos.RequestHeader;
 
@@ -212,14 +213,15 @@ class SimpleServerRpcConnection extends ServerRpcConnection {
           SimpleServerCall reqTooBig = new SimpleServerCall(header.getCallId(), this.service, null,
               null, null, null, this, 0, this.addr, EnvironmentEdgeManager.currentTime(), 0,
               this.rpcServer.bbAllocator, this.rpcServer.cellBlockBuilder, null, responder);
-          this.rpcServer.metrics.exception(SimpleRpcServer.REQUEST_TOO_BIG_EXCEPTION);
+          RequestTooBigException reqTooBigEx = new RequestTooBigException(msg);
+          this.rpcServer.metrics.exception(reqTooBigEx);
           // Make sure the client recognizes the underlying exception
           // Otherwise, throw a DoNotRetryIOException.
           if (VersionInfoUtil.hasMinimumVersion(connectionHeader.getVersionInfo(),
             RequestTooBigException.MAJOR_VERSION, RequestTooBigException.MINOR_VERSION)) {
-            reqTooBig.setResponse(null, null, SimpleRpcServer.REQUEST_TOO_BIG_EXCEPTION, msg);
+            reqTooBig.setResponse(null, null, reqTooBigEx, msg);
           } else {
-            reqTooBig.setResponse(null, null, new DoNotRetryIOException(), msg);
+            reqTooBig.setResponse(null, null, new DoNotRetryIOException(msg), msg);
           }
           // In most cases we will write out the response directly. If not, it is still OK to just
           // close the connection without writing out the reqTooBig response. Do not try to write
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
index f81b3d4..c78bb00 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
@@ -18,7 +18,9 @@
 package org.apache.hadoop.hbase.client;
 
 import static org.apache.hadoop.hbase.ipc.RpcServer.MAX_REQUEST_SIZE;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertThrows;
+
+import java.util.concurrent.ThreadLocalRandom;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.TableName;
@@ -29,59 +31,52 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
-import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.TestName;
 
-@Category({MediumTests.class, ClientTests.class})
+import org.apache.hbase.thirdparty.com.google.common.io.Closeables;
+
+@Category({ MediumTests.class, ClientTests.class })
 public class TestRequestTooBigException {
 
   @ClassRule
   public static final HBaseClassTestRule CLASS_RULE =
-      HBaseClassTestRule.forClass(TestRequestTooBigException.class);
+    HBaseClassTestRule.forClass(TestRequestTooBigException.class);
 
   private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
 
-  @Rule
-  public TestName name = new TestName();
+  private static final TableName NAME = TableName.valueOf("request_too_big");
+
+  private static final byte[] FAMILY = Bytes.toBytes("family");
+
+  private static Table TABLE;
 
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
-    TEST_UTIL.getConfiguration().setInt(MAX_REQUEST_SIZE, 10000);
-    TEST_UTIL.startMiniCluster();
+    TEST_UTIL.getConfiguration().setInt(MAX_REQUEST_SIZE, 10 * 1024);
+    TEST_UTIL.startMiniCluster(1);
+    TABLE = TEST_UTIL.createTable(NAME, FAMILY);
+    TEST_UTIL.waitTableAvailable(NAME);
   }
 
   @AfterClass
   public static void tearDownAfterClass() throws Exception {
+    Closeables.close(TABLE, true);
     TEST_UTIL.shutdownMiniCluster();
   }
 
   @Test
   public void testHbasePutDeleteCell() throws Exception {
-    final TableName tableName = TableName.valueOf(name.getMethodName());
-    final byte[] family = Bytes.toBytes("cf");
-    Table table = TEST_UTIL.createTable(tableName, family);
-    TEST_UTIL.waitTableAvailable(tableName.getName(), 5000);
-    try {
-      byte[] value = new byte[2 * 2014 * 1024];
-      for (int m = 0; m < 10000; m++) {
-        Put p = new Put(Bytes.toBytes("bigrow"));
-        // big request = 400*2 M
-        for (int i = 0; i < 400; i++) {
-          p.addColumn(family, Bytes.toBytes("someQualifier" + i), value);
-        }
-        try {
-          table.put(p);
-          assertTrue("expected RequestTooBigException", false);
-        } catch (RequestTooBigException e) {
-          assertTrue("expected RequestTooBigException", true);
-        }
+    byte[] value = new byte[1024];
+    ThreadLocalRandom.current().nextBytes(value);
+    for (int m = 0; m < 100; m++) {
+      Put p = new Put(Bytes.toBytes("bigrow-" + m));
+      // max request is 10K, big request = 100 * 1K
+      for (int i = 0; i < 100; i++) {
+        p.addColumn(FAMILY, Bytes.toBytes("someQualifier" + i), value);
       }
-    } finally {
-      table.close();
+      final Put finalPut = p;
+      assertThrows(RequestTooBigException.class, () -> TABLE.put(finalPut));
     }
   }
 }
-
-

[hbase] 01/02: HBASE-26170 handleTooBigRequest in NettyRpcServer didn't skip enough bytes (#3564)

Posted by zh...@apache.org.
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

commit 61fbe6f4806d9b2f3c2051c6ed44f1a85f423d97
Author: Xiaolin Ha <ha...@apache.org>
AuthorDate: Thu Aug 5 23:05:26 2021 +0800

    HBASE-26170 handleTooBigRequest in NettyRpcServer didn't skip enough bytes (#3564)
    
    Signed-off-by: stack <st...@apache.com>
---
 .../hadoop/hbase/ipc/NettyRpcFrameDecoder.java     |  3 ++-
 .../hbase/client/TestRequestTooBigException.java   | 26 ++++++++++++----------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
index c46fe78..6891243 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcFrameDecoder.java
@@ -87,7 +87,6 @@ public class NettyRpcFrameDecoder extends ByteToMessageDecoder {
       NettyRpcServer.LOG.warn(requestTooBigMessage);
 
       if (connection.connectionHeaderRead) {
-        in.skipBytes(FRAME_LENGTH_FIELD_LENGTH);
         handleTooBigRequest(in);
         return;
       }
@@ -107,6 +106,7 @@ public class NettyRpcFrameDecoder extends ByteToMessageDecoder {
   }
 
   private void handleTooBigRequest(ByteBuf in) throws IOException {
+    in.skipBytes(FRAME_LENGTH_FIELD_LENGTH);
     in.markReaderIndex();
     int preIndex = in.readerIndex();
     int headerSize = readRawVarint32(in);
@@ -118,6 +118,7 @@ public class NettyRpcFrameDecoder extends ByteToMessageDecoder {
     }
 
     if (in.readableBytes() < headerSize) {
+      NettyRpcServer.LOG.debug("headerSize is larger than readableBytes");
       in.resetReaderIndex();
       return;
     }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
index 5f7f154..f81b3d4 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestRequestTooBigException.java
@@ -17,8 +17,8 @@
  */
 package org.apache.hadoop.hbase.client;
 
+import static org.apache.hadoop.hbase.ipc.RpcServer.MAX_REQUEST_SIZE;
 import static org.junit.Assert.assertTrue;
-
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.TableName;
@@ -48,6 +48,7 @@ public class TestRequestTooBigException {
 
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
+    TEST_UTIL.getConfiguration().setInt(MAX_REQUEST_SIZE, 10000);
     TEST_UTIL.startMiniCluster();
   }
 
@@ -64,17 +65,18 @@ public class TestRequestTooBigException {
     TEST_UTIL.waitTableAvailable(tableName.getName(), 5000);
     try {
       byte[] value = new byte[2 * 2014 * 1024];
-
-      Put p = new Put(Bytes.toBytes("bigrow"));
-      // big request = 400*2 M
-      for (int i = 0; i < 400; i++) {
-        p.addColumn(family, Bytes.toBytes("someQualifier" + i), value);
-      }
-      try {
-        table.put(p);
-        assertTrue("expected RequestTooBigException", false);
-      } catch (RequestTooBigException e) {
-        assertTrue("expected RequestTooBigException", true);
+      for (int m = 0; m < 10000; m++) {
+        Put p = new Put(Bytes.toBytes("bigrow"));
+        // big request = 400*2 M
+        for (int i = 0; i < 400; i++) {
+          p.addColumn(family, Bytes.toBytes("someQualifier" + i), value);
+        }
+        try {
+          table.put(p);
+          assertTrue("expected RequestTooBigException", false);
+        } catch (RequestTooBigException e) {
+          assertTrue("expected RequestTooBigException", true);
+        }
       }
     } finally {
       table.close();