From 7618ac8bc32f7d3482bd4a0850be2bce71c33fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 6 Feb 2020 05:45:54 +0100 Subject: [PATCH] fix: stop sending RPCs on InstanceNotFound (#61) The session pool should handle InstanceNotFound in the same way as DatabaseNotFound errors, i.e. stop sending RPCs and also consider an instance that has been re-created with the same name as a different instance, and hence require the user to re-create any database client before it can be used again. Fixes #60 --- .../spanner/InstanceNotFoundException.java | 39 ++ .../com/google/cloud/spanner/SessionPool.java | 41 +- .../spanner/SpannerExceptionFactory.java | 4 + .../cloud/spanner/DatabaseClientImplTest.java | 451 ++++++++++-------- .../spanner/SpannerExceptionFactoryTest.java | 12 + .../cloud/spanner/it/ITDatabaseTest.java | 19 + 6 files changed, 361 insertions(+), 205 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java new file mode 100644 index 0000000000..6c179ca9b6 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceNotFoundException.java @@ -0,0 +1,39 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.cloud.spanner; + +import com.google.cloud.spanner.SpannerException.ResourceNotFoundException; +import com.google.rpc.ResourceInfo; +import javax.annotation.Nullable; + +/** + * Exception thrown by Cloud Spanner when an operation detects that the instance that is being used + * no longer exists. This type of error has its own subclass as it is a condition that should cause + * the client library to stop trying to send RPCs to the backend until the user has taken action. + */ +public class InstanceNotFoundException extends ResourceNotFoundException { + private static final long serialVersionUID = 45297002L; + + /** Private constructor. Use {@link SpannerExceptionFactory} to create instances. */ + InstanceNotFoundException( + DoNotConstructDirectly token, + @Nullable String message, + ResourceInfo resourceInfo, + @Nullable Throwable cause) { + super(token, message, resourceInfo, cause); + } +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java index f19333f4e3..45112ff4b4 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPool.java @@ -26,6 +26,7 @@ import com.google.cloud.spanner.Options.QueryOption; import com.google.cloud.spanner.Options.ReadOption; import com.google.cloud.spanner.SessionClient.SessionConsumer; +import com.google.cloud.spanner.SpannerException.ResourceNotFoundException; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.MoreObjects; @@ -781,13 +782,14 @@ public void close() { if (lastException != null && isSessionNotFound(lastException)) { invalidateSession(this); } else { - if (lastException != null && isDatabaseNotFound(lastException)) { + if (lastException != null && isDatabaseOrInstanceNotFound(lastException)) { // Mark this session pool as no longer valid and then release the session into the pool as // there is nothing we can do with it anyways. synchronized (lock) { - SessionPool.this.databaseNotFound = + SessionPool.this.resourceNotFoundException = MoreObjects.firstNonNull( - SessionPool.this.databaseNotFound, (DatabaseNotFoundException) lastException); + SessionPool.this.resourceNotFoundException, + (ResourceNotFoundException) lastException); } } lastException = null; @@ -1075,7 +1077,7 @@ private static enum Position { private SettableFuture closureFuture; @GuardedBy("lock") - private DatabaseNotFoundException databaseNotFound; + private ResourceNotFoundException resourceNotFoundException; @GuardedBy("lock") private final LinkedList readSessions = new LinkedList<>(); @@ -1213,8 +1215,8 @@ private boolean isSessionNotFound(SpannerException e) { return e.getErrorCode() == ErrorCode.NOT_FOUND && e.getMessage().contains("Session not found"); } - private boolean isDatabaseNotFound(SpannerException e) { - return e instanceof DatabaseNotFoundException; + private boolean isDatabaseOrInstanceNotFound(SpannerException e) { + return e instanceof DatabaseNotFoundException || e instanceof InstanceNotFoundException; } private boolean isPermissionDenied(SpannerException e) { @@ -1249,7 +1251,7 @@ private PooledSession findSessionToKeepAlive( /** @return true if this {@link SessionPool} is still valid. */ boolean isValid() { synchronized (lock) { - return closureFuture == null && databaseNotFound == null; + return closureFuture == null && resourceNotFoundException == null; } } @@ -1279,14 +1281,14 @@ PooledSession getReadSession() throws SpannerException { span.addAnnotation("Pool has been closed"); throw new IllegalStateException("Pool has been closed"); } - if (databaseNotFound != null) { + if (resourceNotFoundException != null) { span.addAnnotation("Database has been deleted"); throw SpannerExceptionFactory.newSpannerException( ErrorCode.NOT_FOUND, String.format( "The session pool has been invalidated because a previous RPC returned 'Database not found': %s", - databaseNotFound.getMessage()), - databaseNotFound); + resourceNotFoundException.getMessage()), + resourceNotFoundException); } sess = readSessions.poll(); if (sess == null) { @@ -1344,14 +1346,14 @@ PooledSession getReadWriteSession() { span.addAnnotation("Pool has been closed"); throw new IllegalStateException("Pool has been closed"); } - if (databaseNotFound != null) { + if (resourceNotFoundException != null) { span.addAnnotation("Database has been deleted"); throw SpannerExceptionFactory.newSpannerException( ErrorCode.NOT_FOUND, String.format( "The session pool has been invalidated because a previous RPC returned 'Database not found': %s", - databaseNotFound.getMessage()), - databaseNotFound); + resourceNotFoundException.getMessage()), + resourceNotFoundException); } sess = writePreparedSessions.poll(); if (sess == null) { @@ -1495,9 +1497,10 @@ private void handleCreateSessionsFailure(SpannerException e, int count) { break; } } - this.databaseNotFound = + this.resourceNotFoundException = MoreObjects.firstNonNull( - this.databaseNotFound, isDatabaseNotFound(e) ? (DatabaseNotFoundException) e : null); + this.resourceNotFoundException, + isDatabaseOrInstanceNotFound(e) ? (ResourceNotFoundException) e : null); } } @@ -1505,7 +1508,7 @@ private void handlePrepareSessionFailure(SpannerException e, PooledSession sessi synchronized (lock) { if (isSessionNotFound(e)) { invalidateSession(session); - } else if (isDatabaseNotFound(e) || isPermissionDenied(e)) { + } else if (isDatabaseOrInstanceNotFound(e) || isPermissionDenied(e)) { // Database has been deleted or the user has no permission to write to this database. We // should stop trying to prepare any transactions. Also propagate the error to all waiters, // as any further waiting is pointless. @@ -1520,10 +1523,10 @@ private void handlePrepareSessionFailure(SpannerException e, PooledSession sessi if (isClosed()) { decrementPendingClosures(1); } - this.databaseNotFound = + this.resourceNotFoundException = MoreObjects.firstNonNull( - this.databaseNotFound, - isDatabaseNotFound(e) ? (DatabaseNotFoundException) e : null); + this.resourceNotFoundException, + isDatabaseOrInstanceNotFound(e) ? (ResourceNotFoundException) e : null); } else if (readWriteWaiters.size() > 0) { releaseSession(session, Position.FIRST); readWriteWaiters.poll().put(e); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java index 91f71cb000..e9ca06e233 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerExceptionFactory.java @@ -42,6 +42,8 @@ public final class SpannerExceptionFactory { static final String SESSION_RESOURCE_TYPE = "type.googleapis.com/google.spanner.v1.Session"; static final String DATABASE_RESOURCE_TYPE = "type.googleapis.com/google.spanner.admin.database.v1.Database"; + static final String INSTANCE_RESOURCE_TYPE = + "type.googleapis.com/google.spanner.admin.instance.v1.Instance"; private static final Metadata.Key KEY_RESOURCE_INFO = ProtoUtils.keyForProto(ResourceInfo.getDefaultInstance()); @@ -199,6 +201,8 @@ private static SpannerException newSpannerExceptionPreformatted( return new SessionNotFoundException(token, message, resourceInfo, cause); } else if (resourceInfo.getResourceType().equals(DATABASE_RESOURCE_TYPE)) { return new DatabaseNotFoundException(token, message, resourceInfo, cause); + } else if (resourceInfo.getResourceType().equals(INSTANCE_RESOURCE_TYPE)) { + return new InstanceNotFoundException(token, message, resourceInfo, cause); } } // Fall through to the default. diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index c21a87d645..43be8db1c0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -36,6 +36,7 @@ import com.google.spanner.v1.TypeCode; import io.grpc.Server; import io.grpc.Status; +import io.grpc.StatusRuntimeException; import io.grpc.inprocess.InProcessServerBuilder; import java.io.IOException; import java.util.concurrent.ScheduledThreadPoolExecutor; @@ -54,6 +55,8 @@ public class DatabaseClientImplTest { private static final String TEST_PROJECT = "my-project"; private static final String TEST_INSTANCE = "my-instance"; private static final String TEST_DATABASE = "my-database"; + private static final String INSTANCE_NAME = + String.format("projects/%s/instances/%s", TEST_PROJECT, TEST_INSTANCE); private static final String DATABASE_NAME = String.format( "projects/%s/instances/%s/databases/%s", TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE); @@ -268,130 +271,194 @@ public Long run(TransactionContext transaction) throws Exception { } @Test - public void testDatabaseDoesNotExistOnPrepareSession() throws Exception { - mockSpanner.setBeginTransactionExecutionTime( - SimulatedExecutionTime.ofStickyException( - SpannerExceptionFactoryTest.newStatusResourceNotFoundException( - "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, TEST_DATABASE))); - DatabaseClientImpl dbClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - // Wait until all sessions have been created. - Stopwatch watch = Stopwatch.createStarted(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { - Thread.sleep(1L); - } - // Ensure that no sessions could be prepared and that the session pool gives up trying to - // prepare sessions. - watch = watch.reset().start(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && dbClient.pool.getNumberOfSessionsBeingPrepared() > 0) { - Thread.sleep(1L); - } - assertThat(dbClient.pool.getNumberOfSessionsBeingPrepared(), is(equalTo(0))); - assertThat(dbClient.pool.getNumberOfAvailableWritePreparedSessions(), is(equalTo(0))); - int currentNumRequest = mockSpanner.getRequests().size(); - try { - dbClient - .readWriteTransaction() - .run( - new TransactionCallable() { - @Override - public Void run(TransactionContext transaction) throws Exception { - return null; - } - }); - fail("missing expected DatabaseNotFoundException"); - } catch (DatabaseNotFoundException e) { + public void testDatabaseOrInstanceDoesNotExistOnPrepareSession() throws Exception { + StatusRuntimeException[] exceptions = + new StatusRuntimeException[] { + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME), + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, INSTANCE_NAME) + }; + for (StatusRuntimeException exception : exceptions) { + try (Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId(TEST_PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .build() + .getService()) { + mockSpanner.setBeginTransactionExecutionTime( + SimulatedExecutionTime.ofStickyException(exception)); + DatabaseClientImpl dbClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Wait until all sessions have been created. + Stopwatch watch = Stopwatch.createStarted(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { + Thread.sleep(1L); + } + // Ensure that no sessions could be prepared and that the session pool gives up trying to + // prepare sessions. + watch = watch.reset().start(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && dbClient.pool.getNumberOfSessionsBeingPrepared() > 0) { + Thread.sleep(1L); + } + assertThat(dbClient.pool.getNumberOfSessionsBeingPrepared(), is(equalTo(0))); + assertThat(dbClient.pool.getNumberOfAvailableWritePreparedSessions(), is(equalTo(0))); + int currentNumRequest = mockSpanner.getRequests().size(); + try { + dbClient + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Void run(TransactionContext transaction) throws Exception { + return null; + } + }); + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + } + assertThat(mockSpanner.getRequests()).hasSize(currentNumRequest); + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + } } - assertThat(mockSpanner.getRequests()).hasSize(currentNumRequest); } @Test - public void testDatabaseDoesNotExistOnInitialization() throws Exception { - mockSpanner.setBatchCreateSessionsExecutionTime( - SimulatedExecutionTime.ofStickyException( - SpannerExceptionFactoryTest.newStatusResourceNotFoundException( - "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME))); - DatabaseClientImpl dbClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - // Wait until session creation has finished. - Stopwatch watch = Stopwatch.createStarted(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { - Thread.sleep(1L); + public void testDatabaseOrInstanceDoesNotExistOnInitialization() throws Exception { + StatusRuntimeException[] exceptions = + new StatusRuntimeException[] { + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME), + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, INSTANCE_NAME) + }; + for (StatusRuntimeException exception : exceptions) { + try (Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId(TEST_PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .build() + .getService()) { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.ofStickyException(exception)); + DatabaseClientImpl dbClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Wait until session creation has finished. + Stopwatch watch = Stopwatch.createStarted(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { + Thread.sleep(1L); + } + // All session creation should fail and stop trying. + assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); + assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + } } - // All session creation should fail and stop trying. - assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); - assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); } @Test - public void testDatabaseDoesNotExistOnCreate() throws Exception { - mockSpanner.setBatchCreateSessionsExecutionTime( - SimulatedExecutionTime.ofStickyException( - SpannerExceptionFactoryTest.newStatusResourceNotFoundException( - "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME))); - // Ensure there are no sessions in the pool by default. - try (Spanner spanner = - SpannerOptions.newBuilder() - .setProjectId(TEST_PROJECT) - .setChannelProvider(channelProvider) - .setCredentials(NoCredentials.getInstance()) - .setSessionPoolOption(SessionPoolOptions.newBuilder().setMinSessions(0).build()) - .build() - .getService()) { - DatabaseClientImpl dbClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - // The create session failure should propagate to the client and not retry. - try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - // The server should only receive one BatchCreateSessions request. - assertThat(mockSpanner.getRequests()).hasSize(1); - } - try { - dbClient.readWriteTransaction(); - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - // No additional requests should have been sent by the client. - assertThat(mockSpanner.getRequests()).hasSize(1); + public void testDatabaseOrInstanceDoesNotExistOnCreate() throws Exception { + StatusRuntimeException[] exceptions = + new StatusRuntimeException[] { + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME), + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, INSTANCE_NAME) + }; + for (StatusRuntimeException exception : exceptions) { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.ofStickyException(exception)); + // Ensure there are no sessions in the pool by default. + try (Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId(TEST_PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .setSessionPoolOption(SessionPoolOptions.newBuilder().setMinSessions(0).build()) + .build() + .getService()) { + DatabaseClientImpl dbClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // The create session failure should propagate to the client and not retry. + try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + // The server should only receive one BatchCreateSessions request. + assertThat(mockSpanner.getRequests()).hasSize(1); + } + try { + dbClient.readWriteTransaction(); + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + // No additional requests should have been sent by the client. + assertThat(mockSpanner.getRequests()).hasSize(1); + } } + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); } } @Test - public void testDatabaseDoesNotExistOnReplenish() throws Exception { - mockSpanner.setBatchCreateSessionsExecutionTime( - SimulatedExecutionTime.ofStickyException( - SpannerExceptionFactoryTest.newStatusResourceNotFoundException( - "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME))); - DatabaseClientImpl dbClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - // Wait until session creation has finished. - Stopwatch watch = Stopwatch.createStarted(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { - Thread.sleep(1L); - } - // All session creation should fail and stop trying. - assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); - assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); - // Force a maintainer run. This should schedule new session creation. - dbClient.pool.poolMaintainer.maintainPool(); - // Wait until the replenish has finished. - watch = watch.reset().start(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { - Thread.sleep(1L); + public void testDatabaseOrInstanceDoesNotExistOnReplenish() throws Exception { + StatusRuntimeException[] exceptions = + new StatusRuntimeException[] { + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME), + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, INSTANCE_NAME) + }; + for (StatusRuntimeException exception : exceptions) { + try (Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId(TEST_PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .build() + .getService()) { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.ofStickyException(exception)); + DatabaseClientImpl dbClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Wait until session creation has finished. + Stopwatch watch = Stopwatch.createStarted(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { + Thread.sleep(1L); + } + // All session creation should fail and stop trying. + assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); + assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); + // Force a maintainer run. This should schedule new session creation. + dbClient.pool.poolMaintainer.maintainPool(); + // Wait until the replenish has finished. + watch = watch.reset().start(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && dbClient.pool.getNumberOfSessionsBeingCreated() > 0) { + Thread.sleep(1L); + } + // All session creation from replenishPool should fail and stop trying. + assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); + assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); + } + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); } - // All session creation from replenishPool should fail and stop trying. - assertThat(dbClient.pool.getNumberOfSessionsInPool(), is(equalTo(0))); - assertThat(dbClient.pool.getNumberOfSessionsBeingCreated(), is(equalTo(0))); } @Test @@ -442,86 +509,98 @@ public Void run(TransactionContext transaction) throws Exception { * a new {@link DatabaseClient} is created. */ @Test - public void testDatabaseIsDeletedAndThenRecreated() throws Exception { - try (Spanner spanner = - SpannerOptions.newBuilder() - .setProjectId(TEST_PROJECT) - .setChannelProvider(channelProvider) - .setCredentials(NoCredentials.getInstance()) - .build() - .getService()) { - DatabaseClientImpl dbClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - // Wait until all sessions have been created and prepared. - Stopwatch watch = Stopwatch.createStarted(); - while (watch.elapsed(TimeUnit.SECONDS) < 5 - && (dbClient.pool.getNumberOfSessionsBeingCreated() > 0 - || dbClient.pool.getNumberOfSessionsBeingPrepared() > 0)) { - Thread.sleep(1L); - } - // Simulate that the database has been deleted. - mockSpanner.setStickyGlobalExceptions(true); - mockSpanner.addException( + public void testDatabaseOrInstanceIsDeletedAndThenRecreated() throws Exception { + StatusRuntimeException[] exceptions = + new StatusRuntimeException[] { SpannerExceptionFactoryTest.newStatusResourceNotFoundException( - "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME)); + "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, DATABASE_NAME), + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, INSTANCE_NAME) + }; + for (StatusRuntimeException exception : exceptions) { + try (Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId(TEST_PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .build() + .getService()) { + DatabaseClientImpl dbClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + // Wait until all sessions have been created and prepared. + Stopwatch watch = Stopwatch.createStarted(); + while (watch.elapsed(TimeUnit.SECONDS) < 5 + && (dbClient.pool.getNumberOfSessionsBeingCreated() > 0 + || dbClient.pool.getNumberOfSessionsBeingPrepared() > 0)) { + Thread.sleep(1L); + } + // Simulate that the database or instance has been deleted. + mockSpanner.setStickyGlobalExceptions(true); + mockSpanner.addException(exception); - // All subsequent calls should fail with a DatabaseNotFoundException. - try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { - while (rs.next()) {} - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - } - try { - dbClient - .readWriteTransaction() - .run( - new TransactionCallable() { - @Override - public Void run(TransactionContext transaction) throws Exception { - return null; - } - }); - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - } + // All subsequent calls should fail with a DatabaseNotFoundException. + try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { + while (rs.next()) {} + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + } + try { + dbClient + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Void run(TransactionContext transaction) throws Exception { + return null; + } + }); + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + } - // Now simulate that the database has been re-created. The database client should still throw - // DatabaseNotFoundExceptions, as it is not the same database. The server should not receive - // any new requests. - mockSpanner.reset(); - // All subsequent calls should fail with a DatabaseNotFoundException. - try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { - while (rs.next()) {} - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - } - try { - dbClient - .readWriteTransaction() - .run( - new TransactionCallable() { - @Override - public Void run(TransactionContext transaction) throws Exception { - return null; - } - }); - fail("missing expected exception"); - } catch (DatabaseNotFoundException e) { - } - assertThat(mockSpanner.getRequests()).isEmpty(); - // Now get a new database client. Normally multiple calls to Spanner#getDatabaseClient will - // return the same instance, but not when the instance has been invalidated by a - // DatabaseNotFoundException. - DatabaseClientImpl newClient = - (DatabaseClientImpl) - spanner.getDatabaseClient(DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); - assertThat(newClient).isNotSameInstanceAs(dbClient); - // Executing a query should now work without problems. - try (ResultSet rs = newClient.singleUse().executeQuery(SELECT1)) { - while (rs.next()) {} + // Now simulate that the database has been re-created. The database client should still + // throw + // DatabaseNotFoundExceptions, as it is not the same database. The server should not receive + // any new requests. + mockSpanner.reset(); + // All subsequent calls should fail with a DatabaseNotFoundException. + try (ResultSet rs = dbClient.singleUse().executeQuery(SELECT1)) { + while (rs.next()) {} + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + } + try { + dbClient + .readWriteTransaction() + .run( + new TransactionCallable() { + @Override + public Void run(TransactionContext transaction) throws Exception { + return null; + } + }); + fail("missing expected exception"); + } catch (DatabaseNotFoundException | InstanceNotFoundException e) { + } + assertThat(mockSpanner.getRequests()).isEmpty(); + // Now get a new database client. Normally multiple calls to Spanner#getDatabaseClient will + // return the same instance, but not when the instance has been invalidated by a + // DatabaseNotFoundException. + DatabaseClientImpl newClient = + (DatabaseClientImpl) + spanner.getDatabaseClient( + DatabaseId.of(TEST_PROJECT, TEST_INSTANCE, TEST_DATABASE)); + assertThat(newClient).isNotSameInstanceAs(dbClient); + // Executing a query should now work without problems. + try (ResultSet rs = newClient.singleUse().executeQuery(SELECT1)) { + while (rs.next()) {} + } + assertThat(mockSpanner.getRequests()).isNotEmpty(); } - assertThat(mockSpanner.getRequests()).isNotEmpty(); + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java index 1caec1dd19..bc7dd5498d 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerExceptionFactoryTest.java @@ -52,6 +52,12 @@ static DatabaseNotFoundException newDatabaseNotFoundException(String name) { "Database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, name); } + static InstanceNotFoundException newInstanceNotFoundException(String name) { + return (InstanceNotFoundException) + newResourceNotFoundException( + "Instance", SpannerExceptionFactory.INSTANCE_RESOURCE_TYPE, name); + } + static StatusRuntimeException newStatusResourceNotFoundException( String shortName, String resourceType, String resourceName) { ResourceInfo resourceInfo = @@ -186,6 +192,12 @@ public void databaseNotFound() { assertThat(e.getResourceName()).isEqualTo("projects/p/instances/i/databases/d"); } + @Test + public void instanceNotFound() { + InstanceNotFoundException e = newInstanceNotFoundException("projects/p/instances/i"); + assertThat(e.getResourceName()).isEqualTo("projects/p/instances/i"); + } + @Test public void statusRuntimeExceptionSessionNotFound() { SpannerException spannerException = 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 ef50365edc..d24c56661f 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 @@ -24,8 +24,11 @@ import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.spanner.Database; import com.google.cloud.spanner.DatabaseClient; +import com.google.cloud.spanner.DatabaseId; import com.google.cloud.spanner.DatabaseNotFoundException; import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.InstanceId; +import com.google.cloud.spanner.InstanceNotFoundException; import com.google.cloud.spanner.IntegrationTest; import com.google.cloud.spanner.IntegrationTestEnv; import com.google.cloud.spanner.ResultSet; @@ -119,4 +122,20 @@ public void databaseDeletedTest() throws Exception { assertThat(rs.next()).isFalse(); } } + + @Test + public void instanceNotFound() { + InstanceId testId = env.getTestHelper().getInstanceId(); + InstanceId nonExistingInstanceId = + InstanceId.of(testId.getProject(), testId.getInstance() + "-na"); + DatabaseClient client = + env.getTestHelper() + .getClient() + .getDatabaseClient(DatabaseId.of(nonExistingInstanceId, "some-db")); + try (ResultSet rs = client.singleUse().executeQuery(Statement.of("SELECT 1"))) { + fail("missing expected exception"); + } catch (InstanceNotFoundException e) { + assertThat(e.getResourceName()).isEqualTo(nonExistingInstanceId.getName()); + } + } }