You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2018/01/24 01:06:45 UTC

[1/2] hbase git commit: HBASE-19163 "Maximum lock count exceeded" from region server's batch processing

Repository: hbase
Updated Branches:
  refs/heads/branch-1.4 4531555f6 -> a75533ba5


HBASE-19163 "Maximum lock count exceeded" from region server's batch processing

Signed-off-by: Umesh Agashe <ua...@cloudera.com>
Signed-off-by: Michael Stack <st...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/02547d2f
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/02547d2f
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/02547d2f

Branch: refs/heads/branch-1.4
Commit: 02547d2f3b258fb7e3a26a0e29817845af364858
Parents: 4531555
Author: huaxiangsun <hu...@apache.org>
Authored: Fri Jan 19 11:22:00 2018 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Jan 23 16:19:52 2018 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/HRegion.java      | 39 ++++++++++++++++----
 .../hbase/client/TestFromClientSide3.java       | 27 ++++++++++++++
 .../hbase/regionserver/TestAtomicOperation.java |  6 +--
 3 files changed, 62 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/02547d2f/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index faab525..c006a44 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -3176,6 +3176,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
     // We try to set up a batch in the range [firstIndex,lastIndexExclusive)
     int firstIndex = batchOp.nextIndexToProcess;
     int lastIndexExclusive = firstIndex;
+    RowLock prevRowLock = null;
     boolean success = false;
     int noOfPuts = 0, noOfDeletes = 0;
     WALKey walKey = null;
@@ -3257,7 +3258,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
         boolean shouldBlock = numReadyToWrite == 0;
         RowLock rowLock = null;
         try {
-          rowLock = getRowLockInternal(mutation.getRow(), true, shouldBlock);
+          rowLock = getRowLockInternal(mutation.getRow(), true, shouldBlock, prevRowLock);
         } catch (TimeoutIOException e) {
           // We will retry when other exceptions, but we should stop if we timeout .
           throw e;
@@ -3271,7 +3272,12 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
           break;
 
         } else {
-          acquiredRowLocks.add(rowLock);
+          if (rowLock != prevRowLock) {
+            // It is a different row now, add this to the acquiredRowLocks and
+            // set prevRowLock to the new returned rowLock
+            acquiredRowLocks.add(rowLock);
+            prevRowLock = rowLock;
+          }
         }
 
         lastIndexExclusive++;
@@ -3368,7 +3374,7 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
               checkAndPrepareMutation(cpMutation, isInReplay, cpFamilyMap, now);
 
               // Acquire row locks. If not, the whole batch will fail.
-              acquiredRowLocks.add(getRowLockInternal(cpMutation.getRow(), true, true));
+              acquiredRowLocks.add(getRowLockInternal(cpMutation.getRow(), true, true, null));
 
               // Returned mutations from coprocessor correspond to the Mutation at index i. We can
               // directly add the cells from those mutations to the familyMaps of this mutation.
@@ -5464,17 +5470,17 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
   public RowLock getRowLock(byte[] row, boolean readLock, boolean waitForLock) throws IOException {
     // Make sure the row is inside of this region before getting the lock for it.
     checkRow(row, "row lock");
-    return getRowLockInternal(row, readLock, waitForLock);
+    return getRowLockInternal(row, readLock, waitForLock, null);
   }
 
   // getRowLock calls checkRow. Call this to skip checkRow.
   protected RowLock getRowLockInternal(byte[] row)
   throws IOException {
-    return getRowLockInternal(row, false, true);
+    return getRowLockInternal(row, false, true, null);
   }
 
-  protected RowLock getRowLockInternal(byte[] row, boolean readLock, boolean waitForLock)
-  throws IOException {
+  protected RowLock getRowLockInternal(byte[] row, boolean readLock, boolean waitForLock,
+      final RowLock prevRowLock) throws IOException {
     // create an object to use a a key in the row lock map
     HashedBytes rowKey = new HashedBytes(row);
 
@@ -5508,6 +5514,14 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
         //
         // This can fail as
         if (readLock) {
+          // For read lock, if the caller has locked the same row previously, it will not try
+          // to acquire the same read lock. It simply returns the previous row lock.
+          RowLockImpl prevRowLockImpl = (RowLockImpl)prevRowLock;
+          if ((prevRowLockImpl != null) && (prevRowLockImpl.getLock() ==
+              rowLockContext.readWriteLock.readLock())) {
+            success = true;
+            return prevRowLock;
+          }
           result = rowLockContext.newReadLock();
         } else {
           result = rowLockContext.newWriteLock();
@@ -5569,6 +5583,17 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
       }
       Thread.currentThread().interrupt();
       throw iie;
+    } catch (Error error) {
+      // The maximum lock count for read lock is 64K (hardcoded), when this maximum count
+      // is reached, it will throw out an Error. This Error needs to be caught so it can
+      // go ahead to process the minibatch with lock acquired.
+      LOG.warn("Error to get row lock for " + Bytes.toStringBinary(row) + ", cause: " + error);
+      IOException ioe = new IOException();
+      ioe.initCause(error);
+      if (traceScope != null) {
+        traceScope.getSpan().addTimelineAnnotation("Error getting row lock");
+      }
+      throw ioe;
     } finally {
       // Clean up the counts just in case this was the thing keeping the context alive.
       if (!success && rowLockContext != null) {

http://git-wip-us.apache.org/repos/asf/hbase/blob/02547d2f/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
index 385d4ad..515c989 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide3.java
@@ -413,6 +413,33 @@ public class TestFromClientSide3 {
     }
   }
 
+  // Test Table.batch with large amount of mutations against the same key.
+  // It used to trigger read lock's "Maximum lock count exceeded" Error.
+  @Test
+  public void testHTableWithLargeBatch() throws Exception {
+    Table table = TEST_UTIL.createTable(TableName.valueOf("testHTableWithLargeBatch"),
+        new byte[][] { FAMILY });
+    int sixtyFourK = 64 * 1024;
+    try {
+      List actions = new ArrayList();
+      Object[] results = new Object[(sixtyFourK + 1) * 2];
+
+      for (int i = 0; i < sixtyFourK + 1; i ++) {
+        Put put1 = new Put(ROW);
+        put1.addColumn(FAMILY, QUALIFIER, VALUE);
+        actions.add(put1);
+
+        Put put2 = new Put(ANOTHERROW);
+        put2.addColumn(FAMILY, QUALIFIER, VALUE);
+        actions.add(put2);
+      }
+
+      table.batch(actions, results);
+    } finally {
+      table.close();
+    }
+  }
+
   @Test
   public void testBatchWithRowMutation() throws Exception {
     LOG.info("Starting testBatchWithRowMutation");

http://git-wip-us.apache.org/repos/asf/hbase/blob/02547d2f/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
index d9415df..b9f2290 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestAtomicOperation.java
@@ -663,12 +663,12 @@ public class TestAtomicOperation {
     }
 
     @Override
-    public RowLock getRowLockInternal(final byte[] row, boolean readLock, boolean waitForLock)
-      throws IOException {
+    public RowLock getRowLockInternal(final byte[] row, boolean readLock, boolean waitForLock,
+        final RowLock prevRowLock) throws IOException {
       if (testStep == TestStep.CHECKANDPUT_STARTED) {
         latch.countDown();
       }
-      return new WrappedRowLock(super.getRowLockInternal(row, readLock, waitForLock));
+      return new WrappedRowLock(super.getRowLockInternal(row, readLock, waitForLock, prevRowLock));
     }
 
     public class WrappedRowLock implements RowLock {


[2/2] hbase git commit: HBASE-15321 - Ability to open a HRegion from hdfs snapshot.

Posted by ap...@apache.org.
HBASE-15321 - Ability to open a HRegion from hdfs snapshot.


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a75533ba
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a75533ba
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a75533ba

Branch: refs/heads/branch-1.4
Commit: a75533ba550b55bb740be3d33538f163d8a4cf72
Parents: 02547d2
Author: Rahul Gidwani <ch...@apache.org>
Authored: Mon Jan 22 12:22:57 2018 -0800
Committer: Andrew Purtell <ap...@apache.org>
Committed: Tue Jan 23 16:20:00 2018 -0800

----------------------------------------------------------------------
 .../hadoop/hbase/regionserver/HRegion.java      |  25 ++++
 .../hbase/regionserver/HRegionFileSystem.java   |   3 +-
 .../regionserver/TestHdfsSnapshotHRegion.java   | 115 +++++++++++++++++++
 3 files changed, 142 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/a75533ba/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index c006a44..e95a392 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -6998,6 +6998,31 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
     return this;
   }
 
+  /**
+   * Open a Region on a read-only file-system (like hdfs snapshots)
+   * @param conf The Configuration object to use.
+   * @param fs Filesystem to use
+   * @param info Info for region to be opened.
+   * @param htd the table descriptor
+   * @return new HRegion
+   * @throws IOException e
+   */
+  public static HRegion openReadOnlyFileSystemHRegion(final Configuration conf, final FileSystem fs,
+      final Path tableDir, HRegionInfo info, final HTableDescriptor htd) throws IOException {
+    if (info == null) {
+      throw new NullPointerException("Passed region info is null");
+    }
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Opening region (readOnly filesystem): " + info);
+    }
+    if (info.getReplicaId() <= 0) {
+      info = new HRegionInfo((HRegionInfo) info, 1);
+    }
+    HRegion r = HRegion.newHRegion(tableDir, null, fs, conf, info, htd, null);
+    r.writestate.setReadOnly(true);
+    return r.openHRegion(null);
+  }
+
   public static void warmupHRegion(final HRegionInfo info,
       final HTableDescriptor htd, final WAL wal, final Configuration conf,
       final RegionServerServices rsServices,

http://git-wip-us.apache.org/repos/asf/hbase/blob/a75533ba/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
index 3a0b30a..884485c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
@@ -28,6 +28,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
@@ -75,7 +76,7 @@ public class HRegionFileSystem {
   public static final String REGION_SPLITS_DIR = ".splits";
 
   /** Temporary subdirectory of the region directory used for compaction output. */
-  private static final String REGION_TEMP_DIR = ".tmp";
+  @VisibleForTesting static final String REGION_TEMP_DIR = ".tmp";
 
   private final HRegionInfo regionInfo;
   //regionInfo for interacting with FS (getting encodedName, etc)

http://git-wip-us.apache.org/repos/asf/hbase/blob/a75533ba/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHdfsSnapshotHRegion.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHdfsSnapshotHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHdfsSnapshotHRegion.java
new file mode 100644
index 0000000..64c3735
--- /dev/null
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHdfsSnapshotHRegion.java
@@ -0,0 +1,115 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import java.io.IOException;
+import org.apache.commons.lang.StringUtils;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.HBaseTestingUtility;
+import org.apache.hadoop.hbase.HRegionInfo;
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.testclassification.MediumTests;
+import org.apache.hadoop.hbase.testclassification.RegionServerTests;
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.hbase.util.FSUtils;
+import org.apache.hadoop.hdfs.DFSClient;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category({RegionServerTests.class, MediumTests.class})
+public class TestHdfsSnapshotHRegion {
+
+  private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+  private static final String SNAPSHOT_NAME = "foo_snapshot";
+  private Table table;
+  public static final TableName TABLE_NAME = TableName.valueOf("foo");
+  public static final byte[] FAMILY = Bytes.toBytes("f1");
+  private DFSClient client;
+  private String baseDir;
+
+
+  @Before
+  public void setUp() throws Exception {
+    Configuration c = TEST_UTIL.getConfiguration();
+    c.setBoolean("dfs.support.append", true);
+    TEST_UTIL.startMiniCluster(1);
+    table = TEST_UTIL.createMultiRegionTable(TABLE_NAME, FAMILY);
+    TEST_UTIL.loadTable(table, FAMILY);
+
+    // setup the hdfssnapshots
+    client = new DFSClient(TEST_UTIL.getDFSCluster().getURI(), TEST_UTIL.getConfiguration());
+    String fullUrIPath = TEST_UTIL.getDefaultRootDirPath().toString();
+    String uriString = TEST_UTIL.getTestFileSystem().getUri().toString();
+    baseDir = StringUtils.removeStart(fullUrIPath, uriString);
+    client.allowSnapshot(baseDir);
+  }
+
+  @After
+  public void tearDown() throws Exception {
+    client.deleteSnapshot(baseDir, SNAPSHOT_NAME);
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testOpeningReadOnlyRegionBasic() throws Exception {
+    String snapshotDir = client.createSnapshot(baseDir, SNAPSHOT_NAME);
+    HRegionInfo firstRegion = TEST_UTIL.getHBaseAdmin().getTableRegions(table.getName()).get(0);
+    Path tableDir = FSUtils.getTableDir(new Path(snapshotDir), TABLE_NAME);
+    HRegion snapshottedRegion = openSnapshotRegion(firstRegion, tableDir);
+    Assert.assertNotNull(snapshottedRegion);
+    snapshottedRegion.close();
+  }
+
+  @Test
+  public void testSnapshottingWithTmpSplitsAndMergeDirectoriesPresent() throws Exception {
+    // lets get a region and create those directories and make sure we ignore them
+    HRegionInfo firstRegion = TEST_UTIL.getHBaseAdmin().getTableRegions(table.getName()).get(0);
+    String encodedName = firstRegion.getEncodedName();
+    Path tableDir = FSUtils.getTableDir(TEST_UTIL.getDefaultRootDirPath(), TABLE_NAME);
+    Path regionDirectoryPath = new Path(tableDir, encodedName);
+    TEST_UTIL.getTestFileSystem().create(
+        new Path(regionDirectoryPath, HRegionFileSystem.REGION_TEMP_DIR));
+    TEST_UTIL.getTestFileSystem().create(
+        new Path(regionDirectoryPath, HRegionFileSystem.REGION_SPLITS_DIR));
+    TEST_UTIL.getTestFileSystem().create(
+        new Path(regionDirectoryPath, HRegionFileSystem.REGION_MERGES_DIR));
+    // now snapshot
+    String snapshotDir = client.createSnapshot(baseDir, "foo_snapshot");
+    // everything should still open just fine
+    HRegion snapshottedRegion = openSnapshotRegion(firstRegion,
+        FSUtils.getTableDir(new Path(snapshotDir), TABLE_NAME));
+    Assert.assertNotNull(snapshottedRegion); // no errors and the region should open
+    snapshottedRegion.close();
+  }
+
+  private HRegion openSnapshotRegion(HRegionInfo firstRegion, Path tableDir) throws IOException {
+    return HRegion.openReadOnlyFileSystemHRegion(
+        TEST_UTIL.getConfiguration(),
+        TEST_UTIL.getTestFileSystem(),
+        tableDir,
+        firstRegion,
+        table.getTableDescriptor()
+    );
+  }
+}