test: re-introduce parallel test execution (#1503)

* test: add timeout to it database admin test ops

Adds timeout to admin operations in the ITDatabaseAdminTest (database,
backup and restore ops)

* test: decrease it test timeout

We tried increasing the it test timeout to see if that could help with
the intermittent failures. It did not help, so we are rolling it back to
the original value.

* test: re-introduce parallel it test execution

We tried disabling parallel it test execution to see if it would help
with the intermittent failures we were seeing. This did not decrease the
failures, so we are rolling this back.

Author: thiagotnunes

google-cloud-spanner/pom.xml

@@ -74,14 +74,27 @@
             <spanner.gce.config.project_id>gcloud-devel</spanner.gce.config.project_id>
             <spanner.testenv.kms_key.name>projects/gcloud-devel/locations/us-central1/keyRings/spanner-test-keyring/cryptoKeys/spanner-test-key</spanner.testenv.kms_key.name>
           </systemPropertyVariables>
-          <forkedProcessTimeoutInSeconds>6000</forkedProcessTimeoutInSeconds>
+          <forkedProcessTimeoutInSeconds>3000</forkedProcessTimeoutInSeconds>
         </configuration>
         <executions>
           <!-- Executes serial integration tests -->
           <execution>
             <id>default</id>
             <configuration>
-              <groups>com.google.cloud.spanner.SerialIntegrationTest, com.google.cloud.spanner.ParallelIntegrationTest</groups>
+              <groups>com.google.cloud.spanner.SerialIntegrationTest</groups>
+            </configuration>
+          </execution>
+          <!-- Executes parallel integration tests -->
+          <execution>
+            <id>parallel-integration-test</id>
+            <goals>
+              <goal>integration-test</goal>
+            </goals>
+            <configuration>
+              <groups>com.google.cloud.spanner.ParallelIntegrationTest</groups>
+              <forkCount>8</forkCount>
+              <reuseForks>true</reuseForks>
+              <groups>com.google.cloud.spanner.ParallelIntegrationTest</groups>
             </configuration>
           </execution>
         </executions>
@@ -415,7 +428,7 @@
                 <spanner.attempt_directpath>true</spanner.attempt_directpath>
                 <spanner.directpath_test_scenario>ipv4</spanner.directpath_test_scenario>
               </systemPropertyVariables>
-              <forkedProcessTimeoutInSeconds>6000</forkedProcessTimeoutInSeconds>
+              <forkedProcessTimeoutInSeconds>3000</forkedProcessTimeoutInSeconds>
             </configuration>
           </plugin>
         </plugins>

google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java

@@ -72,6 +72,9 @@
 @Category(ParallelIntegrationTest.class)
 @RunWith(JUnit4.class)
 public class ITDatabaseAdminTest {
+  private static final long DATABASE_TIMEOUT_MINUTES = 5;
+  private static final long BACKUP_TIMEOUT_MINUTES = 20;
+  private static final long RESTORE_TIMEOUT_MINUTES = 10;
   @ClassRule public static IntegrationTestEnv env = new IntegrationTestEnv();
   private DatabaseAdminClient dbAdminClient;
   private RemoteSpannerHelper testHelper;
@@ -98,7 +101,7 @@ public void databaseOperations() throws Exception {
     String statement1 = "CREATE TABLE T (\n" + "  K STRING(MAX),\n" + ") PRIMARY KEY(K)";
     OperationFuture<Database, CreateDatabaseMetadata> op =
         dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1));
-    Database db = op.get();
+    Database db = op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
     dbs.add(db);
     assertThat(db.getId().getDatabase()).isEqualTo(dbId);
 
@@ -119,7 +122,7 @@ public void databaseOperations() throws Exception {
     String statement2 = "CREATE TABLE T2 (\n" + "  K2 STRING(MAX),\n" + ") PRIMARY KEY(K2)";
     OperationFuture<?, ?> op2 =
         dbAdminClient.updateDatabaseDdl(instanceId, dbId, ImmutableList.of(statement2), null);
-    op2.get();
+    op2.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
     List<String> statementsInDb = dbAdminClient.getDatabaseDdl(instanceId, dbId);
     assertThat(statementsInDb).containsExactly(statement1, statement2);
 
@@ -140,15 +143,15 @@ public void updateDdlRetry() throws Exception {
     String statement1 = "CREATE TABLE T (\n" + "  K STRING(MAX),\n" + ") PRIMARY KEY(K)";
     OperationFuture<Database, CreateDatabaseMetadata> op =
         dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1));
-    Database db = op.get();
+    Database db = op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
     dbs.add(db);
     String statement2 = "CREATE TABLE T2 (\n" + "  K2 STRING(MAX),\n" + ") PRIMARY KEY(K2)";
     OperationFuture<Void, UpdateDatabaseDdlMetadata> op1 =
         dbAdminClient.updateDatabaseDdl(instanceId, dbId, ImmutableList.of(statement2), "myop");
     OperationFuture<Void, UpdateDatabaseDdlMetadata> op2 =
         dbAdminClient.updateDatabaseDdl(instanceId, dbId, ImmutableList.of(statement2), "myop");
-    op1.get();
-    op2.get();
+    op1.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
+    op2.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
 
     // Remove the progress list from the metadata before comparing, as there could be small
     // differences between the two in the reported progress depending on exactly when each
@@ -167,7 +170,7 @@ public void databaseOperationsViaEntity() throws Exception {
     String statement1 = "CREATE TABLE T (\n" + "  K STRING(MAX),\n" + ") PRIMARY KEY(K)";
     OperationFuture<Database, CreateDatabaseMetadata> op =
         dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1));
-    Database db = op.get();
+    Database db = op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
     dbs.add(db);
     assertThat(db.getId().getDatabase()).isEqualTo(dbId);
 
@@ -176,7 +179,7 @@ public void databaseOperationsViaEntity() throws Exception {
 
     String statement2 = "CREATE TABLE T2 (\n" + "  K2 STRING(MAX),\n" + ") PRIMARY KEY(K2)";
     OperationFuture<?, ?> op2 = db.updateDdl(ImmutableList.of(statement2), null);
-    op2.get();
+    op2.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES);
     Iterable<String> statementsInDb = db.getDdl();
     assertThat(statementsInDb).containsExactly(statement1, statement2);
     db.drop();
@@ -308,12 +311,12 @@ public void testRetryNonIdempotentRpcsReturningLongRunningOperations() throws Ex
                 testHelper.getInstanceId().getInstance(),
                 initialDatabaseId,
                 Collections.emptyList());
-        databases.add(op.get());
+        databases.add(op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES));
         // Keep track of the original create time of this database, as we will drop this database
         // later and create another one with the exact same name. That means that the ListOperations
         // call will return at least two CreateDatabase operations. The retry logic should always
         // pick the last one.
-        initialDbCreateTime = op.get().getCreateTime();
+        initialDbCreateTime = op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES).getCreateTime();
         // Assert that the CreateDatabase RPC was called only once, and that the operation tracking
         // was resumed through a GetOperation call.
         assertThat(createDbInterceptor.methodCount.get()).isEqualTo(1);
@@ -340,7 +343,7 @@ public void testRetryNonIdempotentRpcsReturningLongRunningOperations() throws Ex
                 databaseId,
                 Timestamp.ofTimeSecondsAndNanos(
                     Timestamp.now().getSeconds() + TimeUnit.SECONDS.convert(7L, TimeUnit.DAYS), 0));
-        backups.add(op.get());
+        backups.add(op.get(BACKUP_TIMEOUT_MINUTES, TimeUnit.MINUTES));
         // Assert that the CreateBackup RPC was called only once, and that the operation tracking
         // was resumed through a GetOperation call.
         assertThat(createDbInterceptor.methodCount.get()).isEqualTo(1);
@@ -368,7 +371,7 @@ public void testRetryNonIdempotentRpcsReturningLongRunningOperations() throws Ex
                   backupId,
                   testHelper.getInstanceId().getInstance(),
                   restoredDbId);
-          databases.add(op.get());
+          databases.add(op.get(RESTORE_TIMEOUT_MINUTES, TimeUnit.MINUTES));
           // Assert that the RestoreDatabase RPC was called only once, and that the operation
           // tracking was resumed through a GetOperation call.
           assertThat(createDbInterceptor.methodCount.get()).isEqualTo(1);
@@ -409,7 +412,8 @@ public void testRetryNonIdempotentRpcsReturningLongRunningOperations() throws Ex
                 Collections.emptyList());
         // Check that the second database was created and has a greater creation time than the
         // first.
-        Timestamp secondCreationTime = op.get().getCreateTime();
+        Timestamp secondCreationTime =
+            op.get(DATABASE_TIMEOUT_MINUTES, TimeUnit.MINUTES).getCreateTime();
         // TODO: Change this to greaterThan when the create time of a database is reported back by
         // the server.
         assertThat(secondCreationTime).isAtLeast(initialDbCreateTime);