HBASE-22882 TestFlushSnapshotFromClient#testConcurrentSnapshottingAttempts is flakey (was written flakey)

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) {

hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestFlushSnapshotFromClient.java

@@ -54,6 +54,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;
@@ -142,7 +143,6 @@ public class TestFlushSnapshotFromClient {
 
   /**
    * Test simple flush snapshotting a table that is online
-   * @throws Exception
    */
   @Test
   public void testFlushTableSnapshot() throws Exception {
@@ -175,7 +175,6 @@ public class TestFlushSnapshotFromClient {
 
    /**
    * Test snapshotting a table that is online without flushing
-   * @throws Exception
    */
   @Test
   public void testSkipFlushTableSnapshot() throws Exception {
@@ -215,7 +214,6 @@ public class TestFlushSnapshotFromClient {
 
   /**
    * Test simple flush snapshotting a table that is online
-   * @throws Exception
    */
   @Test
   public void testFlushTableSnapshotWithProcedure() throws Exception {
@@ -446,13 +444,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
@@ -489,7 +494,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);