You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zg...@apache.org on 2019/09/19 00:38:46 UTC

[hbase] branch branch-2.1 updated (39222ca -> 79e6a95)

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

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


    from 39222ca  HBASE-23040 give friendly message about unknown server when running unload for RegionMover.
     new 01669de  HBASE-23044 CatalogJanitor#cleanMergeQualifier may clean wrong parent regions (#637)
     new 79e6a95  HBASE-23037 Make the split WAL related log more readable (#632)

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:
 .../org/apache/hadoop/hbase/MetaTableAccessor.java |  41 +----
 .../hadoop/hbase/master/SplitLogManager.java       |  11 +-
 .../org/apache/hadoop/hbase/wal/WALSplitter.java   |  23 ++-
 .../apache/hadoop/hbase/TestMetaTableAccessor.java | 195 +++++++++++++--------
 4 files changed, 153 insertions(+), 117 deletions(-)


[hbase] 01/02: HBASE-23044 CatalogJanitor#cleanMergeQualifier may clean wrong parent regions (#637)

Posted by zg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

zghao pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/hbase.git

commit 01669de6ea256d1d68bcc2a8ef25f03c37b8667e
Author: Guanghao Zhang <zg...@apache.org>
AuthorDate: Thu Sep 19 08:08:49 2019 +0800

    HBASE-23044 CatalogJanitor#cleanMergeQualifier may clean wrong parent regions (#637)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../org/apache/hadoop/hbase/MetaTableAccessor.java |  41 +----
 .../apache/hadoop/hbase/TestMetaTableAccessor.java | 195 +++++++++++++--------
 2 files changed, 130 insertions(+), 106 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
index 09148d7..9c13c12 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
@@ -59,8 +59,6 @@ import org.apache.hadoop.hbase.client.TableState;
 import org.apache.hadoop.hbase.exceptions.DeserializationException;
 import org.apache.hadoop.hbase.filter.Filter;
 import org.apache.hadoop.hbase.filter.FirstKeyOnlyFilter;
-import org.apache.hadoop.hbase.filter.QualifierFilter;
-import org.apache.hadoop.hbase.filter.RegexStringComparator;
 import org.apache.hadoop.hbase.filter.RowFilter;
 import org.apache.hadoop.hbase.filter.SubstringComparator;
 import org.apache.hadoop.hbase.ipc.CoprocessorRpcChannel;
@@ -374,7 +372,7 @@ public class MetaTableAccessor {
   @Nullable
   public static List<RegionInfo> getMergeRegions(Connection connection, byte[] regionName)
       throws IOException {
-    return getMergeRegions(getMergeRegionsRaw(connection, regionName));
+    return getMergeRegions(getRegionResult(connection, regionName).rawCells());
   }
 
   /**
@@ -428,31 +426,6 @@ public class MetaTableAccessor {
   }
 
   /**
-   * @return Array of Cells made from all columns on the <code>regionName</code> row
-   *   that match the regex 'info:merge.*'.
-   */
-  @Nullable
-  private static Cell [] getMergeRegionsRaw(Connection connection, byte [] regionName)
-      throws IOException {
-    Scan scan = new Scan().withStartRow(regionName).
-        setOneRowLimit().
-        readVersions(1).
-        addFamily(HConstants.CATALOG_FAMILY).
-        setFilter(new QualifierFilter(CompareOperator.EQUAL,
-          new RegexStringComparator(HConstants.MERGE_QUALIFIER_PREFIX_STR+ ".*")));
-    try (Table m = getMetaHTable(connection); ResultScanner scanner = m.getScanner(scan)) {
-      // Should be only one result in this scanner if any.
-      Result result = scanner.next();
-      if (result == null) {
-        return null;
-      }
-      // Should be safe to just return all Cells found since we had filter in place.
-      // All values should be RegionInfos or something wrong.
-      return result.rawCells();
-    }
-  }
-
-  /**
    * Checks if the specified table exists.  Looks at the hbase:meta table hosted on
    * the specified server.
    * @param connection connection we're using
@@ -1910,21 +1883,23 @@ public class MetaTableAccessor {
       throws IOException {
     Delete delete = new Delete(mergeRegion.getRegionName());
     // NOTE: We are doing a new hbase:meta read here.
-    Cell [] cells = getMergeRegionsRaw(connection, mergeRegion.getRegionName());
+    Cell[] cells = getRegionResult(connection, mergeRegion.getRegionName()).rawCells();
     if (cells == null || cells.length == 0) {
       return;
     }
-    List<byte[]> qualifiers = new ArrayList<>(cells.length);
+    List<byte[]> qualifiers = new ArrayList<>();
     for (Cell cell : cells) {
+      if (!isMergeQualifierPrefix(cell)) {
+        continue;
+      }
       byte[] qualifier = CellUtil.cloneQualifier(cell);
       qualifiers.add(qualifier);
       delete.addColumns(getCatalogFamily(), qualifier, HConstants.LATEST_TIMESTAMP);
     }
     deleteFromMetaTable(connection, delete);
     LOG.info("Deleted merge references in " + mergeRegion.getRegionNameAsString() +
-        ", deleted qualifiers " +
-        qualifiers.stream().map(Bytes::toStringBinary).
-            collect(Collectors.joining(", ")));
+        ", deleted qualifiers " + qualifiers.stream().map(Bytes::toStringBinary).
+        collect(Collectors.joining(", ")));
   }
 
   public static Put addRegionInfo(final Put p, final RegionInfo hri)
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java
index 680a751..9c196f0 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java
@@ -17,6 +17,13 @@
  */
 package org.apache.hadoop.hbase;
 
+import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyObject;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
@@ -30,6 +37,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Random;
+import java.util.concurrent.TimeUnit;
+
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.Connection;
@@ -61,7 +70,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
 import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
 
 import org.junit.AfterClass;
-import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
 import org.junit.Rule;
@@ -106,8 +114,49 @@ public class TestMetaTableAccessor {
     UTIL.shutdownMiniCluster();
   }
 
+  /**
+   * Test for HBASE-23044.
+   */
+  @Test
+  public void testGetMergeRegions() throws Exception {
+    TableName tn = TableName.valueOf(this.name.getMethodName());
+    UTIL.createMultiRegionTable(tn, Bytes.toBytes("CF"), 4);
+    UTIL.waitTableAvailable(tn);
+    try (Admin admin = UTIL.getAdmin()) {
+      List<RegionInfo> regions = admin.getRegions(tn);
+      assertEquals(4, regions.size());
+      admin.mergeRegionsAsync(regions.get(0).getRegionName(), regions.get(1).getRegionName(), false)
+          .get(60, TimeUnit.SECONDS);
+      admin.mergeRegionsAsync(regions.get(2).getRegionName(), regions.get(3).getRegionName(), false)
+          .get(60, TimeUnit.SECONDS);
+
+      List<RegionInfo> mergedRegions = admin.getRegions(tn);
+      assertEquals(2, mergedRegions.size());
+      RegionInfo mergedRegion0 = mergedRegions.get(0);
+      RegionInfo mergedRegion1 = mergedRegions.get(1);
+
+      List<RegionInfo> mergeParents =
+          MetaTableAccessor.getMergeRegions(connection, mergedRegion0.getRegionName());
+      assertTrue(mergeParents.contains(regions.get(0)));
+      assertTrue(mergeParents.contains(regions.get(1)));
+      mergeParents = MetaTableAccessor.getMergeRegions(connection, mergedRegion1.getRegionName());
+      assertTrue(mergeParents.contains(regions.get(2)));
+      assertTrue(mergeParents.contains(regions.get(3)));
+
+      // Delete merge qualifiers for mergedRegion0, then cannot getMergeRegions again
+      MetaTableAccessor.deleteMergeQualifiers(connection, mergedRegion0);
+      mergeParents = MetaTableAccessor.getMergeRegions(connection, mergedRegion0.getRegionName());
+      assertNull(mergeParents);
+
+      mergeParents = MetaTableAccessor.getMergeRegions(connection, mergedRegion1.getRegionName());
+      assertTrue(mergeParents.contains(regions.get(2)));
+      assertTrue(mergeParents.contains(regions.get(3)));
+    }
+    UTIL.deleteTable(tn);
+  }
+
   @Test
-  public void testGettingMergeRegions() throws IOException {
+  public void testAddMergeRegions() throws IOException {
     TableName tn = TableName.valueOf(this.name.getMethodName());
     Put put = new Put(Bytes.toBytes(this.name.getMethodName()));
     List<RegionInfo> ris = new ArrayList<>();
@@ -121,12 +170,12 @@ public class TestMetaTableAccessor {
     put = MetaTableAccessor.addMergeRegions(put, ris);
     List<Cell> cells = put.getFamilyCellMap().get(HConstants.CATALOG_FAMILY);
     String previousQualifier = null;
-    Assert.assertEquals(limit, cells.size());
+    assertEquals(limit, cells.size());
     for (Cell cell: cells) {
       LOG.info(cell.toString());
       String qualifier = Bytes.toString(cell.getQualifierArray());
-      Assert.assertTrue(qualifier.startsWith(HConstants.MERGE_QUALIFIER_PREFIX_STR));
-      Assert.assertNotEquals(qualifier, previousQualifier);
+      assertTrue(qualifier.startsWith(HConstants.MERGE_QUALIFIER_PREFIX_STR));
+      assertNotEquals(qualifier, previousQualifier);
       previousQualifier = qualifier;
     }
   }
@@ -134,7 +183,7 @@ public class TestMetaTableAccessor {
   @Test
   public void testIsMetaWhenAllHealthy() throws InterruptedException {
     HMaster m = UTIL.getMiniHBaseCluster().getMaster();
-    Assert.assertTrue(m.waitForMetaOnline());
+    assertTrue(m.waitForMetaOnline());
   }
 
   @Test
@@ -143,7 +192,7 @@ public class TestMetaTableAccessor {
     int index = UTIL.getMiniHBaseCluster().getServerWithMeta();
     HRegionServer rsWithMeta = UTIL.getMiniHBaseCluster().getRegionServer(index);
     rsWithMeta.abort("TESTING");
-    Assert.assertTrue(m.waitForMetaOnline());
+    assertTrue(m.waitForMetaOnline());
   }
 
   /**
@@ -188,8 +237,8 @@ public class TestMetaTableAccessor {
 
     try {
       // Make sure reader and writer are working.
-      Assert.assertTrue(reader.isProgressing());
-      Assert.assertTrue(writer.isProgressing());
+      assertTrue(reader.isProgressing());
+      assertTrue(writer.isProgressing());
 
       // Kill server hosting meta -- twice  . See if our reader/writer ride over the
       // meta moves.  They'll need to retry.
@@ -208,8 +257,8 @@ public class TestMetaTableAccessor {
         }
       }
 
-      Assert.assertTrue("reader: " + reader.toString(), reader.isProgressing());
-      Assert.assertTrue("writer: " + writer.toString(), writer.isProgressing());
+      assertTrue("reader: " + reader.toString(), reader.isProgressing());
+      assertTrue("writer: " + writer.toString(), writer.isProgressing());
     } catch (IOException e) {
       throw e;
     } finally {
@@ -220,7 +269,7 @@ public class TestMetaTableAccessor {
       t.close();
     }
     long exeTime = System.currentTimeMillis() - startTime;
-    Assert.assertTrue("Timeout: test took " + exeTime / 1000 + " sec", exeTime < timeOut);
+    assertTrue("Timeout: test took " + exeTime / 1000 + " sec", exeTime < timeOut);
   }
 
   /**
@@ -276,27 +325,27 @@ public class TestMetaTableAccessor {
   public void testGetRegionsFromMetaTable() throws IOException, InterruptedException {
     MetaTableLocator mtl = new MetaTableLocator();
     List<RegionInfo> regions = mtl.getMetaRegions(UTIL.getZooKeeperWatcher());
-    Assert.assertTrue(regions.size() >= 1);
-    Assert.assertTrue(
+    assertTrue(regions.size() >= 1);
+    assertTrue(
         mtl.getMetaRegionsAndLocations(UTIL.getZooKeeperWatcher()).size() >= 1);
   }
 
   @Test public void testTableExists() throws IOException {
     final TableName tableName = TableName.valueOf(name.getMethodName());
-    Assert.assertFalse(MetaTableAccessor.tableExists(connection, tableName));
+    assertFalse(MetaTableAccessor.tableExists(connection, tableName));
     UTIL.createTable(tableName, HConstants.CATALOG_FAMILY);
-    Assert.assertTrue(MetaTableAccessor.tableExists(connection, tableName));
+    assertTrue(MetaTableAccessor.tableExists(connection, tableName));
     Admin admin = UTIL.getAdmin();
     admin.disableTable(tableName);
     admin.deleteTable(tableName);
-    Assert.assertFalse(MetaTableAccessor.tableExists(connection, tableName));
-    Assert.assertTrue(MetaTableAccessor.tableExists(connection,
+    assertFalse(MetaTableAccessor.tableExists(connection, tableName));
+    assertTrue(MetaTableAccessor.tableExists(connection,
         TableName.META_TABLE_NAME));
     UTIL.createTable(tableName, HConstants.CATALOG_FAMILY);
-    Assert.assertTrue(MetaTableAccessor.tableExists(connection, tableName));
+    assertTrue(MetaTableAccessor.tableExists(connection, tableName));
     admin.disableTable(tableName);
     admin.deleteTable(tableName);
-    Assert.assertFalse(MetaTableAccessor.tableExists(connection, tableName));
+    assertFalse(MetaTableAccessor.tableExists(connection, tableName));
   }
 
   @Test public void testGetRegion() throws IOException, InterruptedException {
@@ -305,7 +354,7 @@ public class TestMetaTableAccessor {
     // Test get on non-existent region.
     Pair<RegionInfo, ServerName> pair =
       MetaTableAccessor.getRegion(connection, Bytes.toBytes("nonexistent-region"));
-    Assert.assertNull(pair);
+    assertNull(pair);
     LOG.info("Finished " + name);
   }
 
@@ -328,18 +377,18 @@ public class TestMetaTableAccessor {
 
     // Now make sure we only get the regions from 1 of the tables at a time
 
-    Assert.assertEquals(1, MetaTableAccessor.getTableRegions(connection, tableName).size());
-    Assert.assertEquals(1, MetaTableAccessor.getTableRegions(connection, greaterName).size());
+    assertEquals(1, MetaTableAccessor.getTableRegions(connection, tableName).size());
+    assertEquals(1, MetaTableAccessor.getTableRegions(connection, greaterName).size());
   }
 
   private static List<RegionInfo> testGettingTableRegions(final Connection connection,
       final TableName name, final int regionCount)
   throws IOException, InterruptedException {
     List<RegionInfo> regions = MetaTableAccessor.getTableRegions(connection, name);
-    Assert.assertEquals(regionCount, regions.size());
+    assertEquals(regionCount, regions.size());
     Pair<RegionInfo, ServerName> pair =
       MetaTableAccessor.getRegion(connection, regions.get(0).getRegionName());
-    Assert.assertEquals(regions.get(0).getEncodedName(),
+    assertEquals(regions.get(0).getEncodedName(),
       pair.getFirst().getEncodedName());
     return regions;
   }
@@ -349,48 +398,48 @@ public class TestMetaTableAccessor {
   throws IOException, InterruptedException {
     Pair<RegionInfo, ServerName> pair =
       MetaTableAccessor.getRegion(connection, region.getRegionName());
-    Assert.assertEquals(region.getEncodedName(),
+    assertEquals(region.getEncodedName(),
       pair.getFirst().getEncodedName());
   }
 
   @Test
   public void testParseReplicaIdFromServerColumn() {
     String column1 = HConstants.SERVER_QUALIFIER_STR;
-    Assert.assertEquals(0,
+    assertEquals(0,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column1)));
     String column2 = column1 + MetaTableAccessor.META_REPLICA_ID_DELIMITER;
-    Assert.assertEquals(-1,
+    assertEquals(-1,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column2)));
     String column3 = column2 + "00";
-    Assert.assertEquals(-1,
+    assertEquals(-1,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column3)));
     String column4 = column3 + "2A";
-    Assert.assertEquals(42,
+    assertEquals(42,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column4)));
     String column5 = column4 + "2A";
-    Assert.assertEquals(-1,
+    assertEquals(-1,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column5)));
     String column6 = HConstants.STARTCODE_QUALIFIER_STR;
-    Assert.assertEquals(-1,
+    assertEquals(-1,
         MetaTableAccessor.parseReplicaIdFromServerColumn(Bytes.toBytes(column6)));
   }
 
   @Test
   public void testMetaReaderGetColumnMethods() {
-    Assert.assertArrayEquals(HConstants.SERVER_QUALIFIER, MetaTableAccessor.getServerColumn(0));
-    Assert.assertArrayEquals(Bytes.toBytes(HConstants.SERVER_QUALIFIER_STR
+    assertArrayEquals(HConstants.SERVER_QUALIFIER, MetaTableAccessor.getServerColumn(0));
+    assertArrayEquals(Bytes.toBytes(HConstants.SERVER_QUALIFIER_STR
       + MetaTableAccessor.META_REPLICA_ID_DELIMITER + "002A"),
       MetaTableAccessor.getServerColumn(42));
 
-    Assert.assertArrayEquals(HConstants.STARTCODE_QUALIFIER,
+    assertArrayEquals(HConstants.STARTCODE_QUALIFIER,
       MetaTableAccessor.getStartCodeColumn(0));
-    Assert.assertArrayEquals(Bytes.toBytes(HConstants.STARTCODE_QUALIFIER_STR
+    assertArrayEquals(Bytes.toBytes(HConstants.STARTCODE_QUALIFIER_STR
       + MetaTableAccessor.META_REPLICA_ID_DELIMITER + "002A"),
       MetaTableAccessor.getStartCodeColumn(42));
 
-    Assert.assertArrayEquals(HConstants.SEQNUM_QUALIFIER,
+    assertArrayEquals(HConstants.SEQNUM_QUALIFIER,
       MetaTableAccessor.getSeqNumColumn(0));
-    Assert.assertArrayEquals(Bytes.toBytes(HConstants.SEQNUM_QUALIFIER_STR
+    assertArrayEquals(Bytes.toBytes(HConstants.SEQNUM_QUALIFIER_STR
       + MetaTableAccessor.META_REPLICA_ID_DELIMITER + "002A"),
       MetaTableAccessor.getSeqNumColumn(42));
   }
@@ -459,14 +508,14 @@ public class TestMetaTableAccessor {
       long seqNum, int replicaId, boolean checkSeqNum) throws IOException {
     Get get = new Get(row);
     Result result = meta.get(get);
-    Assert.assertTrue(Bytes.equals(
+    assertTrue(Bytes.equals(
       result.getValue(HConstants.CATALOG_FAMILY, MetaTableAccessor.getServerColumn(replicaId)),
       Bytes.toBytes(serverName.getHostAndPort())));
-    Assert.assertTrue(Bytes.equals(
+    assertTrue(Bytes.equals(
       result.getValue(HConstants.CATALOG_FAMILY, MetaTableAccessor.getStartCodeColumn(replicaId)),
       Bytes.toBytes(serverName.getStartcode())));
     if (checkSeqNum) {
-      Assert.assertTrue(Bytes.equals(
+      assertTrue(Bytes.equals(
         result.getValue(HConstants.CATALOG_FAMILY, MetaTableAccessor.getSeqNumColumn(replicaId)),
         Bytes.toBytes(seqNum)));
     }
@@ -480,10 +529,10 @@ public class TestMetaTableAccessor {
         MetaTableAccessor.getServerColumn(replicaId));
     Cell startCodeCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
       MetaTableAccessor.getStartCodeColumn(replicaId));
-    Assert.assertNotNull(serverCell);
-    Assert.assertNotNull(startCodeCell);
-    Assert.assertEquals(0, serverCell.getValueLength());
-    Assert.assertEquals(0, startCodeCell.getValueLength());
+    assertNotNull(serverCell);
+    assertNotNull(startCodeCell);
+    assertEquals(0, serverCell.getValueLength());
+    assertEquals(0, startCodeCell.getValueLength());
   }
 
   @Test
@@ -511,12 +560,12 @@ public class TestMetaTableAccessor {
         Cell snCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
           MetaTableAccessor.getServerNameColumn(replicaId));
         if (replicaId == 0) {
-          Assert.assertNotNull(stateCell);
+          assertNotNull(stateCell);
         } else {
-          Assert.assertNull(serverCell);
-          Assert.assertNull(startCodeCell);
-          Assert.assertNull(stateCell);
-          Assert.assertNull(snCell);
+          assertNull(serverCell);
+          assertNull(startCodeCell);
+          assertNull(stateCell);
+          assertNull(snCell);
         }
       }
     } finally {
@@ -707,15 +756,15 @@ public class TestMetaTableAccessor {
         MetaTableAccessor.getStartCodeColumn(0));
       Cell seqNumCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
         MetaTableAccessor.getSeqNumColumn(0));
-      Assert.assertNotNull(serverCell);
-      Assert.assertNotNull(startCodeCell);
-      Assert.assertNotNull(seqNumCell);
-      Assert.assertTrue(serverCell.getValueLength() > 0);
-      Assert.assertTrue(startCodeCell.getValueLength() > 0);
-      Assert.assertTrue(seqNumCell.getValueLength() > 0);
-      Assert.assertEquals(masterSystemTime, serverCell.getTimestamp());
-      Assert.assertEquals(masterSystemTime, startCodeCell.getTimestamp());
-      Assert.assertEquals(masterSystemTime, seqNumCell.getTimestamp());
+      assertNotNull(serverCell);
+      assertNotNull(startCodeCell);
+      assertNotNull(seqNumCell);
+      assertTrue(serverCell.getValueLength() > 0);
+      assertTrue(startCodeCell.getValueLength() > 0);
+      assertTrue(seqNumCell.getValueLength() > 0);
+      assertEquals(masterSystemTime, serverCell.getTimestamp());
+      assertEquals(masterSystemTime, startCodeCell.getTimestamp());
+      assertEquals(masterSystemTime, seqNumCell.getTimestamp());
     }
   }
 
@@ -765,8 +814,8 @@ public class TestMetaTableAccessor {
       Result result = meta.get(get);
       Cell serverCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
           MetaTableAccessor.getServerColumn(0));
-      Assert.assertNotNull(serverCell);
-      Assert.assertEquals(serverNameTime, serverCell.getTimestamp());
+      assertNotNull(serverCell);
+      assertEquals(serverNameTime, serverCell.getTimestamp());
 
       ManualEnvironmentEdge edge = new ManualEnvironmentEdge();
       edge.setValue(masterSystemTime);
@@ -787,9 +836,9 @@ public class TestMetaTableAccessor {
         MetaTableAccessor.getStartCodeColumn(0));
       Cell seqNumCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY,
         MetaTableAccessor.getSeqNumColumn(0));
-      Assert.assertNull(serverCell);
-      Assert.assertNull(startCodeCell);
-      Assert.assertNull(seqNumCell);
+      assertNull(serverCell);
+      assertNull(startCodeCell);
+      assertNull(seqNumCell);
     }
   }
 
@@ -870,7 +919,7 @@ public class TestMetaTableAccessor {
       MetaTableAccessor.splitRegion(connection, parent, -1L, splitA, splitB, loc.getServerName(),
         1);
 
-      Assert.assertTrue(prevCalls < scheduler.numPriorityCalls);
+      assertTrue(prevCalls < scheduler.numPriorityCalls);
     }
   }
 
@@ -912,8 +961,8 @@ public class TestMetaTableAccessor {
         MetaTableAccessor.getServerColumn(splitA.getReplicaId()));
       Cell startCodeCellA = resultA.getColumnLatestCell(HConstants.CATALOG_FAMILY,
         MetaTableAccessor.getStartCodeColumn(splitA.getReplicaId()));
-      Assert.assertNull(serverCellA);
-      Assert.assertNull(startCodeCellA);
+      assertNull(serverCellA);
+      assertNull(startCodeCellA);
 
       Get get2 = new Get(splitA.getRegionName());
       Result resultB = meta.get(get2);
@@ -921,8 +970,8 @@ public class TestMetaTableAccessor {
         MetaTableAccessor.getServerColumn(splitB.getReplicaId()));
       Cell startCodeCellB = resultB.getColumnLatestCell(HConstants.CATALOG_FAMILY,
         MetaTableAccessor.getStartCodeColumn(splitB.getReplicaId()));
-      Assert.assertNull(serverCellB);
-      Assert.assertNull(startCodeCellB);
+      assertNull(serverCellB);
+      assertNull(startCodeCellB);
     } finally {
       if (meta != null) {
         meta.close();
@@ -938,10 +987,10 @@ public class TestMetaTableAccessor {
     final String encodedName = regions.get(0).getRegionInfo().getEncodedName();
     final Result result = MetaTableAccessor.scanByRegionEncodedName(UTIL.getConnection(),
       encodedName);
-    Assert.assertNotNull(result);
-    Assert.assertTrue(result.advance());
+    assertNotNull(result);
+    assertTrue(result.advance());
     final String resultingRowKey = CellUtil.getCellKeyAsString(result.current());
-    Assert.assertTrue(resultingRowKey.contains(encodedName));
+    assertTrue(resultingRowKey.contains(encodedName));
     UTIL.deleteTable(tableName);
   }
 
@@ -950,7 +999,7 @@ public class TestMetaTableAccessor {
     final String encodedName = "nonexistingregion";
     final Result result = MetaTableAccessor.scanByRegionEncodedName(UTIL.getConnection(),
       encodedName);
-    Assert.assertNull(result);
+    assertNull(result);
   }
 }
 


[hbase] 02/02: HBASE-23037 Make the split WAL related log more readable (#632)

Posted by zg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

zghao pushed a commit to branch branch-2.1
in repository https://gitbox.apache.org/repos/asf/hbase.git

commit 79e6a959ab0826655e64c3bffb2690386d09cfa7
Author: Guanghao Zhang <zg...@apache.org>
AuthorDate: Wed Sep 18 21:44:05 2019 +0800

    HBASE-23037 Make the split WAL related log more readable (#632)
    
    Signed-off-by: Duo Zhang <zh...@apache.org>
---
 .../hadoop/hbase/master/SplitLogManager.java       | 11 +++++++----
 .../org/apache/hadoop/hbase/wal/WALSplitter.java   | 23 +++++++++++++++-------
 2 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
index 414fc98..f0411d4 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/SplitLogManager.java
@@ -51,6 +51,7 @@ import org.apache.hadoop.hbase.coordination.SplitLogManagerCoordination.SplitLog
 import org.apache.hadoop.hbase.log.HBaseMarkers;
 import org.apache.hadoop.hbase.monitoring.MonitoredTask;
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
 import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.FSUtils;
 import org.apache.hadoop.hbase.util.HasThread;
@@ -289,10 +290,12 @@ public class SplitLogManager {
       }
       SplitLogCounters.tot_mgr_log_split_batch_success.increment();
     }
-    String msg = "Finished splitting (more than or equal to) " + totalSize +
-        " bytes in " + ((batch == null)? 0: batch.installed) +
-        " log files in " + logDirs + " in " +
-        ((startTime == 0)? startTime: (EnvironmentEdgeManager.currentTime() - startTime)) + "ms";
+    String msg =
+        "Finished splitting (more than or equal to) " + StringUtils.humanSize(totalSize) + " (" +
+            totalSize + " bytes) in " + ((batch == null) ? 0 : batch.installed) + " log files in " +
+            logDirs + " in " +
+            ((startTime == 0) ? startTime : (EnvironmentEdgeManager.currentTime() - startTime)) +
+            "ms";
     status.markComplete(msg);
     LOG.info(msg);
     return totalSize;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java
index fd56d73..afc36d0 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java
@@ -71,6 +71,7 @@ import org.apache.hadoop.hbase.master.SplitLogManager;
 import org.apache.hadoop.hbase.monitoring.MonitoredTask;
 import org.apache.hadoop.hbase.monitoring.TaskMonitor;
 import org.apache.hadoop.hbase.regionserver.HRegion;
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
 import org.apache.hadoop.hbase.regionserver.LastSequenceId;
 import org.apache.hadoop.hbase.regionserver.wal.AbstractFSWAL;
 import org.apache.hadoop.hbase.regionserver.wal.WALCellCodec;
@@ -78,6 +79,7 @@ import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.CancelableProgressable;
 import org.apache.hadoop.hbase.util.ClassSize;
 import org.apache.hadoop.hbase.util.CollectionUtils.IOExceptionSupplier;
+import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
 import org.apache.hadoop.hbase.util.FSUtils;
 import org.apache.hadoop.hbase.util.Pair;
 import org.apache.hadoop.hbase.util.Threads;
@@ -246,9 +248,11 @@ public class WALSplitter {
           "Splitting log file " + logfile.getPath() + "into a temporary staging area.");
     Reader logFileReader = null;
     this.fileBeingSplit = logfile;
+    long startTS = EnvironmentEdgeManager.currentTime();
     try {
       long logLength = logfile.getLen();
-      LOG.info("Splitting WAL={}, length={}", logPath, logLength);
+      LOG.info("Splitting WAL={}, size={} ({} bytes)", logPath, StringUtils.humanSize(logLength),
+          logLength);
       status.setStatus("Opening log file");
       if (reporter != null && !reporter.progress()) {
         progress_failed = true;
@@ -259,6 +263,8 @@ public class WALSplitter {
         LOG.warn("Nothing to split in WAL={}", logPath);
         return true;
       }
+      long openCost = EnvironmentEdgeManager.currentTime() - startTS;
+      LOG.info("Open WAL={} cost {} ms", logPath, openCost);
       int numOpenedFilesBeforeReporting = conf.getInt("hbase.splitlog.report.openedfiles", 3);
       int numOpenedFilesLastCheck = 0;
       outputSink.setReporter(reporter);
@@ -266,6 +272,7 @@ public class WALSplitter {
       outputSinkStarted = true;
       Entry entry;
       Long lastFlushedSequenceId = -1L;
+      startTS = EnvironmentEdgeManager.currentTime();
       while ((entry = getNextLogLine(logFileReader, logPath, skipErrors)) != null) {
         byte[] region = entry.getKey().getEncodedRegionName();
         String encodedRegionNameAsStr = Bytes.toString(region);
@@ -349,11 +356,13 @@ public class WALSplitter {
           progress_failed = outputSink.finishWritingAndClose() == null;
         }
       } finally {
-        String msg =
-            "Processed " + editsCount + " edits across " + outputSink.getNumberOfRecoveredRegions()
-                + " regions; edits skipped=" + editsSkipped + "; log file=" + logPath +
-                ", length=" + logfile.getLen() + // See if length got updated post lease recovery
-                ", corrupted=" + isCorrupted + ", progress failed=" + progress_failed;
+        long processCost = EnvironmentEdgeManager.currentTime() - startTS;
+        // See if length got updated post lease recovery
+        String msg = "Processed " + editsCount + " edits across " +
+            outputSink.getNumberOfRecoveredRegions() + " regions cost " + processCost +
+            " ms; edits skipped=" + editsSkipped + "; WAL=" + logPath + ", size=" +
+            StringUtils.humanSize(logfile.getLen()) + ", length=" + logfile.getLen() +
+            ", corrupted=" + isCorrupted + ", progress failed=" + progress_failed;
         LOG.info(msg);
         status.markComplete(msg);
       }
@@ -1707,7 +1716,7 @@ public class WALSplitter {
         throws InterruptedException, ExecutionException {
       for (final Map.Entry<byte[], RegionEntryBuffer> buffer : entryBuffers.buffers.entrySet()) {
         LOG.info("Submitting writeThenClose of {}",
-            Arrays.toString(buffer.getValue().encodedRegionName));
+            Bytes.toString(buffer.getValue().encodedRegionName));
         completionService.submit(new Callable<Void>() {
           @Override
           public Void call() throws Exception {