Skip to content

Commit 1765e08

Browse files
olavloiteolavloite
authored
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.
1 parent 968877e commit 1765e08

File tree

4 files changed

+126
-147
lines changed

4 files changed

+126
-147
lines changed

‎google-cloud-spanner/src/main/java/com/google/cloud/spanner/testing/RemoteSpannerHelper.java

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616

1717
package com.google.cloud.spanner.testing;
1818

19+
import com.google.api.client.util.BackOff;
20+
import com.google.api.client.util.ExponentialBackOff;
1921
import com.google.api.gax.longrunning.OperationFuture;
2022
import com.google.cloud.spanner.BatchClient;
2123
import com.google.cloud.spanner.Database;
2224
import com.google.cloud.spanner.DatabaseClient;
2325
import com.google.cloud.spanner.DatabaseId;
2426
import com.google.cloud.spanner.Dialect;
27+
import com.google.cloud.spanner.ErrorCode;
2528
import com.google.cloud.spanner.InstanceId;
2629
import com.google.cloud.spanner.Spanner;
2730
import com.google.cloud.spanner.SpannerException;
@@ -34,6 +37,7 @@
3437
import java.util.Collections;
3538
import java.util.List;
3639
import java.util.Random;
40+
import java.util.concurrent.ExecutionException;
3741
import java.util.concurrent.atomic.AtomicInteger;
3842
import java.util.logging.Level;
3943
import java.util.logging.Logger;
@@ -54,7 +58,7 @@ public class RemoteSpannerHelper {
5458
private static int dbRolePrefix = new Random().nextInt(Integer.MAX_VALUE);
5559
private static final AtomicInteger backupSeq = new AtomicInteger();
5660
private static final int backupPrefix = new Random().nextInt(Integer.MAX_VALUE);
57-
private final List<Database> dbs = new ArrayList<>();
61+
private final List<DatabaseId> databaseIds = new ArrayList<>();
5862

5963
protected RemoteSpannerHelper(SpannerOptions options, InstanceId instanceId, Spanner client) {
6064
this.options = options;
@@ -132,30 +136,59 @@ public String getUniqueBackupId() {
132136
public Database createTestDatabase(Dialect dialect, Iterable<String> statements)
133137
throws SpannerException {
134138
String dbId = getUniqueDatabaseId();
139+
DatabaseId databaseId = DatabaseId.of(instanceId.getProject(), instanceId.getInstance(), dbId);
135140
Database databaseToCreate =
136-
client
137-
.getDatabaseAdminClient()
138-
.newDatabaseBuilder(
139-
DatabaseId.of(instanceId.getProject(), instanceId.getInstance(), dbId))
140-
.setDialect(dialect)
141-
.build();
141+
client.getDatabaseAdminClient().newDatabaseBuilder(databaseId).setDialect(dialect).build();
142142
try {
143143
Iterable<String> ddlStatements =
144144
dialect == Dialect.POSTGRESQL ? Collections.emptyList() : statements;
145-
OperationFuture<Database, CreateDatabaseMetadata> op =
146-
client.getDatabaseAdminClient().createDatabase(databaseToCreate, ddlStatements);
147-
Database db = op.get();
145+
Database db = null;
146+
final int maxAttempts = 20;
147+
BackOff backOff =
148+
new ExponentialBackOff.Builder()
149+
.setInitialIntervalMillis(10_000)
150+
.setMaxIntervalMillis(60_000)
151+
.setMaxElapsedTimeMillis(120_000)
152+
.build();
153+
for (int attempts = 0; attempts < maxAttempts; attempts++) {
154+
try {
155+
OperationFuture<Database, CreateDatabaseMetadata> op =
156+
client.getDatabaseAdminClient().createDatabase(databaseToCreate, ddlStatements);
157+
db = op.get();
158+
break;
159+
} catch (ExecutionException executionException) {
160+
SpannerException spannerException =
161+
SpannerExceptionFactory.asSpannerException(executionException.getCause());
162+
if (spannerException.getErrorCode() != ErrorCode.RESOURCE_EXHAUSTED) {
163+
throw executionException;
164+
}
165+
} catch (SpannerException spannerException) {
166+
if (spannerException.getErrorCode() != ErrorCode.RESOURCE_EXHAUSTED) {
167+
throw spannerException;
168+
}
169+
}
170+
long sleep = backOff.nextBackOffMillis();
171+
if (sleep > 0L) {
172+
Thread.sleep(sleep);
173+
}
174+
}
175+
if (db == null) {
176+
throw SpannerExceptionFactory.newSpannerException(
177+
ErrorCode.RESOURCE_EXHAUSTED,
178+
String.format("Failed to create test database after %d attempts", maxAttempts));
179+
}
148180
if (dialect == Dialect.POSTGRESQL && Iterables.size(statements) > 0) {
149181
client
150182
.getDatabaseAdminClient()
151183
.updateDatabaseDdl(instanceId.getInstance(), dbId, statements, null)
152184
.get();
153185
}
154186
logger.log(Level.FINE, "Created test database {0}", db.getId());
155-
dbs.add(db);
156187
return db;
157188
} catch (Exception e) {
158189
throw SpannerExceptionFactory.newSpannerException(e);
190+
} finally {
191+
databaseIds.add(databaseId);
159192
}
160193
}
161194

@@ -167,13 +200,15 @@ public Database createTestDatabase(Iterable<String> statements) throws SpannerEx
167200
public void cleanUp() {
168201
// Drop all the databases we created explicitly.
169202
int numDropped = 0;
170-
for (Database db : dbs) {
203+
for (DatabaseId databaseId : databaseIds) {
171204
try {
172-
logger.log(Level.INFO, "Dropping test database {0}", db.getId());
173-
db.drop();
205+
logger.log(Level.INFO, "Dropping test database {0}", databaseId);
206+
client
207+
.getDatabaseAdminClient()
208+
.dropDatabase(databaseId.getInstanceId().getInstance(), databaseId.getDatabase());
174209
++numDropped;
175-
} catch (SpannerException e) {
176-
logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e);
210+
} catch (Throwable e) {
211+
logger.log(Level.SEVERE, "Failed to drop test database " + databaseId, e);
177212
}
178213
}
179214
logger.log(Level.INFO, "Dropped {0} test database(s)", numDropped);

‎google-cloud-spanner/src/test/java/com/google/cloud/spanner/IntegrationTestEnv.java

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020

2121
import com.google.api.client.util.ExponentialBackOff;
2222
import com.google.api.gax.longrunning.OperationFuture;
23-
import com.google.api.gax.paging.Page;
2423
import com.google.cloud.Timestamp;
24+
import com.google.cloud.spanner.DatabaseInfo.DatabaseField;
2525
import com.google.cloud.spanner.testing.EmulatorSpannerHelper;
2626
import com.google.cloud.spanner.testing.RemoteSpannerHelper;
2727
import com.google.common.collect.Iterators;
@@ -213,27 +213,29 @@ private void cleanUpOldDatabases(InstanceId instanceId) {
213213
long OLD_DB_THRESHOLD_SECS = TimeUnit.SECONDS.convert(6L, TimeUnit.HOURS);
214214
Timestamp currentTimestamp = Timestamp.now();
215215
int numDropped = 0;
216-
Page<Database> page = databaseAdminClient.listDatabases(instanceId.getInstance());
217216
String TEST_DB_REGEX = "(testdb_(.*)_(.*))|(mysample-(.*))";
218217

219218
logger.log(Level.INFO, "Dropping old test databases from {0}", instanceId.getName());
220-
while (page != null) {
221-
for (Database db : page.iterateAll()) {
222-
try {
223-
long timeDiff = currentTimestamp.getSeconds() - db.getCreateTime().getSeconds();
224-
// Delete all databases which are more than OLD_DB_THRESHOLD_SECS seconds
225-
// old.
226-
if ((db.getId().getDatabase().matches(TEST_DB_REGEX))
227-
&& (timeDiff > OLD_DB_THRESHOLD_SECS)) {
228-
logger.log(Level.INFO, "Dropping test database {0}", db.getId());
229-
db.drop();
230-
++numDropped;
219+
for (Database db : databaseAdminClient.listDatabases(instanceId.getInstance()).iterateAll()) {
220+
try {
221+
long timeDiff = currentTimestamp.getSeconds() - db.getCreateTime().getSeconds();
222+
// Delete all databases which are more than OLD_DB_THRESHOLD_SECS seconds old.
223+
if ((db.getId().getDatabase().matches(TEST_DB_REGEX))
224+
&& (timeDiff > OLD_DB_THRESHOLD_SECS)) {
225+
logger.log(Level.INFO, "Dropping test database {0}", db.getId());
226+
if (db.isDropProtectionEnabled()) {
227+
Database updatedDatabase =
228+
databaseAdminClient.newDatabaseBuilder(db.getId()).disableDropProtection().build();
229+
databaseAdminClient
230+
.updateDatabase(updatedDatabase, DatabaseField.DROP_PROTECTION)
231+
.get();
231232
}
232-
} catch (SpannerException e) {
233-
logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e);
233+
db.drop();
234+
++numDropped;
234235
}
236+
} catch (SpannerException | ExecutionException | InterruptedException e) {
237+
logger.log(Level.SEVERE, "Failed to drop test database " + db.getId(), e);
235238
}
236-
page = page.getNextPage();
237239
}
238240
logger.log(Level.INFO, "Dropped {0} test database(s)", numDropped);
239241
}

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

Lines changed: 15 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@
4040
import com.google.cloud.spanner.testing.RemoteSpannerHelper;
4141
import com.google.common.collect.ImmutableList;
4242
import com.google.common.collect.Iterables;
43-
import com.google.spanner.admin.database.v1.CreateDatabaseMetadata;
4443
import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata;
4544
import com.google.spanner.admin.database.v1.UpdateDatabaseMetadata;
4645
import java.util.ArrayList;
@@ -50,7 +49,6 @@
5049
import java.util.concurrent.TimeUnit;
5150
import java.util.logging.Level;
5251
import java.util.logging.Logger;
53-
import org.junit.After;
5452
import org.junit.Before;
5553
import org.junit.ClassRule;
5654
import org.junit.Test;
@@ -68,33 +66,20 @@ public class ITDatabaseAdminTest {
6866
private static final Logger logger = Logger.getLogger(ITDatabaseAdminTest.class.getName());
6967
private DatabaseAdminClient dbAdminClient;
7068
private RemoteSpannerHelper testHelper;
71-
private List<Database> dbs = new ArrayList<>();
7269

7370
@Before
7471
public void setUp() {
7572
testHelper = env.getTestHelper();
7673
dbAdminClient = testHelper.getClient().getDatabaseAdminClient();
7774
}
7875

79-
@After
80-
public void tearDown() {
81-
for (Database db : dbs) {
82-
db.drop();
83-
}
84-
dbs.clear();
85-
}
86-
8776
@Test
8877
public void testDatabaseOperations() throws Exception {
89-
final String databaseId = testHelper.getUniqueDatabaseId();
9078
final String instanceId = testHelper.getInstanceId().getInstance();
9179
final String createTableT = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)";
9280

93-
final Database createdDatabase =
94-
dbAdminClient
95-
.createDatabase(instanceId, databaseId, ImmutableList.of(createTableT))
96-
.get(5, TimeUnit.MINUTES);
97-
dbs.add(createdDatabase);
81+
final Database createdDatabase = testHelper.createTestDatabase(createTableT);
82+
final String databaseId = createdDatabase.getId().getDatabase();
9883

9984
assertEquals(databaseId, createdDatabase.getId().getDatabase());
10085
assertEquals(Dialect.GOOGLE_STANDARD_SQL, createdDatabase.getDialect());
@@ -124,7 +109,6 @@ public void testDatabaseOperations() throws Exception {
124109
assertEquals(databaseDdl, ImmutableList.of(createTableT, createTableT2));
125110

126111
dbAdminClient.dropDatabase(instanceId, databaseId);
127-
dbs.clear();
128112

129113
try {
130114
dbAdminClient.getDatabase(instanceId, databaseId);
@@ -136,13 +120,11 @@ public void testDatabaseOperations() throws Exception {
136120

137121
@Test
138122
public void updateDdlRetry() throws Exception {
139-
String dbId = testHelper.getUniqueDatabaseId();
140123
String instanceId = testHelper.getInstanceId().getInstance();
141124
String statement1 = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)";
142-
OperationFuture<Database, CreateDatabaseMetadata> op =
143-
dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1));
144-
Database db = op.get(TIMEOUT_MINUTES, TimeUnit.MINUTES);
145-
dbs.add(db);
125+
Database db = testHelper.createTestDatabase(statement1);
126+
String dbId = db.getId().getDatabase();
127+
146128
String statement2 = "CREATE TABLE T2 (\n" + " K2 STRING(MAX),\n" + ") PRIMARY KEY(K2)";
147129
OperationFuture<Void, UpdateDatabaseDdlMetadata> op1 =
148130
dbAdminClient.updateDatabaseDdl(instanceId, dbId, ImmutableList.of(statement2), "myop");
@@ -163,13 +145,9 @@ public void updateDdlRetry() throws Exception {
163145

164146
@Test
165147
public void databaseOperationsViaEntity() throws Exception {
166-
String dbId = testHelper.getUniqueDatabaseId();
167-
String instanceId = testHelper.getInstanceId().getInstance();
168148
String statement1 = "CREATE TABLE T (\n" + " K STRING(MAX),\n" + ") PRIMARY KEY(K)";
169-
OperationFuture<Database, CreateDatabaseMetadata> op =
170-
dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of(statement1));
171-
Database db = op.get(TIMEOUT_MINUTES, TimeUnit.MINUTES);
172-
dbs.add(db);
149+
Database db = testHelper.createTestDatabase(statement1);
150+
String dbId = db.getId().getDatabase();
173151
assertThat(db.getId().getDatabase()).isEqualTo(dbId);
174152

175153
db = db.reload();
@@ -181,7 +159,6 @@ public void databaseOperationsViaEntity() throws Exception {
181159
Iterable<String> statementsInDb = db.getDdl();
182160
assertThat(statementsInDb).containsExactly(statement1, statement2);
183161
db.drop();
184-
dbs.clear();
185162
try {
186163
db.reload();
187164
fail("Expected exception");
@@ -191,16 +168,11 @@ public void databaseOperationsViaEntity() throws Exception {
191168
}
192169

193170
@Test
194-
public void listPagination() throws Exception {
195-
List<String> dbIds =
196-
ImmutableList.of(
197-
testHelper.getUniqueDatabaseId(),
198-
testHelper.getUniqueDatabaseId(),
199-
testHelper.getUniqueDatabaseId());
200-
171+
public void listPagination() {
201172
String instanceId = testHelper.getInstanceId().getInstance();
202-
for (String dbId : dbIds) {
203-
dbs.add(dbAdminClient.createDatabase(instanceId, dbId, ImmutableList.of()).get());
173+
List<String> dbIds = new ArrayList<>(3);
174+
for (int n = 0; n < 3; n++) {
175+
dbIds.add(testHelper.createTestDatabase().getId().getDatabase());
204176
}
205177
Page<Database> page = dbAdminClient.listDatabases(instanceId, Options.pageSize(1));
206178
List<String> dbIdsGot = new ArrayList<>();
@@ -228,10 +200,7 @@ public void createAndListDatabaseRoles() throws Exception {
228200
testHelper.getUniqueDatabaseRole());
229201

230202
String instanceId = testHelper.getInstanceId().getInstance();
231-
Database database =
232-
dbAdminClient
233-
.createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of())
234-
.get();
203+
Database database = testHelper.createTestDatabase();
235204

236205
// Create the roles in Db.
237206
List<String> dbRolesCreateStatements = new ArrayList<>();
@@ -280,13 +249,9 @@ public void createAndListDatabaseRoles() throws Exception {
280249
}
281250

282251
@Test
283-
public void updateDatabaseInvalidFieldsToUpdate() throws Exception {
252+
public void updateDatabaseInvalidFieldsToUpdate() {
284253
assumeFalse("Emulator does not drop database protection", isUsingEmulator());
285-
String instanceId = testHelper.getInstanceId().getInstance();
286-
Database database =
287-
dbAdminClient
288-
.createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of())
289-
.get();
254+
Database database = testHelper.createTestDatabase();
290255
logger.log(Level.INFO, "Created database: {0}", database.getId().getName());
291256

292257
Database databaseToUpdate =
@@ -304,10 +269,7 @@ public void updateDatabaseInvalidFieldsToUpdate() throws Exception {
304269
public void dropDatabaseWithProtectionEnabled() throws Exception {
305270
assumeFalse("Emulator does not drop database protection", isUsingEmulator());
306271
String instanceId = testHelper.getInstanceId().getInstance();
307-
Database database =
308-
dbAdminClient
309-
.createDatabase(instanceId, testHelper.getUniqueDatabaseId(), ImmutableList.of())
310-
.get();
272+
Database database = testHelper.createTestDatabase();
311273
logger.log(Level.INFO, "Created database: {0}", database.getId().getName());
312274

313275
// Enable drop protection for the database.

0 commit comments

Comments
 (0)