[hbase] branch branch-2 updated: HBASE-22882 TestFlushSnapshotFromClient#testConcurrentSnapshottingAttempts is flakey (was written flakey)

[st...@apache.org]

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

stack 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 b7c7b4a  HBASE-22882 TestFlushSnapshotFromClient#testConcurrentSnapshottingAttempts is flakey (was written flakey)
b7c7b4a is described below

commit b7c7b4a5702ab94156ee753823bd02342a09e3d2
Author: stack <st...@apache.org>
AuthorDate: Mon Aug 19 12:57:12 2019 -0700

    HBASE-22882 TestFlushSnapshotFromClient#testConcurrentSnapshottingAttempts is flakey (was written flakey)
---
 .../hadoop/hbase/procedure/ProcedureCoordinator.java    |  5 ++---
 .../hbase/snapshot/TestFlushSnapshotFromClient.java     | 17 +++++++++++------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ProcedureCoordinator.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ProcedureCoordinator.java
index 5a1516e..d5800b1 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ProcedureCoordinator.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ProcedureCoordinator.java
@@ -29,6 +29,7 @@ import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
+import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -67,9 +68,9 @@ public class ProcedureCoordinator {
    * The rpc object registers the ProcedureCoordinator and starts any threads in this
    * constructor.
    *
-   * @param rpcs
    * @param pool Used for executing procedures.
    */
+  @VisibleForTesting // Only used in tests. SimpleMasterProcedureManager is a test class.
   public ProcedureCoordinator(ProcedureCoordinatorRpcs rpcs, ThreadPoolExecutor pool) {
     this(rpcs, pool, TIMEOUT_MILLIS_DEFAULT, WAKE_MILLIS_DEFAULT);
   }
@@ -80,9 +81,7 @@ public class ProcedureCoordinator {
    * The rpc object registers the ProcedureCoordinator and starts any threads in
    * this constructor.
    *
-   * @param rpcs
    * @param pool Used for executing procedures.
-   * @param timeoutMillis
    */
   public ProcedureCoordinator(ProcedureCoordinatorRpcs rpcs, ThreadPoolExecutor pool,
       long timeoutMillis, long wakeTimeMillis) {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java
index 74ae830..73d68f2 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java
@@ -52,6 +52,7 @@ import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.ClassRule;
+import org.junit.Ignore;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.slf4j.Logger;
@@ -136,7 +137,6 @@ public class TestFlushSnapshotFromClient {
 
   /**
    * Test simple flush snapshotting a table that is online
-   * @throws Exception
    */
   @Test
   public void testFlushTableSnapshot() throws Exception {
@@ -169,7 +169,6 @@ public class TestFlushSnapshotFromClient {
 
    /**
    * Test snapshotting a table that is online without flushing
-   * @throws Exception
    */
   @Test
   public void testSkipFlushTableSnapshot() throws Exception {
@@ -209,7 +208,6 @@ public class TestFlushSnapshotFromClient {
 
   /**
    * Test simple flush snapshotting a table that is online
-   * @throws Exception
    */
   @Test
   public void testFlushTableSnapshotWithProcedure() throws Exception {
@@ -408,13 +406,20 @@ public class TestFlushSnapshotFromClient {
   /**
    * Demonstrate that we reject snapshot requests if there is a snapshot already running on the
    * same table currently running and that concurrent snapshots on different tables can both
-   * succeed concurretly.
+   * succeed concurrently.
    */
+  @Ignore // Turning this test off. It is written flakey (See original submission in HBASE-7536
+  // for admission). While the test ensures we run one snapshot at a time as the above comment
+  // describes, the second part of the comment where we are supposed to allow snapshots against
+  // different tables run concurrently is where there is issue. There is only one
+  // handler thread on the regionserver-side which means that on a second submission, the
+  // second request gets rejected with RejectedExecutionException. The test used to sort-of
+  // pass with 20 attempts but as often-as-not, fails.
   @Test
   public void testConcurrentSnapshottingAttempts() throws IOException, InterruptedException {
     final TableName TABLE2_NAME = TableName.valueOf(TABLE_NAME + "2");
 
-    int ssNum = 20;
+    int ssNum = 2;
     // make sure we don't fail on listing snapshots
     SnapshotTestingUtils.assertNoSnapshots(admin);
     // create second testing table
@@ -451,7 +456,7 @@ public class TestFlushSnapshotFromClient {
     // build descriptions
     SnapshotDescription[] descs = new SnapshotDescription[ssNum];
     for (int i = 0; i < ssNum; i++) {
-      if(i % 2 ==0) {
+      if (i % 2 == 0) {
         descs[i] = new SnapshotDescription("ss" + i, TABLE_NAME, SnapshotType.FLUSH);
       } else {
         descs[i] = new SnapshotDescription("ss" + i, TABLE2_NAME, SnapshotType.FLUSH);