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/09/28 15:26:58 UTC

[hbase] branch branch-2 updated: HBASE-26238 Short message by Result#compareResults for VerifyReplication to avoid OOME (#3647)

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 786e09a  HBASE-26238 Short message by Result#compareResults for VerifyReplication to avoid OOME (#3647)
786e09a is described below

commit 786e09a93628b983011f3f6001cd7d94b47a3f4d
Author: bitterfox <yo...@linecorp.com>
AuthorDate: Tue Sep 28 22:48:32 2021 +0900

    HBASE-26238 Short message by Result#compareResults for VerifyReplication to avoid OOME (#3647)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../org/apache/hadoop/hbase/client/Result.java     | 34 ++++++++++++++++++---
 .../mapreduce/replication/VerifyReplication.java   |  4 +--
 .../org/apache/hadoop/hbase/client/TestResult.java | 35 ++++++++++++++++++++++
 3 files changed, 67 insertions(+), 6 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
index bbf5ce8..1ef1633 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Result.java
@@ -778,14 +778,35 @@ public class Result implements CellScannable, CellScanner {
    * @throws Exception Every difference is throwing an exception
    */
   public static void compareResults(Result res1, Result res2)
+    throws Exception{
+    compareResults(res1, res2, true);
+  }
+
+  /**
+   * Does a deep comparison of two Results, down to the byte arrays.
+   * @param res1 first result to compare
+   * @param res2 second result to compare
+   * @param verbose includes string representation for all cells in the exception if true;
+   *                otherwise include rowkey only
+   * @throws Exception Every difference is throwing an exception
+   */
+  public static void compareResults(Result res1, Result res2, boolean verbose)
       throws Exception {
     if (res2 == null) {
       throw new Exception("There wasn't enough rows, we stopped at "
           + Bytes.toStringBinary(res1.getRow()));
     }
     if (res1.size() != res2.size()) {
-      throw new Exception("This row doesn't have the same number of KVs: "
-          + res1.toString() + " compared to " + res2.toString());
+      if (verbose) {
+        throw new Exception(
+          "This row doesn't have the same number of KVs: "
+            + res1 + " compared to " + res2);
+      } else {
+        throw new Exception(
+          "This row doesn't have the same number of KVs: row="
+            + Bytes.toStringBinary(res1.getRow())
+            + ", " + res1.size() + " cells are compared to " + res2.size() + " cells");
+      }
     }
     Cell[] ourKVs = res1.rawCells();
     Cell[] replicatedKVs = res2.rawCells();
@@ -793,8 +814,13 @@ public class Result implements CellScannable, CellScanner {
       if (!ourKVs[i].equals(replicatedKVs[i]) ||
           !CellUtil.matchingValue(ourKVs[i], replicatedKVs[i]) ||
           !CellUtil.matchingTags(ourKVs[i], replicatedKVs[i])) {
-        throw new Exception("This result was different: "
-            + res1.toString() + " compared to " + res2.toString());
+        if (verbose) {
+          throw new Exception("This result was different: "
+            + res1 + " compared to " + res2);
+        } else {
+          throw new Exception("This result was different: row="
+            + Bytes.toStringBinary(res1.getRow()));
+        }
       }
     }
   }
diff --git a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/replication/VerifyReplication.java b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/replication/VerifyReplication.java
index f1fdbf1..de79540 100644
--- a/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/replication/VerifyReplication.java
+++ b/hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/replication/VerifyReplication.java
@@ -236,7 +236,7 @@ public class VerifyReplication extends Configured implements Tool {
         if (rowCmpRet == 0) {
           // rowkey is same, need to compare the content of the row
           try {
-            Result.compareResults(value, currentCompareRowInPeerTable);
+            Result.compareResults(value, currentCompareRowInPeerTable, false);
             context.getCounter(Counters.GOODROWS).increment(1);
             if (verbose) {
               LOG.info("Good row key: " + delimiter
@@ -266,7 +266,7 @@ public class VerifyReplication extends Configured implements Tool {
         try {
           Result sourceResult = sourceTable.get(new Get(row.getRow()));
           Result replicatedResult = replicatedTable.get(new Get(row.getRow()));
-          Result.compareResults(sourceResult, replicatedResult);
+          Result.compareResults(sourceResult, replicatedResult, false);
           if (!sourceResult.isEmpty()) {
             context.getCounter(Counters.GOODROWS).increment(1);
             if (verbose) {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
index 1c3d32f..441e21a 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestResult.java
@@ -18,9 +18,14 @@
 package org.apache.hadoop.hbase.client;
 
 import static org.apache.hadoop.hbase.HBaseTestCase.assertByteEquals;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.lessThan;
 
 import java.io.IOException;
 import java.nio.ByteBuffer;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.NoSuchElementException;
@@ -369,6 +374,36 @@ public class TestResult extends TestCase {
     }
   }
 
+  public void testCompareResultMemoryUsage() {
+    List<Cell> cells1 = new ArrayList<>();
+    for (long i = 0; i < 100; i++) {
+      cells1.add(new KeyValue(row, family, Bytes.toBytes(i), value));
+    }
+
+    List<Cell> cells2 = new ArrayList<>();
+    for (long i = 0; i < 100; i++) {
+      cells2.add(new KeyValue(row, family, Bytes.toBytes(i), Bytes.toBytes(i)));
+    }
+
+    Result r1 = Result.create(cells1);
+    Result r2 = Result.create(cells2);
+    try {
+      Result.compareResults(r1, r2);
+      fail();
+    } catch (Exception x) {
+      assertTrue(x.getMessage().startsWith("This result was different:"));
+      assertThat(x.getMessage().length(), is(greaterThan(100)));
+    }
+
+    try {
+      Result.compareResults(r1, r2, false);
+      fail();
+    } catch (Exception x) {
+      assertEquals("This result was different: row=row", x.getMessage());
+      assertThat(x.getMessage().length(), is(lessThan(100)));
+    }
+  }
+
   private Result getArrayBackedTagResult(Tag tag) {
     List<Tag> tags = null;
     if (tag != null) {