Skip to content

Commit

Permalink
test: re-introduce parallel test execution (#1503)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@thiagotnunes
thiagotnunes committed Oct 19, 2021
1 parent d5a37b8 commit 8bc9293
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 15 deletions.
19 changes: 16 additions & 3 deletions google-cloud-spanner/pom.xml
View file
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
28 changes: 16 additions & 12 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java
View file
Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);

Expand All @@ -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
Expand All @@ -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);

Expand All @@ -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();
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 8bc9293

Please sign in to comment.