From e640b130e5218a06352f0f74ba6aa8b6289237e0 Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Tue, 21 Jan 2020 17:13:39 -0800 Subject: [PATCH 1/7] Initial import of Resource Based Routing Impl from old repo --- .../com/google/cloud/spanner/Instance.java | 15 ++ .../cloud/spanner/InstanceConfigRpcCache.java | 121 +++++++++++++++ .../google/cloud/spanner/InstanceInfo.java | 40 ++++- .../google/cloud/spanner/SessionClient.java | 8 +- .../com/google/cloud/spanner/SessionImpl.java | 23 +-- .../com/google/cloud/spanner/SpannerImpl.java | 27 ++-- .../google/cloud/spanner/SpannerOptions.java | 2 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 138 ++++++++++++++---- .../cloud/spanner/spi/v1/SpannerRpc.java | 18 +++ .../cloud/spanner/BatchClientImplTest.java | 7 + .../spanner/BatchCreateSessionsTest.java | 20 ++- .../cloud/spanner/DatabaseClientImplTest.java | 12 +- .../cloud/spanner/GceTestEnvConfig.java | 7 + .../cloud/spanner/SessionClientTest.java | 7 +- .../google/cloud/spanner/SessionImplTest.java | 8 + .../cloud/spanner/SpannerGaxRetryTest.java | 7 +- .../google/cloud/spanner/SpannerImplTest.java | 4 + .../cloud/spanner/SpannerOptionsTest.java | 11 +- .../spanner/TransactionManagerImplTest.java | 22 ++- .../spanner/TransactionRunnerImplTest.java | 19 ++- .../spanner/spi/v1/GapicSpannerRpcTest.java | 19 ++- 21 files changed, 449 insertions(+), 86 deletions(-) create mode 100644 google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java index 5565c67de81..886560de546 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java @@ -21,6 +21,8 @@ import com.google.cloud.Policy; import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; + +import java.util.List; import java.util.Map; /** @@ -83,6 +85,18 @@ public Builder putAllLabels(Map labels) { return this; } + @Override + public InstanceInfo.Builder addEndpointUri(String endpointUrl) { + infoBuilder.addEndpointUri(endpointUrl); + return this; + } + + @Override + public InstanceInfo.Builder addAllEndpointUris(List endpointUris) { + infoBuilder.addAllEndpointUris(endpointUris); + return this; + } + @Override public Instance build() { return new Instance(this); @@ -177,6 +191,7 @@ static Instance fromProto( .setInstanceConfigId(InstanceConfigId.of(proto.getConfig())) .setDisplayName(proto.getDisplayName()) .setNodeCount(proto.getNodeCount()); + builder.addAllEndpointUris(proto.getEndpointUrisList()); State state; switch (proto.getState()) { case STATE_UNSPECIFIED: diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java new file mode 100644 index 00000000000..f9701ba4d90 --- /dev/null +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java @@ -0,0 +1,121 @@ +/* + * Copyright 2019 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.spi.v1.GapicSpannerRpc; +import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import com.google.protobuf.FieldMask; +import com.google.spanner.admin.instance.v1.GetInstanceRequest; +import com.google.spanner.admin.instance.v1.Instance; +import com.google.spanner.admin.instance.v1.InstanceName; +import com.google.spanner.v1.DatabaseName; +import com.google.spanner.v1.SessionName; +import java.util.concurrent.ExecutionException; +import java.util.logging.Level; +import java.util.logging.Logger; + +public class InstanceConfigRpcCache { + + private final LoadingCache cache; + + private Logger logger = Logger.getLogger(InstanceConfigRpcCache.class.getName()); + private final GapicSpannerRpc projectClient; + + public InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { + this.projectClient = projectClient; + cache = + CacheBuilder.newBuilder() + .build( + new CacheLoader() { + @Override + public SpannerRpc load(InstanceName instanceName) throws SpannerException { + GetInstanceRequest request = + GetInstanceRequest.newBuilder() + .setName(instanceName.toString()) + .setFieldMask(FieldMask.newBuilder().addPaths("endpoint_urls")) + .build(); + SpannerOptions.Builder optionsBuilder = projectClient.getOptions().toBuilder(); + try { + Instance instance = projectClient.getInstance(request); + if (instance.getEndpointUrisCount() > 0) { + optionsBuilder.setHost(instance.getEndpointUris(0)); + } else { + return projectClient; + } + } catch (SpannerException e) { + if (e.getErrorCode() == ErrorCode.UNIMPLEMENTED) { + // Ignore... + // This is backwards compatibility. + return projectClient; + } else { + logger.log( + Level.WARNING, + "Failed while resolving endpoint URLs for instance:" + + instanceName.toString() + + " reason: " + + e.getErrorCode().name()); + throw e; + } + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed while resolving endpoint URLs for instance:" + + instanceName.toString()); + throw SpannerExceptionFactory.newSpannerException(e); + } + return new GapicSpannerRpc(optionsBuilder.build(), false); + } + }); + } + + public SpannerRpc get(SessionName sessionName) { + InstanceName instanceName = + InstanceName.of(sessionName.getProject(), sessionName.getInstance()); + return get(instanceName); + } + + public SpannerRpc get(DatabaseName databaseName) { + InstanceName instanceName = + InstanceName.of(databaseName.getProject(), databaseName.getInstance()); + return get(instanceName); + } + + private SpannerRpc get(InstanceName instanceName) { + try { + SpannerRpc spannerRpc = cache.get(instanceName); + return spannerRpc; + } catch (ExecutionException e) { + logger.log( + Level.FINE, "Failed looking up instance in cache. id:" + instanceName.toString(), e); + throw SpannerExceptionFactory.newSpannerException( + ErrorCode.NOT_FOUND, + "Failed getting RPC Client for Instance:" + instanceName.toString(), + e); + } + } + + public void invalidateAll() { + for (SpannerRpc rpc : cache.asMap().values()) { + if (rpc == this.projectClient) continue; + rpc.shutdown(); + } + cache.invalidateAll(); + } +} \ No newline at end of file diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java index 6908a4999ac..4c82e6a0a81 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java @@ -20,9 +20,13 @@ import com.google.cloud.FieldSelector; import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.FieldMask; + +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Objects; @@ -76,6 +80,10 @@ public abstract static class Builder { public abstract Builder putAllLabels(Map labels); + public abstract Builder addEndpointUri(String endpointUrl); + + public abstract Builder addAllEndpointUris(List endpointUris); + public abstract InstanceInfo build(); } @@ -86,10 +94,12 @@ static class BuilderImpl extends Builder { private int nodeCount; private State state; private Map labels; + private List endpointUris; BuilderImpl(InstanceId id) { this.id = id; this.labels = new HashMap<>(); + this.endpointUris = new ArrayList<>(); } BuilderImpl(InstanceInfo instance) { @@ -99,6 +109,7 @@ static class BuilderImpl extends Builder { this.nodeCount = instance.nodeCount; this.state = instance.state; this.labels = new HashMap<>(instance.labels); + this.endpointUris = new ArrayList<>(instance.endpointUris); } @Override @@ -137,6 +148,18 @@ public BuilderImpl putAllLabels(Map labels) { return this; } + @Override + public Builder addEndpointUri(String endpointUrl) { + this.endpointUris.add(endpointUrl); + return this; + } + + @Override + public Builder addAllEndpointUris(List endpointUris) { + this.endpointUris.addAll(endpointUris); + return this; + } + @Override public InstanceInfo build() { return new InstanceInfo(this); @@ -149,6 +172,8 @@ public InstanceInfo build() { private final int nodeCount; private final State state; private final ImmutableMap labels; + private final ImmutableList endpointUris; + InstanceInfo(BuilderImpl builder) { this.id = builder.id; @@ -157,6 +182,7 @@ public InstanceInfo build() { this.nodeCount = builder.nodeCount; this.state = builder.state; this.labels = ImmutableMap.copyOf(builder.labels); + this.endpointUris = ImmutableList.copyOf(builder.endpointUris); } /** Returns the identifier of the instance. */ @@ -189,6 +215,11 @@ public Map getLabels() { return labels; } + /** Returns the endpoint uris for this instance. */ + public ImmutableList getEndpointUris() { + return endpointUris; + } + public Builder toBuilder() { return new BuilderImpl(this); } @@ -202,6 +233,7 @@ public String toString() { .add("nodeCount", nodeCount) .add("state", state) .add("labels", labels) + .add("endpointUris", endpointUris) .toString(); } @@ -219,12 +251,13 @@ public boolean equals(Object o) { && Objects.equals(displayName, that.displayName) && nodeCount == that.nodeCount && state == that.state - && Objects.equals(labels, that.labels); + && Objects.equals(labels, that.labels) + && Objects.equals(endpointUris, that.endpointUris); } @Override public int hashCode() { - return Objects.hash(id, configId, displayName, nodeCount, state, labels); + return Objects.hash(id, configId, displayName, nodeCount, state, labels, endpointUris); } com.google.spanner.admin.instance.v1.Instance toProto() { @@ -232,7 +265,8 @@ com.google.spanner.admin.instance.v1.Instance toProto() { com.google.spanner.admin.instance.v1.Instance.newBuilder() .setName(getId().getName()) .setNodeCount(getNodeCount()) - .putAllLabels(getLabels()); + .putAllLabels(getLabels()) + .addAllEndpointUris(getEndpointUris()); if (getDisplayName() != null) { builder.setDisplayName(getDisplayName()); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java index f71ba57742f..2516a46353e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import com.google.spanner.v1.DatabaseName; import io.opencensus.common.Scope; import io.opencensus.trace.Span; import java.util.ArrayList; @@ -201,10 +202,9 @@ SessionImpl createSession() { } Span span = SpannerImpl.tracer.spanBuilder(SpannerImpl.CREATE_SESSION).startSpan(); try (Scope s = SpannerImpl.tracer.withSpan(span)) { + SpannerRpc spannerRpc = spanner.getRpc(DatabaseName.parse(db.getName())); com.google.spanner.v1.Session session = - spanner - .getRpc() - .createSession(db.getName(), spanner.getOptions().getSessionLabels(), options); + spannerRpc.createSession(db.getName(), spanner.getOptions().getSessionLabels(), options); span.end(); return new SessionImpl(spanner, session.getName(), options); } catch (RuntimeException e) { @@ -278,7 +278,7 @@ private List internalBatchCreateSessions( try (Scope s = SpannerImpl.tracer.withSpan(span)) { List sessions = spanner - .getRpc() + .getRpc(DatabaseName.parse(db.getName())) .batchCreateSessions( db.getName(), sessionCount, spanner.getOptions().getSessionLabels(), options); span.addAnnotation( diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index 5df62a9baee..3e960c7766a 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -30,6 +30,7 @@ import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import com.google.spanner.v1.TransactionOptions; import io.opencensus.common.Scope; @@ -77,6 +78,7 @@ static interface SessionTransaction { private final SpannerImpl spanner; private final String name; + private final SessionName sessionName; private SessionTransaction activeTransaction; private ByteString readyTransactionId; private final Map options; @@ -85,6 +87,7 @@ static interface SessionTransaction { this.spanner = spanner; this.options = options; this.name = checkNotNull(name); + this.sessionName = SessionName.parse(name); } @Override @@ -99,7 +102,8 @@ public String getName() { @Override public long executePartitionedUpdate(Statement stmt) { setActive(null); - PartitionedDMLTransaction txn = new PartitionedDMLTransaction(this, spanner.getRpc()); + PartitionedDMLTransaction txn = + new PartitionedDMLTransaction(this, spanner.getRpc(sessionName)); return txn.executePartitionedUpdate(stmt, spanner.getOptions().getPartitionedDmlTimeout()); } @@ -136,7 +140,7 @@ public Timestamp writeAtLeastOnce(Iterable mutations) throws SpannerEx .build(); Span span = tracer.spanBuilder(SpannerImpl.COMMIT).startSpan(); try (Scope s = tracer.withSpan(span)) { - CommitResponse response = spanner.getRpc().commit(request, options); + CommitResponse response = spanner.getRpc(sessionName).commit(request, options); Timestamp t = Timestamp.fromProto(response.getCommitTimestamp()); span.end(); return t; @@ -157,7 +161,8 @@ public ReadContext singleUse() { @Override public ReadContext singleUse(TimestampBound bound) { return setActive( - new SingleReadContext(this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + new SingleReadContext( + this, bound, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override @@ -169,7 +174,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() { public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) { return setActive( new SingleUseReadOnlyTransaction( - this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + this, bound, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override @@ -181,13 +186,13 @@ public ReadOnlyTransaction readOnlyTransaction() { public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) { return setActive( new MultiUseReadOnlyTransaction( - this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + this, bound, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override public TransactionRunner readWriteTransaction() { return setActive( - new TransactionRunnerImpl(this, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + new TransactionRunnerImpl(this, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override @@ -200,7 +205,7 @@ public void prepareReadWriteTransaction() { public void close() { Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION).startSpan(); try (Scope s = tracer.withSpan(span)) { - spanner.getRpc().deleteSession(name, options); + spanner.getRpc(sessionName).deleteSession(name, options); span.end(); } catch (RuntimeException e) { TraceUtil.endSpanWithFailure(span, e); @@ -218,7 +223,7 @@ ByteString beginTransaction() { TransactionOptions.newBuilder() .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance())) .build(); - Transaction txn = spanner.getRpc().beginTransaction(request, options); + Transaction txn = spanner.getRpc(sessionName).beginTransaction(request, options); if (txn.getId().isEmpty()) { throw newSpannerException(ErrorCode.INTERNAL, "Missing id in transaction\n" + getName()); } @@ -233,7 +238,7 @@ ByteString beginTransaction() { TransactionContextImpl newTransaction() { TransactionContextImpl txn = new TransactionContextImpl( - this, readyTransactionId, spanner.getRpc(), spanner.getDefaultPrefetchChunks()); + this, readyTransactionId, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks()); return txn; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 12cf89dd56b..2b70e26aee9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -35,6 +35,8 @@ import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; +import com.google.spanner.v1.DatabaseName; +import com.google.spanner.v1.SessionName; import io.grpc.Context; import io.opencensus.trace.AttributeValue; import io.opencensus.trace.Tracer; @@ -83,7 +85,7 @@ class SpannerImpl extends BaseService implements Spanner { READ); } - private final SpannerRpc gapicRpc; + private final SpannerRpc spannerRpc; @GuardedBy("this") private final Map dbClients = new HashMap<>(); @@ -98,12 +100,12 @@ class SpannerImpl extends BaseService implements Spanner { private boolean spannerIsClosed = false; @VisibleForTesting - SpannerImpl(SpannerRpc gapicRpc, SpannerOptions options) { + SpannerImpl(SpannerRpc spannerRpc, SpannerOptions options) { super(options); - this.gapicRpc = gapicRpc; - this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); + this.spannerRpc = spannerRpc; + this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), spannerRpc); this.instanceClient = - new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); + new InstanceAdminClientImpl(options.getProjectId(), spannerRpc, dbAdminClient); } SpannerImpl(SpannerOptions options) { @@ -164,8 +166,13 @@ public void cancelled(Context context) { } /** Returns the {@link SpannerRpc} of this {@link SpannerImpl} instance. */ - SpannerRpc getRpc() { - return gapicRpc; + SpannerRpc getRpc(SessionName sessionName) { + return spannerRpc.getRpc(sessionName); + } + + /** Returns the {@link SpannerRpc} of this {@link SpannerImpl} instance. */ + SpannerRpc getRpc(DatabaseName databaseName) { + return spannerRpc.getRpc(databaseName); } /** Returns the default setting for prefetchChunks of this {@link SpannerImpl} instance. */ @@ -253,11 +260,7 @@ public void close() { sessionClient.close(); } sessionClients.clear(); - try { - gapicRpc.shutdown(); - } catch (RuntimeException e) { - logger.log(Level.WARNING, "Failed to close channels", e); - } + spannerRpc.shutdown(); } @Override diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index a6aa502909e..4a687e34e42 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -100,7 +100,7 @@ private static class DefaultSpannerRpcFactory implements SpannerRpcFactory { @Override public ServiceRpc create(SpannerOptions options) { - return new GapicSpannerRpc(options); + return new GapicSpannerRpc(options, true); } } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 19f41618fef..d6927ad7719 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -37,6 +37,7 @@ import com.google.api.gax.rpc.WatchdogProvider; import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.spanner.InstanceConfigRpcCache; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerOptions; @@ -88,6 +89,7 @@ import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; import com.google.spanner.v1.CreateSessionRequest; +import com.google.spanner.v1.DatabaseName; import com.google.spanner.v1.DeleteSessionRequest; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteBatchDmlResponse; @@ -100,6 +102,7 @@ import com.google.spanner.v1.ResultSet; import com.google.spanner.v1.RollbackRequest; import com.google.spanner.v1.Session; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import io.grpc.CallCredentials; import io.grpc.Context; @@ -172,11 +175,12 @@ private synchronized void shutdown() { private final ManagedInstantiatingExecutorProvider executorProvider; private boolean rpcIsClosed; - private final SpannerStub spannerStub; + private final SpannerStub rootConnection; private final InstanceAdminStub instanceAdminStub; private final DatabaseAdminStub databaseAdminStub; private final String projectId; private final String projectName; + private final SpannerOptions options; private final SpannerMetadataProvider metadataProvider; private final CallCredentialsProvider callCredentialsProvider; private final Duration waitTimeout = @@ -188,11 +192,14 @@ private synchronized void shutdown() { private final ScheduledExecutorService spannerWatchdog; - public static GapicSpannerRpc create(SpannerOptions options) { - return new GapicSpannerRpc(options); + private InstanceConfigRpcCache cache; + + public static GapicSpannerRpc create(SpannerOptions options, boolean initCache) { + return new GapicSpannerRpc(options, initCache); } - public GapicSpannerRpc(final SpannerOptions options) { + public GapicSpannerRpc(final SpannerOptions options, boolean initCache) { + this.options = options; this.projectId = options.getProjectId(); String projectNameStr = PROJECT_NAME_TEMPLATE.instantiate("project", this.projectId); try { @@ -272,7 +279,7 @@ public GapicSpannerRpc(final SpannerOptions options) { .withClock(NanoClock.getDefaultClock()); try { - this.spannerStub = + this.rootConnection = GrpcSpannerStub.create( options .getSpannerStubSettings() @@ -304,6 +311,10 @@ public GapicSpannerRpc(final SpannerOptions options) { } catch (Exception e) { throw newSpannerException(e); } + // Init Project Client. + if (initCache) { + this.cache = new InstanceConfigRpcCache(this); + } } @Override @@ -375,11 +386,25 @@ public OperationFuture updateInstance( @Override public Instance getInstance(String instanceName) throws SpannerException { GetInstanceRequest request = GetInstanceRequest.newBuilder().setName(instanceName).build(); + return getInstance(request); + } - GrpcCallContext context = newCallContext(null, instanceName); + @Override + public Instance getInstance(GetInstanceRequest request) throws SpannerException { + GrpcCallContext context = newCallContext(null, request.getName()); return get(instanceAdminStub.getInstanceCallable().futureCall(request, context)); } + private SpannerStub getSpannerStub(DatabaseName databaseName) { + if (this.cache == null) return rootConnection; + return this.cache.get(databaseName).getStub(); + } + + private SpannerStub getSpannerStub(SessionName sessionName) { + if (this.cache == null) return rootConnection; + return this.cache.get(sessionName).getStub(); + } + @Override public void deleteInstance(String instanceName) throws SpannerException { DeleteInstanceRequest request = @@ -501,8 +526,10 @@ public List batchCreateSessions( } BatchCreateSessionsRequest request = requestBuilder.build(); GrpcCallContext context = newCallContext(options, databaseName); - return get(spannerStub.batchCreateSessionsCallable().futureCall(request, context)) - .getSessionList(); + return get(getSpannerStub(DatabaseName.parse(databaseName)) + .batchCreateSessionsCallable() + .futureCall(request, context)) + .getSessionList(); } @Override @@ -517,24 +544,28 @@ public Session createSession( } CreateSessionRequest request = requestBuilder.build(); GrpcCallContext context = newCallContext(options, databaseName); - return get(spannerStub.createSessionCallable().futureCall(request, context)); - } + return get( + getSpannerStub(DatabaseName.parse(databaseName)) + .createSessionCallable() + .futureCall(request, context)); } @Override public void deleteSession(String sessionName, @Nullable Map options) throws SpannerException { DeleteSessionRequest request = DeleteSessionRequest.newBuilder().setName(sessionName).build(); GrpcCallContext context = newCallContext(options, sessionName); - get(spannerStub.deleteSessionCallable().futureCall(request, context)); - } + get(getSpannerStub(SessionName.parse(sessionName)) + .deleteSessionCallable() + .futureCall(request, context)); } @Override public StreamingCall read( ReadRequest request, ResultStreamConsumer consumer, @Nullable Map options) { GrpcCallContext context = newCallContext(options, request.getSession()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); - spannerStub.streamingReadCallable().call(request, responseObserver, context); - final StreamController controller = responseObserver.getController(); + getSpannerStub(SessionName.parse(request.getSession())) + .streamingReadCallable() + .call(request, responseObserver, context); final StreamController controller = responseObserver.getController(); return new StreamingCall() { @Override public void request(int numMessage) { @@ -553,22 +584,28 @@ public void cancel(String message) { @Override public ResultSet executeQuery(ExecuteSqlRequest request, @Nullable Map options) { GrpcCallContext context = newCallContext(options, request.getSession()); - return get(spannerStub.executeSqlCallable().futureCall(request, context)); - } + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .executeSqlCallable() + .futureCall(request, context)); } @Override public ResultSet executePartitionedDml( ExecuteSqlRequest request, @Nullable Map options, Duration timeout) { GrpcCallContext context = newCallContext(options, request.getSession(), timeout); - return get(spannerStub.executeSqlCallable().futureCall(request, context)); - } + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .executeSqlCallable() + .futureCall(request, context)); } @Override public StreamingCall executeQuery( ExecuteSqlRequest request, ResultStreamConsumer consumer, @Nullable Map options) { GrpcCallContext context = newCallContext(options, request.getSession()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); - spannerStub.executeStreamingSqlCallable().call(request, responseObserver, context); + getSpannerStub(SessionName.parse(request.getSession())) + .executeStreamingSqlCallable() + .call(request, responseObserver, context); final StreamController controller = responseObserver.getController(); return new StreamingCall() { @Override @@ -590,43 +627,55 @@ public ExecuteBatchDmlResponse executeBatchDml( ExecuteBatchDmlRequest request, @Nullable Map options) { GrpcCallContext context = newCallContext(options, request.getSession()); - return get(spannerStub.executeBatchDmlCallable().futureCall(request, context)); + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .executeBatchDmlCallable() + .futureCall(request, context)); } @Override public Transaction beginTransaction( BeginTransactionRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); - return get(spannerStub.beginTransactionCallable().futureCall(request, context)); - } + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .beginTransactionCallable() + .futureCall(request, context)); } @Override public CommitResponse commit(CommitRequest commitRequest, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, commitRequest.getSession()); - return get(spannerStub.commitCallable().futureCall(commitRequest, context)); - } + return get( + getSpannerStub(SessionName.parse(commitRequest.getSession())) + .commitCallable() + .futureCall(commitRequest, context)); } @Override public void rollback(RollbackRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); - get(spannerStub.rollbackCallable().futureCall(request, context)); - } + get(getSpannerStub(SessionName.parse(request.getSession())) + .rollbackCallable() + .futureCall(request, context)); } @Override public PartitionResponse partitionQuery( PartitionQueryRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); - return get(spannerStub.partitionQueryCallable().futureCall(request, context)); - } + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .partitionQueryCallable() + .futureCall(request, context)); } @Override public PartitionResponse partitionRead( PartitionReadRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); - return get(spannerStub.partitionReadCallable().futureCall(request, context)); - } + return get( + getSpannerStub(SessionName.parse(request.getSession())) + .partitionReadCallable() + .futureCall(request, context)); } @Override public Policy getDatabaseAdminIAMPolicy(String resource) { @@ -740,11 +789,14 @@ private GrpcCallContext newCallContext( @Override public void shutdown() { this.rpcIsClosed = true; - this.spannerStub.close(); + this.rootConnection.close(); this.instanceAdminStub.close(); this.databaseAdminStub.close(); this.spannerWatchdog.shutdown(); this.executorProvider.shutdown(); + if (this.cache != null) { + this.cache.invalidateAll(); + } } @Override @@ -752,6 +804,30 @@ public boolean isClosed() { return rpcIsClosed; } + @Override + public SpannerOptions getOptions() { + return options; + } + + @Override + public SpannerStub getStub() { + return rootConnection; + } + + @Override + @VisibleForTesting + public SpannerRpc getRpc(SessionName sessionName) { + if (this.cache == null) return this; + return this.cache.get(sessionName); + } + + @Override + @VisibleForTesting + public SpannerRpc getRpc(DatabaseName databaseName) { + if (this.cache == null) return this; + return this.cache.get(databaseName); + } + /** * A {@code ResponseObserver} that exposes the {@code StreamController} and delegates callbacks to * the {@link ResultStreamConsumer}. diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java index d608b3730ac..ef23076e5b6 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/SpannerRpc.java @@ -19,8 +19,11 @@ import com.google.api.gax.longrunning.OperationFuture; import com.google.cloud.ServiceRpc; import com.google.cloud.spanner.SpannerException; +import com.google.cloud.spanner.SpannerOptions; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; +import com.google.cloud.spanner.v1.stub.SpannerStub; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.iam.v1.Policy; import com.google.iam.v1.TestIamPermissionsResponse; @@ -31,12 +34,14 @@ import com.google.spanner.admin.database.v1.Database; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; +import com.google.spanner.admin.instance.v1.GetInstanceRequest; import com.google.spanner.admin.instance.v1.Instance; import com.google.spanner.admin.instance.v1.InstanceConfig; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; +import com.google.spanner.v1.DatabaseName; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteBatchDmlResponse; import com.google.spanner.v1.ExecuteSqlRequest; @@ -48,6 +53,7 @@ import com.google.spanner.v1.ResultSet; import com.google.spanner.v1.RollbackRequest; import com.google.spanner.v1.Session; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import java.util.List; import java.util.Map; @@ -183,6 +189,8 @@ OperationFuture updateInstance( Instance getInstance(String instanceName) throws SpannerException; + Instance getInstance(GetInstanceRequest request) throws SpannerException; + void deleteInstance(String instanceName) throws SpannerException; // Database admin APIs. @@ -279,4 +287,14 @@ TestIamPermissionsResponse testInstanceAdminIAMPermissions( public void shutdown(); boolean isClosed(); + + SpannerOptions getOptions(); + + SpannerStub getStub(); + + @VisibleForTesting + SpannerRpc getRpc(SessionName sessionName); + + @VisibleForTesting + SpannerRpc getRpc(DatabaseName databaseName); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index 002a992b6f8..e26ca7728fd 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -17,6 +17,7 @@ package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.anyMap; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; @@ -32,7 +33,9 @@ import com.google.protobuf.ByteString; import com.google.protobuf.util.Timestamps; import com.google.spanner.v1.BeginTransactionRequest; +import com.google.spanner.v1.DatabaseName; import com.google.spanner.v1.Session; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import java.util.Collections; import java.util.Map; @@ -75,9 +78,13 @@ public void setUp() { GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); when(transportOptions.getExecutorFactory()).thenReturn(mock(ExecutorFactory.class)); when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); + when(spannerOptions.getProjectId()).thenReturn("my-project"); + when(gapicRpc.getOptions()).thenReturn(spannerOptions); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); client = new BatchClientImpl(spanner.getSessionClient(db)); + when(gapicRpc.getRpc(any(DatabaseName.class))).thenReturn(gapicRpc); + when(gapicRpc.getRpc(any(SessionName.class))).thenReturn(gapicRpc); } @SuppressWarnings("unchecked") diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java index 5cbd163fae7..017163f7826 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java @@ -26,6 +26,7 @@ import com.google.cloud.spanner.MockSpannerServiceImpl.SimulatedExecutionTime; import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; import com.google.cloud.spanner.TransactionRunner.TransactionCallable; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.common.base.Stopwatch; import com.google.protobuf.ListValue; import com.google.spanner.v1.ResultSetMetadata; @@ -43,6 +44,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class BatchCreateSessionsTest { @@ -78,6 +81,7 @@ public class BatchCreateSessionsTest { private static MockSpannerServiceImpl mockSpanner; private static Server server; private static LocalChannelProvider channelProvider; + @Mock private SpannerRpc gapicRpc; @BeforeClass public static void startStaticServer() throws IOException { @@ -104,6 +108,7 @@ public static void stopServer() throws InterruptedException { @Before public void setUp() throws IOException { mockSpanner.reset(); + MockitoAnnotations.initMocks(this); } private Spanner createSpanner(int minSessions, int maxSessions) { @@ -112,13 +117,14 @@ private Spanner createSpanner(int minSessions, int maxSessions) { .setMinSessions(minSessions) .setMaxSessions(maxSessions) .build(); - return SpannerOptions.newBuilder() - .setProjectId("[PROJECT]") - .setChannelProvider(channelProvider) - .setSessionPoolOption(sessionPoolOptions) - .setCredentials(NoCredentials.getInstance()) - .build() - .getService(); + SpannerOptions options = + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setChannelProvider(channelProvider) + .setSessionPoolOption(sessionPoolOptions) + .setCredentials(NoCredentials.getInstance()) + .build(); + return options.getService(); } @Test 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 b147bd8d489..9217d24290e 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 @@ -56,6 +56,7 @@ public class DatabaseClientImplTest { private static MockSpannerServiceImpl mockSpanner; private static Server server; private static LocalChannelProvider channelProvider; + private static Spanner spanner; private static final Statement UPDATE_STATEMENT = Statement.of("UPDATE FOO SET BAR=1 WHERE BAZ=2"); private static final Statement INVALID_UPDATE_STATEMENT = @@ -84,7 +85,6 @@ public class DatabaseClientImplTest { .build()) .setMetadata(SELECT1_METADATA) .build(); - private Spanner spanner; @BeforeClass public static void startStaticServer() throws IOException { @@ -116,20 +116,20 @@ public static void stopServer() throws InterruptedException { @Before public void setUp() throws IOException { - spanner = + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + SpannerOptions options = SpannerOptions.newBuilder() .setProjectId("[PROJECT]") .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()) - .build() - .getService(); + .build(); + spanner = options.getService(); } @After public void tearDown() throws Exception { spanner.close(); - mockSpanner.reset(); - mockSpanner.removeAllExecutionTimes(); } /** diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java index eaea833491b..1f8f1dd137e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/GceTestEnvConfig.java @@ -20,6 +20,7 @@ import com.google.auth.oauth2.GoogleCredentials; import com.google.cloud.spanner.spi.v1.SpannerInterceptorProvider; +import com.google.spanner.admin.instance.v1.InstanceName; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -39,6 +40,7 @@ public class GceTestEnvConfig implements TestEnvConfig { public static final String GCE_PROJECT_ID = "spanner.gce.config.project_id"; public static final String GCE_SERVER_URL = "spanner.gce.config.server_url"; public static final String GCE_CREDENTIALS_FILE = "spanner.gce.config.credentials_file"; + public static final String TEST_ENV_INSTANCE = "spanner.testenv.instance"; public static final String GCE_STREAM_BROKEN_PROBABILITY = "spanner.gce.config.stream_broken_probability"; @@ -48,10 +50,15 @@ public GceTestEnvConfig() { String projectId = System.getProperty(GCE_PROJECT_ID, ""); String serverUrl = System.getProperty(GCE_SERVER_URL, ""); String credentialsFile = System.getProperty(GCE_CREDENTIALS_FILE, ""); + String instanceId = System.getProperty(TEST_ENV_INSTANCE, ""); double errorProbability = Double.parseDouble(System.getProperty(GCE_STREAM_BROKEN_PROBABILITY, "0.0")); checkState(errorProbability <= 1.0); SpannerOptions.Builder builder = SpannerOptions.newBuilder(); + if (!instanceId.isEmpty() && InstanceName.isParsableFrom(instanceId)) { + InstanceName instanceName = InstanceName.parse(instanceId); + builder.setProjectId(instanceName.getProject()); + } if (!projectId.isEmpty()) { builder.setProjectId(projectId); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java index ce8b8d44097..78f59216244 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java @@ -17,6 +17,7 @@ package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -38,6 +39,9 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; + +import com.google.spanner.v1.DatabaseName; +import com.google.spanner.v1.SessionName; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -107,7 +111,8 @@ public ScheduledExecutorService get() { when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); when(spanner.getOptions()).thenReturn(spannerOptions); - when(spanner.getRpc()).thenReturn(rpc); + when(spanner.getRpc(any(SessionName.class))).thenReturn(rpc); + when(spanner.getRpc(any(DatabaseName.class))).thenReturn(rpc); } @Test diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java index f3f205a5294..21f1b3f6924 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -34,11 +35,13 @@ import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; +import com.google.spanner.v1.DatabaseName; import com.google.spanner.v1.Mutation.Write; import com.google.spanner.v1.PartialResultSet; import com.google.spanner.v1.ReadRequest; import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.Session; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import java.text.ParseException; import java.util.Arrays; @@ -86,6 +89,9 @@ public void setUp() { GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); when(transportOptions.getExecutorFactory()).thenReturn(mock(ExecutorFactory.class)); when(spannerOptions.getTransportOptions()).thenReturn(transportOptions); + when(spannerOptions.getProjectId()).thenReturn("p1"); + when(spannerOptions.toBuilder()).thenReturn(SpannerOptions.newBuilder().setProjectId("p1")); + when(rpc.getOptions()).thenReturn(spannerOptions); @SuppressWarnings("resource") SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); String dbName = "projects/p1/instances/i1/databases/d1"; @@ -110,6 +116,8 @@ public void setUp() { .build(); Mockito.when(rpc.commit(Mockito.any(CommitRequest.class), Mockito.any(Map.class))) .thenReturn(commitResponse); + Mockito.when(rpc.getRpc(any(DatabaseName.class))).thenReturn(rpc); + Mockito.when(rpc.getRpc(any(SessionName.class))).thenReturn(rpc); session = spanner.getSessionClient(db).createSession(); // We expect the same options, "options", on all calls on "session". options = optionsCaptor.getValue(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index ef57cf7cb97..937a875d6bf 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -95,7 +95,7 @@ public class SpannerGaxRetryTest { .asRuntimeException(); private static MockSpannerServiceImpl mockSpanner; private static Server server; - private static LocalChannelProvider channelProvider; + private static String uniqueName; private Spanner spanner; private DatabaseClient client; private Spanner spannerWithTimeout; @@ -110,7 +110,7 @@ public static void startStaticServer() throws IOException { mockSpanner.putStatementResult(StatementResult.query(SELECT1AND2, SELECT1_RESULTSET)); mockSpanner.putStatementResult(StatementResult.update(UPDATE_STATEMENT, UPDATE_COUNT)); - String uniqueName = InProcessServerBuilder.generateName(); + uniqueName = InProcessServerBuilder.generateName(); server = InProcessServerBuilder.forName(uniqueName) // We need to use a real executor for timeouts to occur. @@ -118,7 +118,6 @@ public static void startStaticServer() throws IOException { .addService(mockSpanner) .build() .start(); - channelProvider = LocalChannelProvider.create(uniqueName); } @AfterClass @@ -133,7 +132,7 @@ public void setUp() throws Exception { SpannerOptions.Builder builder = SpannerOptions.newBuilder() .setProjectId("[PROJECT]") - .setChannelProvider(channelProvider) + .setChannelProvider(LocalChannelProvider.create(uniqueName)) .setCredentials(NoCredentials.getInstance()); // Make sure the session pool is empty by default. builder.setSessionPoolOption( diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 23325e902f1..b36450a76a7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -31,6 +31,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; + +import com.google.spanner.admin.instance.v1.ProjectName; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -58,6 +60,8 @@ public void setUp() { when(spannerOptions.getPrefetchChunks()).thenReturn(1); when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); + when(spannerOptions.getProjectId()).thenReturn(ProjectName.of("[PROJECT]").toString()); + when(rpc.getOptions()).thenReturn(spannerOptions); when(spannerOptions.getSessionLabels()).thenReturn(Collections.emptyMap()); impl = new SpannerImpl(rpc, spannerOptions); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 214773b9fd1..ee4de804582 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -388,13 +388,18 @@ public void testDoNotCacheClosedSpannerInstance() { public void testSetClientLibToken() { final String jdbcToken = "sp-jdbc"; final String hibernateToken = "sp-hib"; - SpannerOptions options = SpannerOptions.newBuilder().setClientLibToken(jdbcToken).build(); + SpannerOptions options = + SpannerOptions.newBuilder().setProjectId("[PROJECT]").setClientLibToken(jdbcToken).build(); assertThat(options.getClientLibToken()).isEqualTo(jdbcToken); - options = SpannerOptions.newBuilder().setClientLibToken(hibernateToken).build(); + options = + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setClientLibToken(hibernateToken) + .build(); assertThat(options.getClientLibToken()).isEqualTo(hibernateToken); - options = SpannerOptions.newBuilder().build(); + options = SpannerOptions.newBuilder().setProjectId("[PROJECT]").build(); assertThat(options.getClientLibToken()).isEqualTo(ServiceOptions.getGoogApiClientLibName()); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java index 9b0dcdf4a79..34b8c8803a0 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -34,6 +35,8 @@ import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; +import com.google.spanner.v1.DatabaseName; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import java.util.Arrays; import java.util.Collections; @@ -189,6 +192,7 @@ public void commitAfterRollbackFails() { @Test public void usesPreparedTransaction() { SpannerOptions options = mock(SpannerOptions.class); + SpannerOptions.Builder builder = mock(SpannerOptions.Builder.class); when(options.getNumChannels()).thenReturn(4); GrpcTransportOptions transportOptions = mock(GrpcTransportOptions.class); when(transportOptions.getExecutorFactory()).thenReturn(new TestExecutorFactory()); @@ -196,6 +200,10 @@ public void usesPreparedTransaction() { SessionPoolOptions sessionPoolOptions = SessionPoolOptions.newBuilder().setMinSessions(0).build(); when(options.getSessionPoolOptions()).thenReturn(sessionPoolOptions); + when(options.getProjectId()).thenReturn("test"); + when(builder.build()).thenReturn(options); + when(options.toBuilder()).thenReturn(builder); + when(options.getClientLibToken()).thenReturn("test"); when(options.getSessionLabels()).thenReturn(Collections.emptyMap()); SpannerRpc rpc = mock(SpannerRpc.class); when(rpc.batchCreateSessions( @@ -205,9 +213,18 @@ public void usesPreparedTransaction() { @Override public List answer(InvocationOnMock invocation) throws Throwable { + DatabaseName databaseName = + DatabaseName.parse((String) invocation.getArguments()[0]); + String sessionName = + SessionName.of( + databaseName.getProject(), + databaseName.getInstance(), + databaseName.getDatabase(), + UUID.randomUUID().toString()) + .toString(); return Arrays.asList( com.google.spanner.v1.Session.newBuilder() - .setName((String) invocation.getArguments()[0]) + .setName(sessionName) .setCreateTime( com.google.protobuf.Timestamp.newBuilder() .setSeconds(System.currentTimeMillis() * 1000)) @@ -236,6 +253,9 @@ public CommitResponse answer(InvocationOnMock invocation) throws Throwable { .build(); } }); + when(rpc.getOptions()).thenReturn(options); + when(rpc.getRpc(any(SessionName.class))).thenReturn(rpc); + when(rpc.getRpc(any(DatabaseName.class))).thenReturn(rpc); DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java index 35cdefa0bb1..9a110947989 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -35,13 +36,16 @@ import com.google.protobuf.Timestamp; import com.google.rpc.Code; import com.google.rpc.RetryInfo; +import com.google.spanner.admin.instance.v1.ProjectName; import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.CommitRequest; import com.google.spanner.v1.CommitResponse; +import com.google.spanner.v1.DatabaseName; import com.google.spanner.v1.ExecuteBatchDmlRequest; import com.google.spanner.v1.ExecuteBatchDmlResponse; import com.google.spanner.v1.ResultSet; import com.google.spanner.v1.ResultSetStats; +import com.google.spanner.v1.SessionName; import com.google.spanner.v1.Transaction; import io.grpc.Metadata; import io.grpc.Status; @@ -105,8 +109,10 @@ public void usesPreparedTransaction() { SessionPoolOptions sessionPoolOptions = SessionPoolOptions.newBuilder().setMinSessions(0).build(); when(options.getSessionPoolOptions()).thenReturn(sessionPoolOptions); + when(options.getProjectId()).thenReturn(ProjectName.of("test").toString()); when(options.getSessionLabels()).thenReturn(Collections.emptyMap()); SpannerRpc rpc = mock(SpannerRpc.class); + when(rpc.getOptions()).thenReturn(options); when(rpc.batchCreateSessions( Mockito.anyString(), Mockito.eq(1), Mockito.anyMap(), Mockito.anyMap())) .thenAnswer( @@ -114,9 +120,18 @@ public void usesPreparedTransaction() { @Override public List answer(InvocationOnMock invocation) throws Throwable { + DatabaseName databaseName = + DatabaseName.parse((String) invocation.getArguments()[0]); + String sessionName = + SessionName.of( + databaseName.getProject(), + databaseName.getInstance(), + databaseName.getDatabase(), + UUID.randomUUID().toString()) + .toString(); return Arrays.asList( com.google.spanner.v1.Session.newBuilder() - .setName((String) invocation.getArguments()[0]) + .setName(sessionName) .setCreateTime( Timestamp.newBuilder().setSeconds(System.currentTimeMillis() * 1000)) .build()); @@ -143,6 +158,8 @@ public CommitResponse answer(InvocationOnMock invocation) throws Throwable { .build(); } }); + when(rpc.getRpc(any(DatabaseName.class))).thenReturn(rpc); + when(rpc.getRpc(any(SessionName.class))).thenReturn(rpc); DatabaseId db = DatabaseId.of("test", "test", "test"); try (SpannerImpl spanner = new SpannerImpl(rpc, options)) { DatabaseClient client = spanner.getDatabaseClient(db); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index 413799bc2c1..7d1f406414f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -136,6 +136,18 @@ public void startServer() throws IOException { mockInstanceAdmin = new MockInstanceAdminImpl(); mockDatabaseAdmin = new MockDatabaseAdminImpl(); + mockInstanceAdmin.addResponse( + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); + mockInstanceAdmin.addResponse( + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); address = new InetSocketAddress("localhost", 0); server = NettyServerBuilder.forAddress(address) @@ -240,6 +252,7 @@ public void testMultipleOpenSpanners() throws InterruptedException { // Create Spanner instance. SpannerOptions options = createSpannerOptions(); Spanner spanner = options.getService(); + mockInstanceAdmin.addResponse(Instance.getDefaultInstance()); spanners.add(spanner); // Get a database client and do a query. This should initiate threads for the Spanner service. DatabaseClient client = @@ -289,7 +302,7 @@ public CallCredentials getCallCredentials() { } }) .build(); - GapicSpannerRpc rpc = new GapicSpannerRpc(options); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, true); // GoogleAuthLibraryCallCredentials doesn't implement equals, so we can only check for the // existence. assertThat(rpc.newCallContext(optionsMap, "/some/resource").getCallOptions().getCredentials()) @@ -310,7 +323,7 @@ public CallCredentials getCallCredentials() { } }) .build(); - GapicSpannerRpc rpc = new GapicSpannerRpc(options); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, true); assertThat(rpc.newCallContext(optionsMap, "/some/resource").getCallOptions().getCredentials()) .isNull(); rpc.shutdown(); @@ -319,7 +332,7 @@ public CallCredentials getCallCredentials() { @Test public void testNoCallCredentials() { SpannerOptions options = SpannerOptions.newBuilder().setCredentials(STATIC_CREDENTIALS).build(); - GapicSpannerRpc rpc = new GapicSpannerRpc(options); + GapicSpannerRpc rpc = new GapicSpannerRpc(options, true); assertThat(rpc.newCallContext(optionsMap, "/some/resource").getCallOptions().getCredentials()) .isNull(); rpc.shutdown(); From 44ee82226c00e2742f9fd3e3afb4b8ca28ea0c21 Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Tue, 21 Jan 2020 17:29:53 -0800 Subject: [PATCH 2/7] reorder --- .../spanner/spi/v1/GapicSpannerRpcTest.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index 7d1f406414f..4e20894759e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -136,18 +136,6 @@ public void startServer() throws IOException { mockInstanceAdmin = new MockInstanceAdminImpl(); mockDatabaseAdmin = new MockDatabaseAdminImpl(); - mockInstanceAdmin.addResponse( - Instance.newBuilder() - .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) - .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) - .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) - .build()); - mockInstanceAdmin.addResponse( - Instance.newBuilder() - .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) - .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) - .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) - .build()); address = new InetSocketAddress("localhost", 0); server = NettyServerBuilder.forAddress(address) @@ -171,6 +159,18 @@ public ServerCall.Listener interceptCall( }) .build() .start(); + mockInstanceAdmin.addResponse( + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); + mockInstanceAdmin.addResponse( + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); optionsMap.put(Option.CHANNEL_HINT, Long.valueOf(1L)); } From aee69925f8565cab048169801d48b4ef437bb590 Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Wed, 22 Jan 2020 12:00:50 -0800 Subject: [PATCH 3/7] Applied formatting --- .../com/google/cloud/spanner/Instance.java | 1 - .../cloud/spanner/InstanceConfigRpcCache.java | 98 +++++++++---------- .../google/cloud/spanner/InstanceInfo.java | 2 - .../google/cloud/spanner/SessionClient.java | 2 +- .../com/google/cloud/spanner/SessionImpl.java | 14 ++- .../com/google/cloud/spanner/SpannerImpl.java | 1 - .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 82 +++++++++------- .../spanner/BatchCreateSessionsTest.java | 12 +-- .../cloud/spanner/DatabaseClientImplTest.java | 2 +- .../cloud/spanner/SessionClientTest.java | 5 +- .../google/cloud/spanner/SpannerImplTest.java | 3 +- .../cloud/spanner/SpannerOptionsTest.java | 10 +- .../spanner/TransactionManagerImplTest.java | 14 +-- .../spanner/TransactionRunnerImplTest.java | 14 +-- .../spanner/spi/v1/GapicSpannerRpcTest.java | 20 ++-- 15 files changed, 145 insertions(+), 135 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java index 886560de546..bf1fcdf5486 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/Instance.java @@ -21,7 +21,6 @@ import com.google.cloud.Policy; import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; - import java.util.List; import java.util.Map; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java index f9701ba4d90..3c234b4cab9 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java @@ -41,59 +41,59 @@ public class InstanceConfigRpcCache { public InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { this.projectClient = projectClient; cache = - CacheBuilder.newBuilder() - .build( - new CacheLoader() { - @Override - public SpannerRpc load(InstanceName instanceName) throws SpannerException { - GetInstanceRequest request = - GetInstanceRequest.newBuilder() - .setName(instanceName.toString()) - .setFieldMask(FieldMask.newBuilder().addPaths("endpoint_urls")) - .build(); - SpannerOptions.Builder optionsBuilder = projectClient.getOptions().toBuilder(); - try { - Instance instance = projectClient.getInstance(request); - if (instance.getEndpointUrisCount() > 0) { - optionsBuilder.setHost(instance.getEndpointUris(0)); - } else { - return projectClient; - } - } catch (SpannerException e) { - if (e.getErrorCode() == ErrorCode.UNIMPLEMENTED) { - // Ignore... - // This is backwards compatibility. - return projectClient; - } else { - logger.log( - Level.WARNING, - "Failed while resolving endpoint URLs for instance:" - + instanceName.toString() - + " reason: " - + e.getErrorCode().name()); - throw e; - } - } catch (Exception e) { - logger.log( - Level.WARNING, - "Failed while resolving endpoint URLs for instance:" - + instanceName.toString()); - throw SpannerExceptionFactory.newSpannerException(e); - } - return new GapicSpannerRpc(optionsBuilder.build(), false); - } - }); + CacheBuilder.newBuilder() + .build( + new CacheLoader() { + @Override + public SpannerRpc load(InstanceName instanceName) throws SpannerException { + GetInstanceRequest request = + GetInstanceRequest.newBuilder() + .setName(instanceName.toString()) + .setFieldMask(FieldMask.newBuilder().addPaths("endpoint_urls")) + .build(); + SpannerOptions.Builder optionsBuilder = projectClient.getOptions().toBuilder(); + try { + Instance instance = projectClient.getInstance(request); + if (instance.getEndpointUrisCount() > 0) { + optionsBuilder.setHost(instance.getEndpointUris(0)); + } else { + return projectClient; + } + } catch (SpannerException e) { + if (e.getErrorCode() == ErrorCode.UNIMPLEMENTED) { + // Ignore... + // This is backwards compatibility. + return projectClient; + } else { + logger.log( + Level.WARNING, + "Failed while resolving endpoint URLs for instance:" + + instanceName.toString() + + " reason: " + + e.getErrorCode().name()); + throw e; + } + } catch (Exception e) { + logger.log( + Level.WARNING, + "Failed while resolving endpoint URLs for instance:" + + instanceName.toString()); + throw SpannerExceptionFactory.newSpannerException(e); + } + return new GapicSpannerRpc(optionsBuilder.build(), false); + } + }); } public SpannerRpc get(SessionName sessionName) { InstanceName instanceName = - InstanceName.of(sessionName.getProject(), sessionName.getInstance()); + InstanceName.of(sessionName.getProject(), sessionName.getInstance()); return get(instanceName); } public SpannerRpc get(DatabaseName databaseName) { InstanceName instanceName = - InstanceName.of(databaseName.getProject(), databaseName.getInstance()); + InstanceName.of(databaseName.getProject(), databaseName.getInstance()); return get(instanceName); } @@ -103,11 +103,11 @@ private SpannerRpc get(InstanceName instanceName) { return spannerRpc; } catch (ExecutionException e) { logger.log( - Level.FINE, "Failed looking up instance in cache. id:" + instanceName.toString(), e); + Level.FINE, "Failed looking up instance in cache. id:" + instanceName.toString(), e); throw SpannerExceptionFactory.newSpannerException( - ErrorCode.NOT_FOUND, - "Failed getting RPC Client for Instance:" + instanceName.toString(), - e); + ErrorCode.NOT_FOUND, + "Failed getting RPC Client for Instance:" + instanceName.toString(), + e); } } @@ -118,4 +118,4 @@ public void invalidateAll() { } cache.invalidateAll(); } -} \ No newline at end of file +} diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java index 4c82e6a0a81..2fdd4983a68 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java @@ -23,7 +23,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.protobuf.FieldMask; - import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -174,7 +173,6 @@ public InstanceInfo build() { private final ImmutableMap labels; private final ImmutableList endpointUris; - InstanceInfo(BuilderImpl builder) { this.id = builder.id; this.configId = builder.configId; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java index 2516a46353e..4ca22803d98 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionClient.java @@ -204,7 +204,7 @@ SessionImpl createSession() { try (Scope s = SpannerImpl.tracer.withSpan(span)) { SpannerRpc spannerRpc = spanner.getRpc(DatabaseName.parse(db.getName())); com.google.spanner.v1.Session session = - spannerRpc.createSession(db.getName(), spanner.getOptions().getSessionLabels(), options); + spannerRpc.createSession(db.getName(), spanner.getOptions().getSessionLabels(), options); span.end(); return new SessionImpl(spanner, session.getName(), options); } catch (RuntimeException e) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index 3e960c7766a..4d6d5175d29 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -103,7 +103,7 @@ public String getName() { public long executePartitionedUpdate(Statement stmt) { setActive(null); PartitionedDMLTransaction txn = - new PartitionedDMLTransaction(this, spanner.getRpc(sessionName)); + new PartitionedDMLTransaction(this, spanner.getRpc(sessionName)); return txn.executePartitionedUpdate(stmt, spanner.getOptions().getPartitionedDmlTimeout()); } @@ -161,8 +161,8 @@ public ReadContext singleUse() { @Override public ReadContext singleUse(TimestampBound bound) { return setActive( - new SingleReadContext( - this, bound, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); + new SingleReadContext( + this, bound, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override @@ -192,7 +192,8 @@ public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) { @Override public TransactionRunner readWriteTransaction() { return setActive( - new TransactionRunnerImpl(this, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); + new TransactionRunnerImpl( + this, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks())); } @Override @@ -238,7 +239,10 @@ ByteString beginTransaction() { TransactionContextImpl newTransaction() { TransactionContextImpl txn = new TransactionContextImpl( - this, readyTransactionId, spanner.getRpc(sessionName), spanner.getDefaultPrefetchChunks()); + this, + readyTransactionId, + spanner.getRpc(sessionName), + spanner.getDefaultPrefetchChunks()); return txn; } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 2b70e26aee9..6d9e96075de 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -50,7 +50,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; -import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index d6927ad7719..4f55ae75ec0 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -529,7 +529,7 @@ public List batchCreateSessions( return get(getSpannerStub(DatabaseName.parse(databaseName)) .batchCreateSessionsCallable() .futureCall(request, context)) - .getSessionList(); + .getSessionList(); } @Override @@ -545,18 +545,21 @@ public Session createSession( CreateSessionRequest request = requestBuilder.build(); GrpcCallContext context = newCallContext(options, databaseName); return get( - getSpannerStub(DatabaseName.parse(databaseName)) - .createSessionCallable() - .futureCall(request, context)); } + getSpannerStub(DatabaseName.parse(databaseName)) + .createSessionCallable() + .futureCall(request, context)); + } @Override public void deleteSession(String sessionName, @Nullable Map options) throws SpannerException { DeleteSessionRequest request = DeleteSessionRequest.newBuilder().setName(sessionName).build(); GrpcCallContext context = newCallContext(options, sessionName); - get(getSpannerStub(SessionName.parse(sessionName)) - .deleteSessionCallable() - .futureCall(request, context)); } + get( + getSpannerStub(SessionName.parse(sessionName)) + .deleteSessionCallable() + .futureCall(request, context)); + } @Override public StreamingCall read( @@ -564,8 +567,9 @@ public StreamingCall read( GrpcCallContext context = newCallContext(options, request.getSession()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); getSpannerStub(SessionName.parse(request.getSession())) - .streamingReadCallable() - .call(request, responseObserver, context); final StreamController controller = responseObserver.getController(); + .streamingReadCallable() + .call(request, responseObserver, context); + final StreamController controller = responseObserver.getController(); return new StreamingCall() { @Override public void request(int numMessage) { @@ -585,18 +589,20 @@ public void cancel(String message) { public ResultSet executeQuery(ExecuteSqlRequest request, @Nullable Map options) { GrpcCallContext context = newCallContext(options, request.getSession()); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .executeSqlCallable() - .futureCall(request, context)); } + getSpannerStub(SessionName.parse(request.getSession())) + .executeSqlCallable() + .futureCall(request, context)); + } @Override public ResultSet executePartitionedDml( ExecuteSqlRequest request, @Nullable Map options, Duration timeout) { GrpcCallContext context = newCallContext(options, request.getSession(), timeout); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .executeSqlCallable() - .futureCall(request, context)); } + getSpannerStub(SessionName.parse(request.getSession())) + .executeSqlCallable() + .futureCall(request, context)); + } @Override public StreamingCall executeQuery( @@ -604,8 +610,8 @@ public StreamingCall executeQuery( GrpcCallContext context = newCallContext(options, request.getSession()); SpannerResponseObserver responseObserver = new SpannerResponseObserver(consumer); getSpannerStub(SessionName.parse(request.getSession())) - .executeStreamingSqlCallable() - .call(request, responseObserver, context); + .executeStreamingSqlCallable() + .call(request, responseObserver, context); final StreamController controller = responseObserver.getController(); return new StreamingCall() { @Override @@ -628,9 +634,9 @@ public ExecuteBatchDmlResponse executeBatchDml( GrpcCallContext context = newCallContext(options, request.getSession()); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .executeBatchDmlCallable() - .futureCall(request, context)); + getSpannerStub(SessionName.parse(request.getSession())) + .executeBatchDmlCallable() + .futureCall(request, context)); } @Override @@ -638,44 +644,50 @@ public Transaction beginTransaction( BeginTransactionRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .beginTransactionCallable() - .futureCall(request, context)); } + getSpannerStub(SessionName.parse(request.getSession())) + .beginTransactionCallable() + .futureCall(request, context)); + } @Override public CommitResponse commit(CommitRequest commitRequest, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, commitRequest.getSession()); return get( - getSpannerStub(SessionName.parse(commitRequest.getSession())) - .commitCallable() - .futureCall(commitRequest, context)); } + getSpannerStub(SessionName.parse(commitRequest.getSession())) + .commitCallable() + .futureCall(commitRequest, context)); + } @Override public void rollback(RollbackRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); - get(getSpannerStub(SessionName.parse(request.getSession())) - .rollbackCallable() - .futureCall(request, context)); } + get( + getSpannerStub(SessionName.parse(request.getSession())) + .rollbackCallable() + .futureCall(request, context)); + } @Override public PartitionResponse partitionQuery( PartitionQueryRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .partitionQueryCallable() - .futureCall(request, context)); } + getSpannerStub(SessionName.parse(request.getSession())) + .partitionQueryCallable() + .futureCall(request, context)); + } @Override public PartitionResponse partitionRead( PartitionReadRequest request, @Nullable Map options) throws SpannerException { GrpcCallContext context = newCallContext(options, request.getSession()); return get( - getSpannerStub(SessionName.parse(request.getSession())) - .partitionReadCallable() - .futureCall(request, context)); } + getSpannerStub(SessionName.parse(request.getSession())) + .partitionReadCallable() + .futureCall(request, context)); + } @Override public Policy getDatabaseAdminIAMPolicy(String resource) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java index 017163f7826..3a8013c21e7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java @@ -118,12 +118,12 @@ private Spanner createSpanner(int minSessions, int maxSessions) { .setMaxSessions(maxSessions) .build(); SpannerOptions options = - SpannerOptions.newBuilder() - .setProjectId("[PROJECT]") - .setChannelProvider(channelProvider) - .setSessionPoolOption(sessionPoolOptions) - .setCredentials(NoCredentials.getInstance()) - .build(); + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setChannelProvider(channelProvider) + .setSessionPoolOption(sessionPoolOptions) + .setCredentials(NoCredentials.getInstance()) + .build(); return options.getService(); } 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 9217d24290e..1e4a319c54a 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 @@ -123,7 +123,7 @@ public void setUp() throws IOException { .setProjectId("[PROJECT]") .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()) - .build(); + .build(); spanner = options.getService(); } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java index 78f59216244..f74e5e64e21 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionClientTest.java @@ -27,6 +27,8 @@ import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory; import com.google.cloud.spanner.SessionClient.SessionConsumer; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.spanner.v1.DatabaseName; +import com.google.spanner.v1.SessionName; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -39,9 +41,6 @@ import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; - -import com.google.spanner.v1.DatabaseName; -import com.google.spanner.v1.SessionName; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index b36450a76a7..de14be0504e 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -28,11 +28,10 @@ import com.google.cloud.ServiceRpc; import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.spanner.admin.instance.v1.ProjectName; import java.util.Collections; import java.util.HashMap; import java.util.Map; - -import com.google.spanner.admin.instance.v1.ProjectName; import org.junit.After; import org.junit.Before; import org.junit.Test; diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index ee4de804582..541db84dcfa 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -389,14 +389,14 @@ public void testSetClientLibToken() { final String jdbcToken = "sp-jdbc"; final String hibernateToken = "sp-hib"; SpannerOptions options = - SpannerOptions.newBuilder().setProjectId("[PROJECT]").setClientLibToken(jdbcToken).build(); + SpannerOptions.newBuilder().setProjectId("[PROJECT]").setClientLibToken(jdbcToken).build(); assertThat(options.getClientLibToken()).isEqualTo(jdbcToken); options = - SpannerOptions.newBuilder() - .setProjectId("[PROJECT]") - .setClientLibToken(hibernateToken) - .build(); + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setClientLibToken(hibernateToken) + .build(); assertThat(options.getClientLibToken()).isEqualTo(hibernateToken); options = SpannerOptions.newBuilder().setProjectId("[PROJECT]").build(); diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java index 34b8c8803a0..9a5ddb8f4bd 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionManagerImplTest.java @@ -214,14 +214,14 @@ public void usesPreparedTransaction() { public List answer(InvocationOnMock invocation) throws Throwable { DatabaseName databaseName = - DatabaseName.parse((String) invocation.getArguments()[0]); + DatabaseName.parse((String) invocation.getArguments()[0]); String sessionName = - SessionName.of( - databaseName.getProject(), - databaseName.getInstance(), - databaseName.getDatabase(), - UUID.randomUUID().toString()) - .toString(); + SessionName.of( + databaseName.getProject(), + databaseName.getInstance(), + databaseName.getDatabase(), + UUID.randomUUID().toString()) + .toString(); return Arrays.asList( com.google.spanner.v1.Session.newBuilder() .setName(sessionName) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java index 9a110947989..47cb8154363 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/TransactionRunnerImplTest.java @@ -121,14 +121,14 @@ public void usesPreparedTransaction() { public List answer(InvocationOnMock invocation) throws Throwable { DatabaseName databaseName = - DatabaseName.parse((String) invocation.getArguments()[0]); + DatabaseName.parse((String) invocation.getArguments()[0]); String sessionName = - SessionName.of( - databaseName.getProject(), - databaseName.getInstance(), - databaseName.getDatabase(), - UUID.randomUUID().toString()) - .toString(); + SessionName.of( + databaseName.getProject(), + databaseName.getInstance(), + databaseName.getDatabase(), + UUID.randomUUID().toString()) + .toString(); return Arrays.asList( com.google.spanner.v1.Session.newBuilder() .setName(sessionName) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java index 4e20894759e..531bf1f77cb 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpcTest.java @@ -160,17 +160,17 @@ public ServerCall.Listener interceptCall( .build() .start(); mockInstanceAdmin.addResponse( - Instance.newBuilder() - .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) - .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) - .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) - .build()); + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); mockInstanceAdmin.addResponse( - Instance.newBuilder() - .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) - .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) - .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) - .build()); + Instance.newBuilder() + .setName(InstanceName.format("[PROJECT]", "[INSTANCE]")) + .setConfig(InstanceConfigName.format("[PROJECT]", "[TEST-CONFIG]")) + .addEndpointUris("http://" + address.getHostString() + ":" + server.getPort()) + .build()); optionsMap.put(Option.CHANNEL_HINT, Long.valueOf(1L)); } From cf0d792e95a14c3422d3e68a868564dbb0cbfa55 Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Wed, 22 Jan 2020 12:07:51 -0800 Subject: [PATCH 4/7] Moved InstanceConfigRpcCache to v1 package per comment Changed to package private --- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 1 - .../{ => spi/v1}/InstanceConfigRpcCache.java | 18 ++++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) rename google-cloud-spanner/src/main/java/com/google/cloud/spanner/{ => spi/v1}/InstanceConfigRpcCache.java (90%) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 4f55ae75ec0..a15729fedbb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -37,7 +37,6 @@ import com.google.api.gax.rpc.WatchdogProvider; import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.cloud.spanner.InstanceConfigRpcCache; import com.google.cloud.spanner.SpannerException; import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerOptions; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java similarity index 90% rename from google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java rename to google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java index 3c234b4cab9..621bf2bbffd 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceConfigRpcCache.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java @@ -14,10 +14,12 @@ * limitations under the License. */ -package com.google.cloud.spanner; +package com.google.cloud.spanner.spi.v1; -import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; -import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.cloud.spanner.ErrorCode; +import com.google.cloud.spanner.SpannerException; +import com.google.cloud.spanner.SpannerExceptionFactory; +import com.google.cloud.spanner.SpannerOptions; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; @@ -31,14 +33,14 @@ import java.util.logging.Level; import java.util.logging.Logger; -public class InstanceConfigRpcCache { +class InstanceConfigRpcCache { private final LoadingCache cache; private Logger logger = Logger.getLogger(InstanceConfigRpcCache.class.getName()); private final GapicSpannerRpc projectClient; - public InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { + InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { this.projectClient = projectClient; cache = CacheBuilder.newBuilder() @@ -85,13 +87,13 @@ public SpannerRpc load(InstanceName instanceName) throws SpannerException { }); } - public SpannerRpc get(SessionName sessionName) { + SpannerRpc get(SessionName sessionName) { InstanceName instanceName = InstanceName.of(sessionName.getProject(), sessionName.getInstance()); return get(instanceName); } - public SpannerRpc get(DatabaseName databaseName) { + SpannerRpc get(DatabaseName databaseName) { InstanceName instanceName = InstanceName.of(databaseName.getProject(), databaseName.getInstance()); return get(instanceName); @@ -111,7 +113,7 @@ private SpannerRpc get(InstanceName instanceName) { } } - public void invalidateAll() { + void invalidateAll() { for (SpannerRpc rpc : cache.asMap().values()) { if (rpc == this.projectClient) continue; rpc.shutdown(); From db780c998cdaebe80c4db36b8a6fc06f92e4bbda Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Wed, 22 Jan 2020 12:16:18 -0800 Subject: [PATCH 5/7] Added feature flag --- .../google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java index 621bf2bbffd..4640322278e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java @@ -40,14 +40,20 @@ class InstanceConfigRpcCache { private Logger logger = Logger.getLogger(InstanceConfigRpcCache.class.getName()); private final GapicSpannerRpc projectClient; + private static final String RBR_ENABLED_FLAG = "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"; + + private final boolean rbrEnabled; + InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { this.projectClient = projectClient; + this.rbrEnabled = Boolean.parseBoolean(System.getProperty(RBR_ENABLED_FLAG, "false")); cache = CacheBuilder.newBuilder() .build( new CacheLoader() { @Override public SpannerRpc load(InstanceName instanceName) throws SpannerException { + if (!rbrEnabled) return projectClient; GetInstanceRequest request = GetInstanceRequest.newBuilder() .setName(instanceName.toString()) From 37d142411bbdd47349bd01fcef0ee51ef8a5ad4a Mon Sep 17 00:00:00 2001 From: Maor Bril Date: Wed, 29 Jan 2020 15:16:49 -0800 Subject: [PATCH 6/7] Adapted clirr --- .../clirr-ignored-differences.xml | 40 +++++++++++++++++++ .../google/cloud/spanner/InstanceInfo.java | 1 + .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 4 +- 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index bdef45796c9..ad00b6955c9 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -11,4 +11,44 @@ com/google/cloud/spanner/spi/v1/SpannerRpc * asyncDeleteSession(*) + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + * getRpc(*) + + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + * getStub() + + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + * getInstance(*) + + + 7012 + com/google/cloud/spanner/spi/v1/SpannerRpc + * getOptions() + + + 7004 + com/google/cloud/spanner/spi/v1/GapicSpannerRpc + * create(*) + + + 7004 + com/google/cloud/spanner/spi/v1/GapicSpannerRpc + GapicSpannerRpc(*) + + + 7013 + com/google/cloud/spanner/InstanceInfo$Builder + * addEndpointUri(*) + + + 7013 + com/google/cloud/spanner/InstanceInfo$Builder + * addAllEndpointUris(*) + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java index 2fdd4983a68..b3b3ed846bb 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceInfo.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkNotNull; +import com.google.api.core.InternalApi; import com.google.cloud.FieldSelector; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 95c0634bb46..c0b5c23de16 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -561,8 +561,8 @@ public ApiFuture asyncDeleteSession(String sessionName, @Nullable Map Date: Sun, 2 Feb 2020 21:08:32 -0800 Subject: [PATCH 7/7] Addressed comments per spec --- .../spanner/spi/v1/InstanceConfigRpcCache.java | 16 +++++++++++++--- .../cloud/spanner/BatchCreateSessionsTest.java | 7 +++---- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java index 4640322278e..3349055453e 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/InstanceConfigRpcCache.java @@ -1,5 +1,5 @@ /* - * Copyright 2019 Google LLC + * 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. @@ -40,10 +40,17 @@ class InstanceConfigRpcCache { private Logger logger = Logger.getLogger(InstanceConfigRpcCache.class.getName()); private final GapicSpannerRpc projectClient; - private static final String RBR_ENABLED_FLAG = "GOOGLE_CLOUD_ENABLE_RESOURCE_BASED_ROUTING"; + private static final String RBR_ENABLED_FLAG = "GOOGLE_CLOUD_SPANNER_ENABLE_RESOURCE_BASED_ROUTING"; private final boolean rbrEnabled; + private static final String PERMISSIONS_ERROR_MSG = "The client library attempted to connect to an endpoint closer to your Cloud " + + "Spanner data but was unable to do so. The client library will fall back and " + + "route requests to the endpoint given in the client options, which may result in " + + "increased latency. We recommend including the scope " + + "https://www.googleapis.com/auth/spanner.admin so that the client library can " + + "get an instance-specific endpoint and efficiently route requests."; + InstanceConfigRpcCache(final GapicSpannerRpc projectClient) { this.projectClient = projectClient; this.rbrEnabled = Boolean.parseBoolean(System.getProperty(RBR_ENABLED_FLAG, "false")); @@ -57,7 +64,7 @@ public SpannerRpc load(InstanceName instanceName) throws SpannerException { GetInstanceRequest request = GetInstanceRequest.newBuilder() .setName(instanceName.toString()) - .setFieldMask(FieldMask.newBuilder().addPaths("endpoint_urls")) + .setFieldMask(FieldMask.newBuilder().addPaths("endpoint_uris")) .build(); SpannerOptions.Builder optionsBuilder = projectClient.getOptions().toBuilder(); try { @@ -72,6 +79,9 @@ public SpannerRpc load(InstanceName instanceName) throws SpannerException { // Ignore... // This is backwards compatibility. return projectClient; + } else if (e.getErrorCode() == ErrorCode.PERMISSION_DENIED) { + logger.log(Level.WARNING, PERMISSIONS_ERROR_MSG); + return projectClient; } else { logger.log( Level.WARNING, diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java index 3a8013c21e7..195a389c7a6 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchCreateSessionsTest.java @@ -117,14 +117,13 @@ private Spanner createSpanner(int minSessions, int maxSessions) { .setMinSessions(minSessions) .setMaxSessions(maxSessions) .build(); - SpannerOptions options = - SpannerOptions.newBuilder() + return SpannerOptions.newBuilder() .setProjectId("[PROJECT]") .setChannelProvider(channelProvider) .setSessionPoolOption(sessionPoolOptions) .setCredentials(NoCredentials.getInstance()) - .build(); - return options.getService(); + .build() + .getService(); } @Test