Skip to content

Commit

Permalink
fix: stop sending RPCs on InstanceNotFound (#61)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
olavloite committed Feb 6, 2020
1 parent c7aab1c commit 7618ac8
Show file tree
Hide file tree
Showing 6 changed files with 361 additions and 205 deletions.
@@ -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);
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1075,7 +1077,7 @@ private static enum Position {
private SettableFuture<Void> closureFuture;

@GuardedBy("lock")
private DatabaseNotFoundException databaseNotFound;
private ResourceNotFoundException resourceNotFoundException;

@GuardedBy("lock")
private final LinkedList<PooledSession> readSessions = new LinkedList<>();
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1495,17 +1497,18 @@ 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);
}
}

private void handlePrepareSessionFailure(SpannerException e, PooledSession session) {
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.
Expand All @@ -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);
Expand Down
Expand Up @@ -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<ResourceInfo> KEY_RESOURCE_INFO =
ProtoUtils.keyForProto(ResourceInfo.getDefaultInstance());

Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 7618ac8

Please sign in to comment.