From a6329385a012aa8009ed0561a340e8a752b77a6a Mon Sep 17 00:00:00 2001 From: bitterfox Date: Tue, 28 Sep 2021 22:48:32 +0900 Subject: [PATCH] HBASE-26238 Short message by Result#compareResults for VerifyReplication to avoid OOME (#3647) Signed-off-by: Duo Zhang --- .../apache/hadoop/hbase/client/Result.java | 34 +++++++++++++++--- .../replication/VerifyReplication.java | 4 +-- .../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 bbf5ce84364..1ef1633d193 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 f1fdbf175f5..de7954052d1 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 1c3d32fff02..441e21ad29e 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 cells1 = new ArrayList<>(); + for (long i = 0; i < 100; i++) { + cells1.add(new KeyValue(row, family, Bytes.toBytes(i), value)); + } + + List 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 tags = null; if (tag != null) {