From 1765e08b98f6823e473201fe2ea64866ecb2752e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Tue, 6 Feb 2024 18:54:15 +0100 Subject: [PATCH] chore: remove drop protection before cleanup (#2863) The cleanup function used to remove old test databases did not remove the drop protection flag before trying to drop a database. --- .../spanner/testing/RemoteSpannerHelper.java | 67 ++++++++--- .../cloud/spanner/IntegrationTestEnv.java | 34 +++--- .../cloud/spanner/it/ITDatabaseAdminTest.java | 68 +++--------- .../cloud/spanner/it/ITDatabaseTest.java | 104 +++++++----------- 4 files changed, 126 insertions(+), 147 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java index 4b9b82240c4..e2001364abb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java @@ -16,12 +16,15 @@ package com.google.cloud.spanner.testing; +import com.google.api.client.util.BackOff; +import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.spanner.BatchClient; import com.google.cloud.spanner.Database; import com.google.cloud.spanner.DatabaseClient; import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.Dialect; +import com.google.cloud.spanner.ErrorCode; import com.google.cloud.spanner.InstanceId; import com.google.cloud.spanner.Spanner; import com.google.cloud.spanner.SpannerException; @@ -34,6 +37,7 @@ import java.util.Collections; import java.util.List; import java.util.Random; +import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; import java.util.logging.Logger; @@ -54,7 +58,7 @@ public class RemoteSpannerHelper { private static int dbRolePrefix = new Random().nextInt(Integer.MAX_VALUE); private static final AtomicInteger backupSeq = new AtomicInteger(); private static final int backupPrefix = new Random().nextInt(Integer.MAX_VALUE); - private final List dbs = new ArrayList<>(); + private final List databaseIds = new ArrayList<>(); protected RemoteSpannerHelper(SpannerOptions options, InstanceId instanceId, Spanner client) { this.options = options; @@ -132,19 +136,47 @@ public String getUniqueBackupId() { public Database createTestDatabase(Dialect dialect, Iterable statements) throws SpannerException { String dbId = getUniqueDatabaseId(); + DatabaseId databaseId = DatabaseId.of(instanceId.getProject(), instanceId.getInstance(), dbId); Database databaseToCreate = - client - .getDatabaseAdminClient() - .newDatabaseBuilder( - DatabaseId.of(instanceId.getProject(), instanceId.getInstance(), dbId)) - .setDialect(dialect) - .build(); + client.getDatabaseAdminClient().newDatabaseBuilder(databaseId).setDialect(dialect).build(); try { Iterable ddlStatements = dialect == Dialect.POSTGRESQL ? Collections.emptyList() : statements; - OperationFuture op = - client.getDatabaseAdminClient().createDatabase(databaseToCreate, ddlStatements); - Database db = op.get(); + Database db = null; + final int maxAttempts = 20; + BackOff backOff = + new ExponentialBackOff.Builder() + .setInitialIntervalMillis(10_000) + .setMaxIntervalMillis(60_000) + .setMaxElapsedTimeMillis(120_000) + .build(); + for (int attempts = 0; attempts < maxAttempts; attempts++) { + try { + OperationFuture op = + client.getDatabaseAdminClient().createDatabase(databaseToCreate, ddlStatements); + db = op.get(); + break; + } catch (ExecutionException executionException) { + SpannerException spannerException = + SpannerExceptionFactory.asSpannerException(executionException.getCause()); + if (spannerException.getErrorCode() != ErrorCode.RESOURCE_EXHAUSTED) { + throw executionException; + } + } catch (SpannerException spannerException) { + if (spannerException.getErrorCode() != ErrorCode.RESOURCE_EXHAUSTED) { + throw spannerException; + } + } + long sleep = backOff.nextBackOffMillis(); + if (sleep > 0L) { + Thread.sleep(sleep); + } + } + if (db == null) { + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.RESOURCE_EXHAUSTED, + String.format("Failed to create test database after %d attempts", maxAttempts)); + } if (dialect == Dialect.POSTGRESQL && Iterables.size(statements) > 0) { client .getDatabaseAdminClient() @@ -152,10 +184,11 @@ public Database createTestDatabase(Dialect dialect, Iterable statements) .get(); } logger.log(Level.FINE, "Created test database {0}", db.getId()); - dbs.add(db); return db; } catch (Exception e) { throw SpannerExceptionFactory.newSpannerException(e); + } finally { + databaseIds.add(databaseId); } } @@ -167,13 +200,15 @@ public Database createTestDatabase(Iterable statements) throws SpannerEx public void cleanUp() { // Drop all the databases we created explicitly. int numDropped = 0; - for (Database db : dbs) { + for (DatabaseId databaseId : databaseIds) { try { - logger.log(Level.INFO, "Dropping test database {0}", db.getId()); - db.drop(); + logger.log(Level.INFO, "Dropping test database {0}", databaseId); + client + .getDatabaseAdminClient() + .dropDatabase(databaseId.getInstanceId().getInstance(), databaseId.getDatabase()); ++numDropped; - } catch (SpannerException e) { - logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e); + } catch (Throwable e) { + logger.log(Level.SEVERE, "Failed to drop test database " + databaseId, e); } } logger.log(Level.INFO, "Dropped {0} test database(s)", numDropped); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java index 453535ee098..4d4f639d3d9 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java @@ -20,8 +20,8 @@ import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.longrunning.OperationFuture; -import com.google.api.gax.paging.Page; import com.google.cloud.Timestamp; +import com.google.cloud.spanner.DatabaseInfo.DatabaseField; import com.google.cloud.spanner.testing.EmulatorSpannerHelper; import com.google.cloud.spanner.testing.RemoteSpannerHelper; import com.google.common.collect.Iterators; @@ -213,27 +213,29 @@ private void cleanUpOldDatabases(InstanceId instanceId) { long OLD_DB_THRESHOLD_SECS = TimeUnit.SECONDS.convert(6L, TimeUnit.HOURS); Timestamp currentTimestamp = Timestamp.now(); int numDropped = 0; - Page page = databaseAdminClient.listDatabases(instanceId.getInstance()); String TEST_DB_REGEX = "(testdb_(.*)_(.*))|(mysample-(.*))"; logger.log(Level.INFO, "Dropping old test databases from {0}", instanceId.getName()); - while (page != null) { - for (Database db : page.iterateAll()) { - try { - long timeDiff = currentTimestamp.getSeconds() - db.getCreateTime().getSeconds(); - // Delete all databases which are more than OLD_DB_THRESHOLD_SECS seconds - // old. - if ((db.getId().getDatabase().matches(TEST_DB_REGEX)) - && (timeDiff > OLD_DB_THRESHOLD_SECS)) { - logger.log(Level.INFO, "Dropping test database {0}", db.getId()); - db.drop(); - ++numDropped; + for (Database db : databaseAdminClient.listDatabases(instanceId.getInstance()).iterateAll()) { + try { + long timeDiff = currentTimestamp.getSeconds() - db.getCreateTime().getSeconds(); + // Delete all databases which are more than OLD_DB_THRESHOLD_SECS seconds old. + if ((db.getId().getDatabase().matches(TEST_DB_REGEX)) + && (timeDiff > OLD_DB_THRESHOLD_SECS)) { + logger.log(Level.INFO, "Dropping test database {0}", db.getId()); + if (db.isDropProtectionEnabled()) { + Database updatedDatabase = + databaseAdminClient.newDatabaseBuilder(db.getId()).disableDropProtection().build(); + databaseAdminClient + .updateDatabase(updatedDatabase, DatabaseField.DROP_PROTECTION) + .get(); } - } catch (SpannerException e) { - logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e); + db.drop(); + ++numDropped; } + } catch (SpannerException | ExecutionException | InterruptedException e) { + logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e); } - page = page.getNextPage(); } logger.log(Level.INFO, "Dropped {0} test database(s)", numDropped); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java index 6bb02a7a61f..b606d1faf58 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseAdminTest.java @@ -40,7 +40,6 @@ import com.google.cloud.spanner.testing.RemoteSpannerHelper; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import com.google.spanner.admin.database.v1.UpdateDatabaseMetadata; import java.util.ArrayList; @@ -50,7 +49,6 @@ import java.util.concurrent.TimeUnit; import java.util.logging.Level; import java.util.logging.Logger; -import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; @@ -68,7 +66,6 @@ public class ITDatabaseAdminTest { private static final Logger logger = Logger.getLogger(ITDatabaseAdminTest.class.getName()); private DatabaseAdminClient dbAdminClient; private RemoteSpannerHelper testHelper; - private List dbs = new ArrayList<>(); @Before public void setUp() { @@ -76,25 +73,13 @@ public void setUp() { dbAdminClient = testHelper.getClient().getDatabaseAdminClient(); } - @After - public void tearDown() { - for (Database db : dbs) { - db.drop(); - } - dbs.clear(); - } - @Test public void testDatabaseOperations() throws Exception { - final String databaseId = testHelper.getUniqueDatabaseId(); final String instanceId = testHelper.getInstanceId().getInstance(); final String createTableT = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)"; - final Database createdDatabase = - dbAdminClient - .createDatabase(instanceId, databaseId, ImmutableList.of(createTableT)) - .get(5, TimeUnit.MINUTES); - dbs.add(createdDatabase); + final Database createdDatabase = testHelper.createTestDatabase(createTableT); + final String databaseId = createdDatabase.getId().getDatabase(); assertEquals(databaseId, createdDatabase.getId().getDatabase()); assertEquals(Dialect.GOOGLE_STANDARD_SQL, createdDatabase.getDialect()); @@ -124,7 +109,6 @@ public void testDatabaseOperations() throws Exception { assertEquals(databaseDdl, ImmutableList.of(createTableT, createTableT2)); dbAdminClient.dropDatabase(instanceId, databaseId); - dbs.clear(); try { dbAdminClient.getDatabase(instanceId, databaseId); @@ -136,13 +120,11 @@ public void testDatabaseOperations() throws Exception { @Test public void updateDdlRetry() throws Exception { - String dbId = testHelper.getUniqueDatabaseId(); String instanceId = testHelper.getInstanceId().getInstance(); String statement1 = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)"; - OperationFuture op = - dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1)); - Database db = op.get(TIMEOUT_MINUTES, TimeUnit.MINUTES); - dbs.add(db); + Database db = testHelper.createTestDatabase(statement1); + String dbId = db.getId().getDatabase(); + String statement2 = "CREATE TABLE T2 (\n" + " K2 STRING(MAX),\n" + ") PRIMARY KEY(K2)"; OperationFuture op1 = dbAdminClient.updateDatabaseDdl(instanceId, dbId, ImmutableList.of(statement2), "myop"); @@ -163,13 +145,9 @@ public void updateDdlRetry() throws Exception { @Test public void databaseOperationsViaEntity() throws Exception { - String dbId = testHelper.getUniqueDatabaseId(); - String instanceId = testHelper.getInstanceId().getInstance(); String statement1 = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)"; - OperationFuture op = - dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1)); - Database db = op.get(TIMEOUT_MINUTES, TimeUnit.MINUTES); - dbs.add(db); + Database db = testHelper.createTestDatabase(statement1); + String dbId = db.getId().getDatabase(); assertThat(db.getId().getDatabase()).isEqualTo(dbId); db = db.reload(); @@ -181,7 +159,6 @@ public void databaseOperationsViaEntity() throws Exception { Iterable statementsInDb = db.getDdl(); assertThat(statementsInDb).containsExactly(statement1, statement2); db.drop(); - dbs.clear(); try { db.reload(); fail("Expected exception"); @@ -191,16 +168,11 @@ public void databaseOperationsViaEntity() throws Exception { } @Test - public void listPagination() throws Exception { - List dbIds = - ImmutableList.of( - testHelper.getUniqueDatabaseId(), - testHelper.getUniqueDatabaseId(), - testHelper.getUniqueDatabaseId()); - + public void listPagination() { String instanceId = testHelper.getInstanceId().getInstance(); - for (String dbId : dbIds) { - dbs.add(dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of()).get()); + List dbIds = new ArrayList<>(3); + for (int n = 0; n < 3; n++) { + dbIds.add(testHelper.createTestDatabase().getId().getDatabase()); } Page page = dbAdminClient.listDatabases(instanceId, Options.pageSize(1)); List dbIdsGot = new ArrayList<>(); @@ -228,10 +200,7 @@ public void createAndListDatabaseRoles() throws Exception { testHelper.getUniqueDatabaseRole()); String instanceId = testHelper.getInstanceId().getInstance(); - Database database = - dbAdminClient - .createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of()) - .get(); + Database database = testHelper.createTestDatabase(); // Create the roles in Db. List dbRolesCreateStatements = new ArrayList<>(); @@ -280,13 +249,9 @@ public void createAndListDatabaseRoles() throws Exception { } @Test - public void updateDatabaseInvalidFieldsToUpdate() throws Exception { + public void updateDatabaseInvalidFieldsToUpdate() { assumeFalse("Emulator does not drop database protection", isUsingEmulator()); - String instanceId = testHelper.getInstanceId().getInstance(); - Database database = - dbAdminClient - .createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of()) - .get(); + Database database = testHelper.createTestDatabase(); logger.log(Level.INFO, "Created database: {0}", database.getId().getName()); Database databaseToUpdate = @@ -304,10 +269,7 @@ public void updateDatabaseInvalidFieldsToUpdate() throws Exception { public void dropDatabaseWithProtectionEnabled() throws Exception { assumeFalse("Emulator does not drop database protection", isUsingEmulator()); String instanceId = testHelper.getInstanceId().getInstance(); - Database database = - dbAdminClient - .createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of()) - .get(); + Database database = testHelper.createTestDatabase(); logger.log(Level.INFO, "Created database: {0}", database.getId().getName()); // Enable drop protection for the database. diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseTest.java index 006d3e2c8a8..ab45eb67179 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITDatabaseTest.java @@ -19,14 +19,12 @@ import static com.google.cloud.spanner.testing.EmulatorSpannerHelper.isUsingEmulator; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; import static org.junit.Assume.assumeFalse; import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.spanner.Database; -import com.google.cloud.spanner.DatabaseAdminClient; import com.google.cloud.spanner.DatabaseClient; import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.DatabaseNotFoundException; @@ -46,9 +44,6 @@ import java.math.BigDecimal; import java.util.ArrayList; import java.util.Collections; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import javax.annotation.Nullable; import org.junit.ClassRule; import org.junit.Test; @@ -156,69 +151,54 @@ public void instanceNotFound() { } @Test - public void testNumericPrimaryKey() - throws InterruptedException, ExecutionException, TimeoutException { + public void testNumericPrimaryKey() { assumeFalse("Emulator does not support numeric primary keys", isUsingEmulator()); - final String projectId = env.getTestHelper().getInstanceId().getProject(); - final String instanceId = env.getTestHelper().getInstanceId().getInstance(); - final String databaseId = env.getTestHelper().getUniqueDatabaseId(); final String table = "NumericTable"; - final DatabaseId id = DatabaseId.of(projectId, instanceId, databaseId); - final DatabaseAdminClient databaseAdminClient = - env.getTestHelper().getClient().getDatabaseAdminClient(); - try { - // Creates table with numeric primary key - final OperationFuture operation = - databaseAdminClient.createDatabase( - instanceId, - databaseId, - Collections.singletonList( - "CREATE TABLE " + table + " (" + "Id NUMERIC NOT NULL" + ") PRIMARY KEY (Id)")); - final Database database = operation.get(10, TimeUnit.MINUTES); - assertNotNull(database); - - // Writes data into the table - final DatabaseClient databaseClient = env.getTestHelper().getClient().getDatabaseClient(id); - final ArrayList mutations = new ArrayList<>(); - for (int i = 0; i < 5; i++) { - mutations.add( - Mutation.newInsertBuilder(table).set("Id").to(new BigDecimal(i + "")).build()); - } - databaseClient.write(mutations); - - // Reads the data to verify the writes - try (final ResultSet resultSet = - databaseClient.singleUse().read(table, KeySet.all(), Collections.singletonList("Id"))) { - for (int i = 0; resultSet.next(); i++) { - assertEquals(new BigDecimal(i + ""), resultSet.getBigDecimal("Id")); - } - } + // Creates table with numeric primary key + Database database = + env.getTestHelper() + .createTestDatabase( + "CREATE TABLE " + table + " (" + "Id NUMERIC NOT NULL" + ") PRIMARY KEY (Id)"); + + // Writes data into the table + final DatabaseClient databaseClient = + env.getTestHelper().getClient().getDatabaseClient(database.getId()); + final ArrayList mutations = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + mutations.add(Mutation.newInsertBuilder(table).set("Id").to(new BigDecimal(i + "")).build()); + } + databaseClient.write(mutations); - // Deletes data from the table, leaving only the Id = 0 row - databaseClient - .readWriteTransaction() - .run( - new TransactionCallable() { - @Nullable - @Override - public Object run(TransactionContext transaction) throws Exception { - transaction.executeUpdate(Statement.of("DELETE FROM " + table + " WHERE Id > 0")); - return null; - } - }); - - // Reads the data to verify the deletes only left a single row left - try (final ResultSet resultSet = - databaseClient - .singleUse() - .executeQuery(Statement.of("SELECT COUNT(1) as cnt FROM " + table))) { - resultSet.next(); - assertEquals(1L, resultSet.getLong("cnt")); + // Reads the data to verify the writes + try (final ResultSet resultSet = + databaseClient.singleUse().read(table, KeySet.all(), Collections.singletonList("Id"))) { + for (int i = 0; resultSet.next(); i++) { + assertEquals(new BigDecimal(i + ""), resultSet.getBigDecimal("Id")); } - } finally { - databaseAdminClient.dropDatabase(instanceId, databaseId); + } + + // Deletes data from the table, leaving only the Id = 0 row + databaseClient + .readWriteTransaction() + .run( + new TransactionCallable() { + @Nullable + @Override + public Object run(TransactionContext transaction) throws Exception { + transaction.executeUpdate(Statement.of("DELETE FROM " + table + " WHERE Id > 0")); + return null; + } + }); + + // Reads the data to verify the deletes only left a single row left + try (final ResultSet resultSet = + databaseClient + .singleUse() + .executeQuery(Statement.of("SELECT COUNT(1) as cnt FROM " + table))) { + resultSet.next(); + assertEquals(1L, resultSet.getLong("cnt")); } } }