From 365836e3b1451b46fff695552093c212479a4f47 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 6 Apr 2019 11:25:29 +0200 Subject: [PATCH 01/26] fixed merge conflicts --- .../main/java/com/google/cloud/spanner/SpannerImpl.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 5e60686c7e5b..79f73909cbf5 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -27,6 +27,13 @@ import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; +import com.google.cloud.Timestamp; +import com.google.cloud.spanner.AbstractReadContext.MultiUseReadOnlyTransaction; +import com.google.cloud.spanner.AbstractReadContext.SingleReadContext; +import com.google.cloud.spanner.AbstractReadContext.SingleUseReadOnlyTransaction; +import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl; +import com.google.cloud.spanner.Options.ListOption; +import com.google.cloud.spanner.SessionImpl.SessionTransaction; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.annotations.VisibleForTesting; From 1b975ae6e84804c9dbeeaa439c3afc3d5e0ad5db Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 12 Apr 2019 07:38:01 +0200 Subject: [PATCH 02/26] changed references after rebase on master --- .../main/java/com/google/cloud/spanner/SpannerImpl.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 79f73909cbf5..5e60686c7e5b 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -27,13 +27,6 @@ import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; -import com.google.cloud.Timestamp; -import com.google.cloud.spanner.AbstractReadContext.MultiUseReadOnlyTransaction; -import com.google.cloud.spanner.AbstractReadContext.SingleReadContext; -import com.google.cloud.spanner.AbstractReadContext.SingleUseReadOnlyTransaction; -import com.google.cloud.spanner.TransactionRunnerImpl.TransactionContextImpl; -import com.google.cloud.spanner.Options.ListOption; -import com.google.cloud.spanner.SessionImpl.SessionTransaction; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.annotations.VisibleForTesting; From a6a53421ffd60018bb97d666d37b9c94e5053c73 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sat, 13 Apr 2019 10:32:00 +0200 Subject: [PATCH 03/26] rebased on refactored SpannerImpl --- .../cloud/spanner/AbstractReadContext.java | 40 ++-- .../google/cloud/spanner/BatchClientImpl.java | 16 +- .../spanner/DatabaseAdminClientImpl.java | 27 ++- .../spanner/InstanceAdminClientImpl.java | 37 ++-- .../spanner/PartitionedDMLTransaction.java | 19 +- .../com/google/cloud/spanner/SessionImpl.java | 28 +-- .../com/google/cloud/spanner/SpannerImpl.java | 180 ++++++++++++++---- .../cloud/spanner/TransactionRunnerImpl.java | 26 +-- .../cloud/spanner/BatchClientImplTest.java | 27 +-- .../spanner/DatabaseAdminClientImplTest.java | 25 ++- .../spanner/InstanceAdminClientImplTest.java | 23 ++- .../google/cloud/spanner/SessionImplTest.java | 40 ++-- .../cloud/spanner/SpannerImplRetryTest.java | 106 +++++++---- .../google/cloud/spanner/SpannerImplTest.java | 27 +-- 14 files changed, 388 insertions(+), 233 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index ef37267d724e..aa7dc2b64cbc 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -65,8 +65,8 @@ static class SingleReadContext extends AbstractReadContext { private boolean used; SingleReadContext( - SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { - super(session, rpc, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { + super(session, spanner, defaultPrefetchChunks); this.bound = bound; } @@ -101,8 +101,8 @@ static class SingleUseReadOnlyTransaction extends SingleReadContext private Timestamp timestamp; SingleUseReadOnlyTransaction( - SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { - super(session, bound, rpc, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { + super(session, bound, spanner, defaultPrefetchChunks); } @Override @@ -150,8 +150,8 @@ static class MultiUseReadOnlyTransaction extends AbstractReadContext private ByteString transactionId; MultiUseReadOnlyTransaction( - SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { - super(session, rpc, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { + super(session, spanner, defaultPrefetchChunks); checkArgument( bound.getMode() != TimestampBound.Mode.MAX_STALENESS && bound.getMode() != TimestampBound.Mode.MIN_READ_TIMESTAMP, @@ -165,9 +165,9 @@ static class MultiUseReadOnlyTransaction extends AbstractReadContext SessionImpl session, ByteString transactionId, Timestamp timestamp, - SpannerRpc rpc, + SpannerImpl spanner, int defaultPrefetchChunks) { - super(session, rpc, defaultPrefetchChunks); + super(session, spanner, defaultPrefetchChunks); this.transactionId = transactionId; this.timestamp = timestamp; } @@ -227,13 +227,13 @@ void initTransaction() { .setOptions(options) .build(); Transaction transaction = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public Transaction call() throws Exception { - return rpc.beginTransaction(request, session.getOptions()); + return spanner.getRpc().beginTransaction(request, session.getOptions()); } - }); + }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); if (!transaction.hasReadTimestamp()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, "Missing expected transaction.read_timestamp metadata field"); @@ -261,7 +261,7 @@ public Transaction call() throws Exception { final Object lock = new Object(); final SessionImpl session; - final SpannerRpc rpc; + final SpannerImpl spanner; final Span span; private final int defaultPrefetchChunks; @@ -279,14 +279,14 @@ public Transaction call() throws Exception { // much more frequently. private static final int MAX_BUFFERED_CHUNKS = 512; - AbstractReadContext(SessionImpl session, SpannerRpc rpc, int defaultPrefetchChunks) { - this(session, rpc, defaultPrefetchChunks, Tracing.getTracer().getCurrentSpan()); + AbstractReadContext(SessionImpl session, SpannerImpl spanner, int defaultPrefetchChunks) { + this(session, spanner, defaultPrefetchChunks, Tracing.getTracer().getCurrentSpan()); } private AbstractReadContext( - SessionImpl session, SpannerRpc rpc, int defaultPrefetchChunks, Span span) { + SessionImpl session, SpannerImpl spanner, int defaultPrefetchChunks, Span span) { this.session = session; - this.rpc = rpc; + this.spanner = spanner; this.defaultPrefetchChunks = defaultPrefetchChunks; this.span = span; } @@ -418,7 +418,7 @@ ResultSet executeQueryInternalWithOptions( final int prefetchChunks = readOptions.hasPrefetchChunks() ? readOptions.prefetchChunks() : defaultPrefetchChunks; ResumableStreamIterator stream = - new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.QUERY, span) { + new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.QUERY.method, span) { @Override CloseableIterator startStream(@Nullable ByteString resumeToken) { GrpcStreamIterator stream = new GrpcStreamIterator(prefetchChunks); @@ -426,7 +426,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken request.setResumeToken(resumeToken); } SpannerRpc.StreamingCall call = - rpc.executeQuery(request.build(), stream.consumer(), session.getOptions()); + spanner.getRpc().executeQuery(request.build(), stream.consumer(), session.getOptions()); call.request(prefetchChunks); stream.setCall(call); return stream; @@ -525,7 +525,7 @@ ResultSet readInternalWithOptions( final int prefetchChunks = readOptions.hasPrefetchChunks() ? readOptions.prefetchChunks() : defaultPrefetchChunks; ResumableStreamIterator stream = - new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.READ, span) { + new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.READ.method, span) { @Override CloseableIterator startStream(@Nullable ByteString resumeToken) { GrpcStreamIterator stream = new GrpcStreamIterator(prefetchChunks); @@ -533,7 +533,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken builder.setResumeToken(resumeToken); } SpannerRpc.StreamingCall call = - rpc.read(builder.build(), stream.consumer(), session.getOptions()); + spanner.getRpc().read(builder.build(), stream.consumer(), session.getOptions()); call.request(prefetchChunks); stream.setCall(call); return stream; diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java index 3e19ee30e026..e0c60f96f62a 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java @@ -65,7 +65,7 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa super( checkNotNull(session), checkNotNull(bound), - checkNotNull(spanner).getOptions().getSpannerRpcV1(), + checkNotNull(spanner), spanner.getOptions().getPrefetchChunks()); this.sessionName = session.getName(); this.options = session.getOptions(); @@ -78,7 +78,7 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa checkNotNull(session), checkNotNull(batchTransactionId).getTransactionId(), batchTransactionId.getTimestamp(), - checkNotNull(spanner).getOptions().getSpannerRpcV1(), + checkNotNull(spanner), spanner.getOptions().getPrefetchChunks()); this.sessionName = session.getName(); this.options = session.getOptions(); @@ -136,13 +136,13 @@ public List partitionReadUsingIndex( final PartitionReadRequest request = builder.build(); PartitionResponse response = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public PartitionResponse call() throws Exception { - return rpc.partitionRead(request, options); + return spanner.getRpc().partitionRead(request, options); } - }); + }, SpannerImpl.READ.retryOnErrorCodes); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = @@ -181,13 +181,13 @@ public List partitionQuery( final PartitionQueryRequest request = builder.build(); PartitionResponse response = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public PartitionResponse call() throws Exception { - return rpc.partitionQuery(request, options); + return spanner.getRpc().partitionQuery(request, options); } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java index b9f7ce682046..7d0bedc1f0f3 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java @@ -24,7 +24,6 @@ import com.google.api.gax.paging.Page; import com.google.cloud.spanner.Options.ListOption; import com.google.cloud.spanner.SpannerImpl.PageFetcher; -import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.base.Preconditions; import com.google.protobuf.Empty; @@ -38,11 +37,11 @@ /** Default implementation of {@link DatabaseAdminClient}. */ class DatabaseAdminClientImpl implements DatabaseAdminClient { private final String projectId; - private final SpannerRpc rpc; + private final SpannerImpl spanner; - DatabaseAdminClientImpl(String projectId, SpannerRpc rpc) { + DatabaseAdminClientImpl(String projectId, SpannerImpl spanner) { this.projectId = projectId; - this.rpc = rpc; + this.spanner = spanner; } /** Generates a random operation id for long-running database operations. */ @@ -58,7 +57,7 @@ public OperationFuture createDatabase( String instanceName = getInstanceName(instanceId); String createStatement = "CREATE DATABASE `" + databaseId + "`"; OperationFuture - rawOperationFuture = rpc.createDatabase(instanceName, createStatement, statements); + rawOperationFuture = spanner.getRpc().createDatabase(instanceName, createStatement, statements); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), @@ -88,10 +87,10 @@ public Database getDatabase(String instanceId, String databaseId) throws Spanner new Callable() { @Override public Database call() throws Exception { - return Database.fromProto(rpc.getDatabase(dbName), DatabaseAdminClientImpl.this); + return Database.fromProto(spanner.getRpc().getDatabase(dbName), DatabaseAdminClientImpl.this); } }; - return SpannerImpl.runWithRetries(callable); + return spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -104,7 +103,7 @@ public OperationFuture updateDatabaseDdl( final String dbName = getDatabaseName(instanceId, databaseId); final String opId = operationId != null ? operationId : randomOperationId(); OperationFuture rawOperationFuture = - rpc.updateDatabaseDdl(dbName, statements, opId); + spanner.getRpc().updateDatabaseDdl(dbName, statements, opId); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), @@ -131,11 +130,11 @@ public void dropDatabase(String instanceId, String databaseId) throws SpannerExc new Callable() { @Override public Void call() throws Exception { - rpc.dropDatabase(dbName); + spanner.getRpc().dropDatabase(dbName); return null; } }; - SpannerImpl.runWithRetries(callable); + spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -145,10 +144,10 @@ public List getDatabaseDdl(String instanceId, String databaseId) { new Callable>() { @Override public List call() throws Exception { - return rpc.getDatabaseDdl(dbName); + return spanner.getRpc().getDatabaseDdl(dbName); } }; - return SpannerImpl.runWithRetries(callable); + return spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -159,11 +158,11 @@ public Page listDatabases(String instanceId, ListOption... options) { !listOptions.hasFilter(), "Filter option is not support by" + "listDatabases"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher() { + new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { @Override public Paginated getNextPage( String nextPageToken) { - return rpc.listDatabases(instanceName, pageSize, nextPageToken); + return spanner.getRpc().listDatabases(instanceName, pageSize, nextPageToken); } @Override diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java index e065d7f2ffb7..15b68de63f15 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java @@ -25,7 +25,6 @@ import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.spanner.Options.ListOption; import com.google.cloud.spanner.SpannerImpl.PageFetcher; -import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.base.Preconditions; import com.google.protobuf.FieldMask; @@ -39,25 +38,25 @@ class InstanceAdminClientImpl implements InstanceAdminClient { PathTemplate.create("projects/{project}"); private final DatabaseAdminClient dbClient; private final String projectId; - private final SpannerRpc rpc; + private final SpannerImpl spanner; - InstanceAdminClientImpl(String projectId, SpannerRpc rpc, DatabaseAdminClient dbClient) { + InstanceAdminClientImpl(String projectId, SpannerImpl spanner, DatabaseAdminClient dbClient) { this.projectId = projectId; - this.rpc = rpc; + this.spanner = spanner; this.dbClient = dbClient; } @Override public InstanceConfig getInstanceConfig(String configId) throws SpannerException { final String instanceConfigName = new InstanceConfigId(projectId, configId).getName(); - return SpannerImpl.runWithRetries( + return spanner.runWithRetries( new Callable() { @Override public InstanceConfig call() { return InstanceConfig.fromProto( - rpc.getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); + spanner.getRpc().getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -67,11 +66,11 @@ public Page listInstanceConfigs(ListOption... options) { !listOptions.hasFilter(), "Filter option is not supported by listInstanceConfigs"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher() { + new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { @Override public Paginated getNextPage( String nextPageToken) { - return rpc.listInstanceConfigs(pageSize, nextPageToken); + return spanner.getRpc().listInstanceConfigs(pageSize, nextPageToken); } @Override @@ -92,7 +91,7 @@ public OperationFuture createInstance(Instance String projectName = PROJECT_NAME_TEMPLATE.instantiate("project", projectId); OperationFuture rawOperationFuture = - rpc.createInstance(projectName, instance.getId().getInstance(), instance.toProto()); + spanner.getRpc().createInstance(projectName, instance.getId().getInstance(), instance.toProto()); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), @@ -120,14 +119,14 @@ public Instance apply(Exception e) { @Override public Instance getInstance(String instanceId) throws SpannerException { final String instanceName = new InstanceId(projectId, instanceId).getName(); - return SpannerImpl.runWithRetries( + return spanner.runWithRetries( new Callable() { @Override public Instance call() { return Instance.fromProto( - rpc.getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); + spanner.getRpc().getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -136,11 +135,11 @@ public Page listInstances(ListOption... options) throws SpannerExcepti final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; final String filter = listOptions.filter(); PageFetcher pageFetcher = - new PageFetcher() { + new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { @Override public Paginated getNextPage( String nextPageToken) { - return rpc.listInstances(pageSize, nextPageToken, filter); + return spanner.getRpc().listInstances(pageSize, nextPageToken, filter); } @Override @@ -156,14 +155,14 @@ public Instance fromProto(com.google.spanner.admin.instance.v1.Instance proto) { @Override public void deleteInstance(final String instanceId) throws SpannerException { - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public Void call() { - rpc.deleteInstance(new InstanceId(projectId, instanceId).getName()); + spanner.getRpc().deleteInstance(new InstanceId(projectId, instanceId).getName()); return null; } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Override @@ -175,7 +174,7 @@ public OperationFuture updateInstance( : InstanceInfo.InstanceField.toFieldMask(fieldsToUpdate); OperationFuture - rawOperationFuture = rpc.updateInstance(instance.toProto(), fieldMask); + rawOperationFuture = spanner.getRpc().updateInstance(instance.toProto(), fieldMask); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java index cefd128c499c..6823e629b674 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkState; import com.google.cloud.spanner.SessionImpl.SessionTransaction; -import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.ExecuteSqlRequest; @@ -34,12 +33,12 @@ class PartitionedDMLTransaction implements SessionTransaction { private final ByteString transactionId; private final SessionImpl session; - private final SpannerRpc rpc; + private final SpannerImpl spanner; private volatile boolean isValid = true; - PartitionedDMLTransaction(SessionImpl session, SpannerRpc rpc) { + PartitionedDMLTransaction(SessionImpl session, SpannerImpl spanner) { this.session = session; - this.rpc = rpc; + this.spanner = spanner; this.transactionId = initTransaction(); } @@ -52,13 +51,13 @@ private ByteString initTransaction() { .setPartitionedDml(TransactionOptions.PartitionedDml.getDefaultInstance())) .build(); Transaction txn = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public Transaction call() throws Exception { - return rpc.beginTransaction(request, session.getOptions()); + return spanner.getRpc().beginTransaction(request, session.getOptions()); } - }); + }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); if (txn.getId().isEmpty()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, @@ -84,13 +83,13 @@ long executePartitionedUpdate(Statement statement) { } } com.google.spanner.v1.ResultSet resultSet = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public com.google.spanner.v1.ResultSet call() throws Exception { - return rpc.executeQuery(builder.build(), session.getOptions()); + return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); } - }); + }, SpannerImpl.QUERY.retryOnErrorCodes); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "Partitioned DML response missing stats possibly due to non-DML statement as input"); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index 6b1314f49836..b8ffd21e141d 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -100,7 +100,7 @@ public String getName() { @Override public long executePartitionedUpdate(Statement stmt) { setActive(null); - PartitionedDMLTransaction txn = new PartitionedDMLTransaction(this, spanner.getRpc()); + PartitionedDMLTransaction txn = new PartitionedDMLTransaction(this, spanner); return txn.executePartitionedUpdate(stmt); } @@ -135,16 +135,16 @@ public Timestamp writeAtLeastOnce(Iterable mutations) throws SpannerEx TransactionOptions.newBuilder() .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance())) .build(); - Span span = tracer.spanBuilder(SpannerImpl.COMMIT).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.COMMIT.method).startSpan(); try (Scope s = tracer.withSpan(span)) { CommitResponse response = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public CommitResponse call() throws Exception { return spanner.getRpc().commit(request, options); } - }); + }, SpannerImpl.COMMIT.retryOnErrorCodes); Timestamp t = Timestamp.fromProto(response.getCommitTimestamp()); span.end(); return t; @@ -165,7 +165,7 @@ public ReadContext singleUse() { @Override public ReadContext singleUse(TimestampBound bound) { return setActive( - new SingleReadContext(this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + new SingleReadContext(this, bound, spanner, spanner.getDefaultPrefetchChunks())); } @Override @@ -177,7 +177,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() { public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) { return setActive( new SingleUseReadOnlyTransaction( - this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + this, bound, spanner, spanner.getDefaultPrefetchChunks())); } @Override @@ -189,7 +189,7 @@ public ReadOnlyTransaction readOnlyTransaction() { public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) { return setActive( new MultiUseReadOnlyTransaction( - this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); + this, bound, spanner, spanner.getDefaultPrefetchChunks())); } @Override @@ -206,16 +206,16 @@ public void prepareReadWriteTransaction() { @Override public void close() { - Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION.method).startSpan(); try (Scope s = tracer.withSpan(span)) { - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public Void call() throws Exception { spanner.getRpc().deleteSession(name, options); return null; } - }); + }, SpannerImpl.DELETE_SESSION.retryOnErrorCodes); span.end(); } catch (RuntimeException e) { TraceUtil.endSpanWithFailure(span, e); @@ -224,7 +224,7 @@ public Void call() throws Exception { } ByteString beginTransaction() { - Span span = tracer.spanBuilder(SpannerImpl.BEGIN_TRANSACTION).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.BEGIN_TRANSACTION.method).startSpan(); try (Scope s = tracer.withSpan(span)) { final BeginTransactionRequest request = BeginTransactionRequest.newBuilder() @@ -234,13 +234,13 @@ ByteString beginTransaction() { .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance())) .build(); Transaction txn = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public Transaction call() throws Exception { return spanner.getRpc().beginTransaction(request, options); } - }); + }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); if (txn.getId().isEmpty()) { throw newSpannerException(ErrorCode.INTERNAL, "Missing id in transaction\n" + getName()); } @@ -255,7 +255,7 @@ public Transaction call() throws Exception { TransactionContextImpl newTransaction() { TransactionContextImpl txn = new TransactionContextImpl( - this, readyTransactionId, spanner.getRpc(), spanner.getDefaultPrefetchChunks()); + this, readyTransactionId, spanner, spanner.getDefaultPrefetchChunks()); return txn; } diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 5e60686c7e5b..bdee01c85a65 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -20,10 +20,37 @@ import static com.google.cloud.spanner.SpannerExceptionFactory.newSpannerExceptionForCancellation; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; - +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.Set; +import java.util.concurrent.Callable; +import java.util.concurrent.CountDownLatch; +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; import com.google.api.client.util.BackOff; import com.google.api.client.util.ExponentialBackOff; +import com.google.api.core.ApiClock; import com.google.api.gax.paging.Page; +import com.google.api.gax.retrying.BasicResultRetryAlgorithm; +import com.google.api.gax.retrying.DirectRetryingExecutor; +import com.google.api.gax.retrying.ExponentialRetryAlgorithm; +import com.google.api.gax.retrying.ResultRetryAlgorithm; +import com.google.api.gax.retrying.RetryAlgorithm; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.retrying.RetryingExecutor; +import com.google.api.gax.retrying.RetryingFuture; +import com.google.api.gax.retrying.TimedRetryAlgorithm; +import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; @@ -34,6 +61,7 @@ import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import io.grpc.Context; @@ -42,22 +70,6 @@ import io.opencensus.trace.Span; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; -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; /** Default implementation of the Cloud Spanner interface. */ class SpannerImpl extends BaseService implements Spanner { @@ -67,20 +79,47 @@ class SpannerImpl extends BaseService implements Spanner { private static final Logger logger = Logger.getLogger(SpannerImpl.class.getName()); private static final Tracer tracer = Tracing.getTracer(); - private static final String CREATE_SESSION = "CloudSpannerOperation.CreateSession"; - static final String DELETE_SESSION = "CloudSpannerOperation.DeleteSession"; - static final String BEGIN_TRANSACTION = "CloudSpannerOperation.BeginTransaction"; - static final String COMMIT = "CloudSpannerOperation.Commit"; - static final String QUERY = "CloudSpannerOperation.ExecuteStreamingQuery"; - static final String READ = "CloudSpannerOperation.ExecuteStreamingRead"; + static final class RpcCall { + final String method; + final Set retryOnErrorCodes; + + private RpcCall(String method, Set retryOnErrorCodes) { + this.method = method; + this.retryOnErrorCodes = retryOnErrorCodes; + } + } + + static final Set DEFAULT_RETRY_ERROR_CODES = + Sets.newHashSet(ErrorCode.UNAVAILABLE, ErrorCode.DEADLINE_EXCEEDED); + static final Set NON_IDEMPOTENT_RETRY_ERROR_CODES = + Sets.newHashSet(ErrorCode.UNAVAILABLE); + static final RpcCall CREATE_SESSION = + new RpcCall("CloudSpannerOperation.CreateSession", DEFAULT_RETRY_ERROR_CODES); + static final RpcCall DELETE_SESSION = + new RpcCall("CloudSpannerOperation.DeleteSession", DEFAULT_RETRY_ERROR_CODES); + static final RpcCall BEGIN_TRANSACTION = + new RpcCall("CloudSpannerOperation.BeginTransaction", DEFAULT_RETRY_ERROR_CODES); + static final RpcCall COMMIT = + new RpcCall("CloudSpannerOperation.Commit", NON_IDEMPOTENT_RETRY_ERROR_CODES); + static final RpcCall QUERY = + new RpcCall("CloudSpannerOperation.ExecuteStreamingQuery", DEFAULT_RETRY_ERROR_CODES); + static final RpcCall READ = + new RpcCall("CloudSpannerOperation.ExecuteStreamingRead", DEFAULT_RETRY_ERROR_CODES); static { - TraceUtil.exportSpans(CREATE_SESSION, DELETE_SESSION, BEGIN_TRANSACTION, COMMIT, QUERY, READ); + TraceUtil.exportSpans( + CREATE_SESSION.method, + DELETE_SESSION.method, + BEGIN_TRANSACTION.method, + COMMIT.method, + QUERY.method, + READ.method); } private final Random random = new Random(); private final SpannerRpc gapicRpc; - private final int defaultPrefetchChunks; + private final RetrySettings retrySettings; + private final ApiClock clock; @GuardedBy("this") private final Map dbClients = new HashMap<>(); @@ -91,17 +130,18 @@ class SpannerImpl extends BaseService implements Spanner { @GuardedBy("this") private boolean spannerIsClosed = false; - SpannerImpl(SpannerRpc gapicRpc, int defaultPrefetchChunks, SpannerOptions options) { + @VisibleForTesting + SpannerImpl(SpannerRpc gapicRpc, SpannerOptions options) { super(options); this.gapicRpc = gapicRpc; - this.defaultPrefetchChunks = defaultPrefetchChunks; - this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); - this.instanceClient = - new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); + this.retrySettings = options.getRetrySettings(); + this.clock = options.getClock(); + this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), this); + this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), this, dbAdminClient); } SpannerImpl(SpannerOptions options) { - this(options.getSpannerRpcV1(), options.getPrefetchChunks(), options); + this(options.getSpannerRpcV1(), options); } static ExponentialBackOff newBackOff() { @@ -157,12 +197,33 @@ public void cancelled(Context context) { } } + private static class SpannerRetryAlgorithm extends BasicResultRetryAlgorithm { + private final Set retryOnErrorCodes; + + private SpannerRetryAlgorithm(Set retryOnErrorCodes) { + this.retryOnErrorCodes = retryOnErrorCodes; + } + + @Override + public boolean shouldRetry(Throwable prevThrowable, T prevResponse) { + if (prevThrowable != null) { + if (prevThrowable instanceof SpannerException) { + SpannerException spannerException = (SpannerException) prevThrowable; + if (retryOnErrorCodes.contains(spannerException.getErrorCode())) { + return spannerException.isRetryable(); + } + } + } + return false; + } + } + /** * Helper to execute some work, retrying with backoff on retryable errors. * *

TODO: Consider replacing with RetryHelper from gcloud-core. */ - static T runWithRetries(Callable callable) { + T runWithRetries(Callable callable, Set retryOnErrorCodes) { // Use same backoff setting as abort, somewhat arbitrarily. Span span = tracer.getCurrentSpan(); ExponentialBackOff backOff = newBackOff(); @@ -174,8 +235,8 @@ static T runWithRetries(Callable callable) { span.addAnnotation( "Starting operation", ImmutableMap.of("Attempt", AttributeValue.longAttributeValue(attempt))); - T result = callable.call(); - return result; + return runWithRetries( + callable, retrySettings, new SpannerRetryAlgorithm<>(retryOnErrorCodes), clock); } catch (SpannerException e) { if (!e.isRetryable()) { throw e; @@ -194,6 +255,36 @@ static T runWithRetries(Callable callable) { } } + private static V runWithRetries( + Callable callable, + RetrySettings retrySettings, + ResultRetryAlgorithm resultRetryAlgorithm, + ApiClock clock) { + try { + @SuppressWarnings("unchecked") + ResultRetryAlgorithm algorithm = (ResultRetryAlgorithm) resultRetryAlgorithm; + return run(callable, new ExponentialRetryAlgorithm(retrySettings, clock), algorithm); + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e.getCause()); + } catch (InterruptedException e) { + throw newSpannerException(ErrorCode.CANCELLED, "Operation cancelled.", e); + } + } + + private static V run( + Callable callable, + TimedRetryAlgorithm timedAlgorithm, + ResultRetryAlgorithm resultAlgorithm) + throws ExecutionException, InterruptedException { + RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); + RetryingExecutor executor = new DirectRetryingExecutor<>(retryAlgorithm); + + RetryingFuture retryingFuture = executor.createFuture(callable); + executor.submit(retryingFuture); + return retryingFuture.get(); + } + /** Returns the {@link SpannerRpc} of this {@link SpannerImpl} instance. */ SpannerRpc getRpc() { return gapicRpc; @@ -201,13 +292,13 @@ SpannerRpc getRpc() { /** Returns the default setting for prefetchChunks of this {@link SpannerImpl} instance. */ int getDefaultPrefetchChunks() { - return defaultPrefetchChunks; + return getOptions().getPrefetchChunks(); } SessionImpl createSession(final DatabaseId db) throws SpannerException { final Map options = optionMap(SessionOption.channelHint(random.nextLong())); - Span span = tracer.spanBuilder(CREATE_SESSION).startSpan(); + Span span = tracer.spanBuilder(CREATE_SESSION.method).startSpan(); try (Scope s = tracer.withSpan(span)) { com.google.spanner.v1.Session session = runWithRetries( @@ -217,7 +308,7 @@ public com.google.spanner.v1.Session call() throws Exception { return gapicRpc.createSession( db.getName(), getOptions().getSessionLabels(), options); } - }); + }, CREATE_SESSION.retryOnErrorCodes); span.end(); return new SessionImpl(this, session.getName(), options); } catch (RuntimeException e) { @@ -339,19 +430,28 @@ Object value() { return ImmutableMap.copyOf(tmp); } + + /** Helper class for gRPC calls that can return paginated results. */ abstract static class PageFetcher implements NextPageFetcher { + private final SpannerImpl spanner; + private final Set retryOnErrorCodes; private String nextPageToken; + PageFetcher(SpannerImpl spanner, Set retryOnErrorCodes) { + this.spanner = spanner; + this.retryOnErrorCodes = retryOnErrorCodes; + } + @Override public Page getNextPage() { Paginated nextPage = - runWithRetries( + spanner.runWithRetries( new Callable>() { @Override public Paginated call() { return getNextPage(nextPageToken); } - }); + }, retryOnErrorCodes); this.nextPageToken = nextPage.getNextPageToken(); List results = new ArrayList<>(); for (T proto : nextPage.getResults()) { @@ -377,4 +477,4 @@ public void execute(Runnable command) { command.run(); } } -} +} \ No newline at end of file diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index 7d95d33b9c2f..ad0e2d11f854 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -73,9 +73,9 @@ static class TransactionContextImpl extends AbstractReadContext implements Trans TransactionContextImpl( SessionImpl session, @Nullable ByteString transactionId, - SpannerRpc rpc, + SpannerImpl spanner, int defaultPrefetchChunks) { - super(session, rpc, defaultPrefetchChunks); + super(session, spanner, defaultPrefetchChunks); this.transactionId = transactionId; } @@ -122,16 +122,16 @@ void commit() { mutations = null; } final CommitRequest commitRequest = builder.build(); - Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT, span).startSpan(); + Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT.method, span).startSpan(); try (Scope s = tracer.withSpan(opSpan)) { CommitResponse commitResponse = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public CommitResponse call() throws Exception { - return rpc.commit(commitRequest, session.getOptions()); + return spanner.getRpc().commit(commitRequest, session.getOptions()); } - }); + }, SpannerImpl.COMMIT.retryOnErrorCodes); if (!commitResponse.hasCommitTimestamp()) { throw newSpannerException( @@ -178,7 +178,7 @@ void rollback() { // response. Normally, the next thing that will happen is that we will make a fresh // transaction attempt, which should implicitly abort this one. span.addAnnotation("Starting Rollback"); - rpc.rollback( + spanner.getRpc().rollback( RollbackRequest.newBuilder() .setSession(session.getName()) .setTransactionId(transactionId) @@ -239,13 +239,13 @@ public long executeUpdate(Statement statement) { final ExecuteSqlRequest.Builder builder = getExecuteSqlRequestBuilder(statement, QueryMode.NORMAL); com.google.spanner.v1.ResultSet resultSet = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public com.google.spanner.v1.ResultSet call() throws Exception { - return rpc.executeQuery(builder.build(), session.getOptions()); + return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); } - }); + }, SpannerImpl.QUERY.retryOnErrorCodes); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "DML response missing stats possibly due to non-DML statement as input"); @@ -259,13 +259,13 @@ public long[] batchUpdate(Iterable statements) { beforeReadOrQuery(); final ExecuteBatchDmlRequest.Builder builder = getExecuteBatchDmlRequestBuilder(statements); com.google.spanner.v1.ExecuteBatchDmlResponse response = - SpannerImpl.runWithRetries( + spanner.runWithRetries( new Callable() { @Override public com.google.spanner.v1.ExecuteBatchDmlResponse call() throws Exception { - return rpc.executeBatchDml(builder.build(), session.getOptions()); + return spanner.getRpc().executeBatchDml(builder.build(), session.getOptions()); } - }); + }, SpannerImpl.QUERY.retryOnErrorCodes); long[] results = new long[response.getResultSetsCount()]; for (int i = 0; i < response.getResultSetsCount(); ++i) { results[i] = response.getResultSets(i).getStats().getRowCountExact(); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index 395e11cbea7b..b0ab15e981c0 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -17,18 +17,10 @@ package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.anyMap; -import static org.mockito.Mockito.eq; +import static org.mockito.Matchers.anyMap; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; - -import com.google.cloud.Timestamp; -import com.google.cloud.spanner.spi.v1.SpannerRpc; -import com.google.protobuf.ByteString; -import com.google.protobuf.util.Timestamps; -import com.google.spanner.v1.BeginTransactionRequest; -import com.google.spanner.v1.Session; -import com.google.spanner.v1.Transaction; import java.util.Map; import org.junit.Before; import org.junit.Test; @@ -38,6 +30,15 @@ import org.mockito.Captor; import org.mockito.Mock; import org.mockito.Mockito; +import com.google.api.core.NanoClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.Timestamp; +import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.protobuf.ByteString; +import com.google.protobuf.util.Timestamps; +import com.google.spanner.v1.BeginTransactionRequest; +import com.google.spanner.v1.Session; +import com.google.spanner.v1.Transaction; /** Unit tests for {@link com.google.cloud.spanner.BatchClientImpl}. */ @RunWith(JUnit4.class) @@ -59,8 +60,10 @@ public final class BatchClientImplTest { public void setUp() { initMocks(this); DatabaseId db = DatabaseId.of(DB_NAME); - SpannerImpl spanner = new SpannerImpl(gapicRpc, 1, spannerOptions); - client = new BatchClientImpl(db, spanner); + when(spannerOptions.getPrefetchChunks()).thenReturn(1); + when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); + when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); + SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); client = new BatchClientImpl(db, spanner); } @Test diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java index 3315faa8d03c..d1e971adf3f0 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java @@ -20,8 +20,17 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; - +import java.util.Collections; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; +import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.retrying.RetrySettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.collect.ImmutableList; @@ -32,13 +41,6 @@ import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.database.v1.Database; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; -import java.util.Collections; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Mock; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.DatabaseAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -57,8 +59,11 @@ public class DatabaseAdminClientImplTest { @Before public void setUp() { initMocks(this); - client = new DatabaseAdminClientImpl(PROJECT_ID, rpc); - } + SpannerOptions spannerOptions = Mockito.mock(SpannerOptions.class); + when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); + when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); + SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); + client = new DatabaseAdminClientImpl(PROJECT_ID, spanner); } private Database getDatabaseProto() { return Database.newBuilder().setName(DB_NAME).setState(Database.State.READY).build(); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java index 0170ada64bd9..fa01a252fa05 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java @@ -20,8 +20,16 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; - +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; +import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.retrying.RetrySettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.collect.ImmutableList; @@ -30,12 +38,6 @@ import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; import com.google.spanner.admin.instance.v1.InstanceConfig; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Mock; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.InstanceAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -55,8 +57,11 @@ public class InstanceAdminClientImplTest { @Before public void setUp() { initMocks(this); - client = new InstanceAdminClientImpl(PROJECT_ID, rpc, dbClient); - } + SpannerOptions spannerOptions = Mockito.mock(SpannerOptions.class); + when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); + when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); + SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); + client = new InstanceAdminClientImpl(PROJECT_ID, spanner, dbClient); } @Test public void getInstanceConfig() { diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java index 919a0026ed31..ad62c2f3667a 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java @@ -18,22 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; - -import com.google.cloud.Timestamp; -import com.google.cloud.spanner.TransactionRunner.TransactionCallable; -import com.google.cloud.spanner.spi.v1.SpannerRpc; -import com.google.protobuf.ByteString; -import com.google.protobuf.ListValue; -import com.google.protobuf.util.Timestamps; -import com.google.spanner.v1.BeginTransactionRequest; -import com.google.spanner.v1.CommitRequest; -import com.google.spanner.v1.CommitResponse; -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.Transaction; +import static org.mockito.Mockito.when; import java.text.ParseException; import java.util.Arrays; import java.util.Calendar; @@ -55,6 +40,23 @@ import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; +import com.google.api.core.NanoClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.Timestamp; +import com.google.cloud.spanner.TransactionRunner.TransactionCallable; +import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.protobuf.ByteString; +import com.google.protobuf.ListValue; +import com.google.protobuf.util.Timestamps; +import com.google.spanner.v1.BeginTransactionRequest; +import com.google.spanner.v1.CommitRequest; +import com.google.spanner.v1.CommitResponse; +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.Transaction; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.SessionImpl}. */ @RunWith(JUnit4.class) @@ -71,9 +73,11 @@ public class SessionImplTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); + when(spannerOptions.getPrefetchChunks()).thenReturn(1); + when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); + when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); @SuppressWarnings("resource") - SpannerImpl spanner = new SpannerImpl(rpc, 1, spannerOptions); - String dbName = "projects/p1/instances/i1/databases/d1"; + SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); String dbName = "projects/p1/instances/i1/databases/d1"; String sessionName = dbName + "/sessions/s1"; DatabaseId db = DatabaseId.of(dbName); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java index 247fe44e2c92..f2c5d5842a56 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java @@ -18,7 +18,11 @@ import static com.google.cloud.spanner.SpannerMatchers.isSpannerException; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.mock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import io.grpc.Context; import java.util.concurrent.Callable; import java.util.concurrent.Executors; @@ -32,10 +36,13 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mockito; +import org.threeten.bp.Duration; /** Unit tests for {@link SpannerImpl#runWithRetries}. */ @RunWith(JUnit4.class) public class SpannerImplRetryTest { + private static final String PROJECT_ID = "Test-Project"; + interface StringCallable extends Callable { @Override String call(); @@ -55,18 +62,26 @@ static class NonRetryableException extends SpannerException { } @Rule public ExpectedException expectedException = ExpectedException.none(); - + SpannerImpl spanner; StringCallable callable; @Before public void setUp() { callable = Mockito.mock(StringCallable.class); + SpannerOptions options = + SpannerOptions.newBuilder() + .setCredentials(NoCredentials.getInstance()) + .setProjectId(PROJECT_ID) + .build(); + SpannerRpc rpc = mock(SpannerRpc.class); + spanner = new SpannerImpl(rpc, options); } @Test public void ok() { Mockito.when(callable.call()).thenReturn("r"); - assertThat(SpannerImpl.runWithRetries(callable)).isEqualTo("r"); + assertThat(spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES)) + .isEqualTo("r"); } @Test @@ -74,7 +89,7 @@ public void nonRetryableFailure() { Mockito.when(callable.call()) .thenThrow(new NonRetryableException(ErrorCode.FAILED_PRECONDITION, "Failed by test")); expectedException.expect(isSpannerException(ErrorCode.FAILED_PRECONDITION)); - SpannerImpl.runWithRetries(callable); + spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Test @@ -84,7 +99,8 @@ public void retryableFailure() { .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #2")) .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #3")) .thenReturn("r"); - assertThat(SpannerImpl.runWithRetries(callable)).isEqualTo("r"); + assertThat(spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES)) + .isEqualTo("r"); } @Test @@ -95,43 +111,63 @@ public void retryableFailureFollowedByPermanentFailure() { .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #3")) .thenThrow(new NonRetryableException(ErrorCode.FAILED_PRECONDITION, "Failed by test")); expectedException.expect(isSpannerException(ErrorCode.FAILED_PRECONDITION)); - SpannerImpl.runWithRetries(callable); + spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } @Test public void contextCancelled() { - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); - Context.CancellableContext context = Context.current().withCancellation(); - Runnable work = - context.wrap( - new Runnable() { - @Override - public void run() { - SpannerImpl.runWithRetries(callable); - } - }); - context.cancel(new RuntimeException("Cancelled by test")); - expectedException.expect(isSpannerException(ErrorCode.CANCELLED)); - work.run(); + SpannerOptions options = + SpannerOptions.newBuilder() + .setCredentials(NoCredentials.getInstance()) + .setProjectId(PROJECT_ID) + .setRetrySettings( + RetrySettings.newBuilder().setTotalTimeout(Duration.ofMillis(5L)).build()) + .build(); + SpannerRpc rpc = mock(SpannerRpc.class); + try (SpannerImpl contextSpanner = new SpannerImpl(rpc, options)) { + Mockito.when(callable.call()) + .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); + Context.CancellableContext context = Context.current().withCancellation(); + Runnable work = + context.wrap( + new Runnable() { + @Override + public void run() { + contextSpanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + } + }); + context.cancel(new RuntimeException("Cancelled by test")); + expectedException.expect(isSpannerException(ErrorCode.CANCELLED)); + work.run(); + } } @Test public void contextDeadlineExceeded() { - ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); - Context.CancellableContext context = - Context.current().withDeadlineAfter(10, TimeUnit.NANOSECONDS, executor); - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); - Runnable work = - context.wrap( - new Runnable() { - @Override - public void run() { - SpannerImpl.runWithRetries(callable); - } - }); - expectedException.expect(isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); - work.run(); + SpannerOptions options = + SpannerOptions.newBuilder() + .setCredentials(NoCredentials.getInstance()) + .setProjectId(PROJECT_ID) + .setRetrySettings( + RetrySettings.newBuilder().setTotalTimeout(Duration.ofMillis(5L)).build()) + .build(); + SpannerRpc rpc = mock(SpannerRpc.class); + try (SpannerImpl contextSpanner = new SpannerImpl(rpc, options)) { + ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); + Context.CancellableContext context = + Context.current().withDeadlineAfter(10, TimeUnit.MILLISECONDS, executor); + Mockito.when(callable.call()) + .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); + Runnable work = + context.wrap( + new Runnable() { + @Override + public void run() { + contextSpanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + } + }); + expectedException.expect(isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); + work.run(); + } } -} +} \ No newline at end of file diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 8fe6e5c91d7b..38e79becf0cc 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -20,9 +20,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; - -import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.cloud.spanner.spi.v1.SpannerRpc; +import static org.mockito.Mockito.when; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Callable; @@ -36,6 +34,10 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import com.google.api.core.NanoClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.spanner.spi.v1.SpannerRpc; /** Unit tests for {@link SpannerImpl}. */ @RunWith(JUnit4.class) @@ -49,7 +51,10 @@ public class SpannerImplTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); - impl = new SpannerImpl(rpc, 1, spannerOptions); + when(spannerOptions.getPrefetchChunks()).thenReturn(1); + when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); + when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); + impl = new SpannerImpl(rpc, spannerOptions); } @Test @@ -99,7 +104,7 @@ public void getDbclientAgainGivesSame() { @Test public void getDbclientAfterCloseThrows() { - SpannerImpl imp = new SpannerImpl(rpc, 1, spannerOptions); + SpannerImpl imp = new SpannerImpl(rpc, spannerOptions); Map labels = new HashMap<>(); labels.put("env", "dev"); Mockito.when(spannerOptions.getSessionLabels()).thenReturn(labels); @@ -124,13 +129,13 @@ public void getDbclientAfterCloseThrows() { @Test public void exceptionIsTranslated() { try { - SpannerImpl.runWithRetries( + impl.runWithRetries( new Callable() { @Override public Void call() throws Exception { throw new Exception("Should be translated to SpannerException"); } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } catch (SpannerException e) { assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL); assertThat(e.getMessage().contains("Unexpected exception thrown")); @@ -143,7 +148,7 @@ public void sslHandshakeExceptionIsNotRetryable() { // retryable. boolean gotExpectedException = false; try { - SpannerImpl.runWithRetries( + impl.runWithRetries( new Callable() { @Override public Void call() throws Exception { @@ -152,7 +157,7 @@ public Void call() throws Exception { "This exception should not be retryable", new SSLHandshakeException("some SSL handshake exception")); } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } catch (SpannerException e) { gotExpectedException = true; assertThat(e.isRetryable(), is(false)); @@ -162,7 +167,7 @@ public Void call() throws Exception { assertThat(gotExpectedException, is(true)); // Verify that any other SpannerException with code UNAVAILABLE is retryable. - SpannerImpl.runWithRetries( + impl.runWithRetries( new Callable() { private boolean firstTime = true; @@ -179,6 +184,6 @@ public Void call() throws Exception { } return null; } - }); + }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); } } From 2d85dc31fa739ba4738bdef047ac87dcc522b1df Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Sun, 14 Apr 2019 18:06:12 +0200 Subject: [PATCH 04/26] enable gax retries on rpc calls --- .../cloud/spanner/AbstractReadContext.java | 6 +- .../google/cloud/spanner/BatchClientImpl.java | 4 +- .../spanner/DatabaseAdminClientImpl.java | 8 +- .../spanner/InstanceAdminClientImpl.java | 10 +- .../spanner/PartitionedDMLTransaction.java | 4 +- .../com/google/cloud/spanner/SessionImpl.java | 12 +- .../com/google/cloud/spanner/SpannerImpl.java | 139 ++----- .../google/cloud/spanner/SpannerOptions.java | 29 ++ .../cloud/spanner/TransactionRunnerImpl.java | 8 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 69 ++-- .../spanner/SpannerImplGaxRetryTest.java | 352 ++++++++++++++++++ .../cloud/spanner/SpannerImplRetryTest.java | 173 --------- .../google/cloud/spanner/SpannerImplTest.java | 47 +-- 13 files changed, 476 insertions(+), 385 deletions(-) create mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java delete mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index aa7dc2b64cbc..3a099baa8c4f 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -233,7 +233,7 @@ void initTransaction() { public Transaction call() throws Exception { return spanner.getRpc().beginTransaction(request, session.getOptions()); } - }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); + }); if (!transaction.hasReadTimestamp()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, "Missing expected transaction.read_timestamp metadata field"); @@ -418,7 +418,7 @@ ResultSet executeQueryInternalWithOptions( final int prefetchChunks = readOptions.hasPrefetchChunks() ? readOptions.prefetchChunks() : defaultPrefetchChunks; ResumableStreamIterator stream = - new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.QUERY.method, span) { + new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.QUERY, span) { @Override CloseableIterator startStream(@Nullable ByteString resumeToken) { GrpcStreamIterator stream = new GrpcStreamIterator(prefetchChunks); @@ -525,7 +525,7 @@ ResultSet readInternalWithOptions( final int prefetchChunks = readOptions.hasPrefetchChunks() ? readOptions.prefetchChunks() : defaultPrefetchChunks; ResumableStreamIterator stream = - new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.READ.method, span) { + new ResumableStreamIterator(MAX_BUFFERED_CHUNKS, SpannerImpl.READ, span) { @Override CloseableIterator startStream(@Nullable ByteString resumeToken) { GrpcStreamIterator stream = new GrpcStreamIterator(prefetchChunks); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java index e0c60f96f62a..d6f6215cdbbc 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java @@ -142,7 +142,7 @@ public List partitionReadUsingIndex( public PartitionResponse call() throws Exception { return spanner.getRpc().partitionRead(request, options); } - }, SpannerImpl.READ.retryOnErrorCodes); + }); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = @@ -187,7 +187,7 @@ public List partitionQuery( public PartitionResponse call() throws Exception { return spanner.getRpc().partitionQuery(request, options); } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + }); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java index 7d0bedc1f0f3..4678fac6a2cd 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java @@ -90,7 +90,7 @@ public Database call() throws Exception { return Database.fromProto(spanner.getRpc().getDatabase(dbName), DatabaseAdminClientImpl.this); } }; - return spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + return spanner.runWithRetries(callable); } @Override @@ -134,7 +134,7 @@ public Void call() throws Exception { return null; } }; - spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + spanner.runWithRetries(callable); } @Override @@ -147,7 +147,7 @@ public List call() throws Exception { return spanner.getRpc().getDatabaseDdl(dbName); } }; - return spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + return spanner.runWithRetries(callable); } @Override @@ -158,7 +158,7 @@ public Page listDatabases(String instanceId, ListOption... options) { !listOptions.hasFilter(), "Filter option is not support by" + "listDatabases"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { + new PageFetcher(spanner) { @Override public Paginated getNextPage( String nextPageToken) { diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java index 15b68de63f15..90b65c25fb51 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java @@ -56,7 +56,7 @@ public InstanceConfig call() { return InstanceConfig.fromProto( spanner.getRpc().getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + }); } @Override @@ -66,7 +66,7 @@ public Page listInstanceConfigs(ListOption... options) { !listOptions.hasFilter(), "Filter option is not supported by listInstanceConfigs"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { + new PageFetcher(spanner) { @Override public Paginated getNextPage( String nextPageToken) { @@ -126,7 +126,7 @@ public Instance call() { return Instance.fromProto( spanner.getRpc().getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + }); } @Override @@ -135,7 +135,7 @@ public Page listInstances(ListOption... options) throws SpannerExcepti final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; final String filter = listOptions.filter(); PageFetcher pageFetcher = - new PageFetcher(spanner, SpannerImpl.DEFAULT_RETRY_ERROR_CODES) { + new PageFetcher(spanner) { @Override public Paginated getNextPage( String nextPageToken) { @@ -162,7 +162,7 @@ public Void call() { spanner.getRpc().deleteInstance(new InstanceId(projectId, instanceId).getName()); return null; } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + }); } @Override diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java index 6823e629b674..237af748c9c7 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java @@ -57,7 +57,7 @@ private ByteString initTransaction() { public Transaction call() throws Exception { return spanner.getRpc().beginTransaction(request, session.getOptions()); } - }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); + }); if (txn.getId().isEmpty()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, @@ -89,7 +89,7 @@ long executePartitionedUpdate(Statement statement) { public com.google.spanner.v1.ResultSet call() throws Exception { return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); } - }, SpannerImpl.QUERY.retryOnErrorCodes); + }); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "Partitioned DML response missing stats possibly due to non-DML statement as input"); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index b8ffd21e141d..e446782b76c9 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -135,7 +135,7 @@ public Timestamp writeAtLeastOnce(Iterable mutations) throws SpannerEx TransactionOptions.newBuilder() .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance())) .build(); - Span span = tracer.spanBuilder(SpannerImpl.COMMIT.method).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.COMMIT).startSpan(); try (Scope s = tracer.withSpan(span)) { CommitResponse response = spanner.runWithRetries( @@ -144,7 +144,7 @@ public Timestamp writeAtLeastOnce(Iterable mutations) throws SpannerEx public CommitResponse call() throws Exception { return spanner.getRpc().commit(request, options); } - }, SpannerImpl.COMMIT.retryOnErrorCodes); + }); Timestamp t = Timestamp.fromProto(response.getCommitTimestamp()); span.end(); return t; @@ -206,7 +206,7 @@ public void prepareReadWriteTransaction() { @Override public void close() { - Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION.method).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION).startSpan(); try (Scope s = tracer.withSpan(span)) { spanner.runWithRetries( new Callable() { @@ -215,7 +215,7 @@ public Void call() throws Exception { spanner.getRpc().deleteSession(name, options); return null; } - }, SpannerImpl.DELETE_SESSION.retryOnErrorCodes); + }); span.end(); } catch (RuntimeException e) { TraceUtil.endSpanWithFailure(span, e); @@ -224,7 +224,7 @@ public Void call() throws Exception { } ByteString beginTransaction() { - Span span = tracer.spanBuilder(SpannerImpl.BEGIN_TRANSACTION.method).startSpan(); + Span span = tracer.spanBuilder(SpannerImpl.BEGIN_TRANSACTION).startSpan(); try (Scope s = tracer.withSpan(span)) { final BeginTransactionRequest request = BeginTransactionRequest.newBuilder() @@ -240,7 +240,7 @@ ByteString beginTransaction() { public Transaction call() throws Exception { return spanner.getRpc().beginTransaction(request, options); } - }, SpannerImpl.BEGIN_TRANSACTION.retryOnErrorCodes); + }); if (txn.getId().isEmpty()) { throw newSpannerException(ErrorCode.INTERNAL, "Missing id in transaction\n" + getName()); } diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index bdee01c85a65..69302aab5c53 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -33,27 +33,19 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; import com.google.api.client.util.BackOff; import com.google.api.client.util.ExponentialBackOff; -import com.google.api.core.ApiClock; import com.google.api.gax.paging.Page; -import com.google.api.gax.retrying.BasicResultRetryAlgorithm; -import com.google.api.gax.retrying.DirectRetryingExecutor; -import com.google.api.gax.retrying.ExponentialRetryAlgorithm; -import com.google.api.gax.retrying.ResultRetryAlgorithm; -import com.google.api.gax.retrying.RetryAlgorithm; -import com.google.api.gax.retrying.RetrySettings; -import com.google.api.gax.retrying.RetryingExecutor; -import com.google.api.gax.retrying.RetryingFuture; -import com.google.api.gax.retrying.TimedRetryAlgorithm; import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; +import com.google.cloud.RetryHelper.RetryHelperException; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.annotations.VisibleForTesting; @@ -61,7 +53,6 @@ import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; -import com.google.common.collect.Sets; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import io.grpc.Context; @@ -79,47 +70,19 @@ class SpannerImpl extends BaseService implements Spanner { private static final Logger logger = Logger.getLogger(SpannerImpl.class.getName()); private static final Tracer tracer = Tracing.getTracer(); - static final class RpcCall { - final String method; - final Set retryOnErrorCodes; - - private RpcCall(String method, Set retryOnErrorCodes) { - this.method = method; - this.retryOnErrorCodes = retryOnErrorCodes; - } - } - - static final Set DEFAULT_RETRY_ERROR_CODES = - Sets.newHashSet(ErrorCode.UNAVAILABLE, ErrorCode.DEADLINE_EXCEEDED); - static final Set NON_IDEMPOTENT_RETRY_ERROR_CODES = - Sets.newHashSet(ErrorCode.UNAVAILABLE); - static final RpcCall CREATE_SESSION = - new RpcCall("CloudSpannerOperation.CreateSession", DEFAULT_RETRY_ERROR_CODES); - static final RpcCall DELETE_SESSION = - new RpcCall("CloudSpannerOperation.DeleteSession", DEFAULT_RETRY_ERROR_CODES); - static final RpcCall BEGIN_TRANSACTION = - new RpcCall("CloudSpannerOperation.BeginTransaction", DEFAULT_RETRY_ERROR_CODES); - static final RpcCall COMMIT = - new RpcCall("CloudSpannerOperation.Commit", NON_IDEMPOTENT_RETRY_ERROR_CODES); - static final RpcCall QUERY = - new RpcCall("CloudSpannerOperation.ExecuteStreamingQuery", DEFAULT_RETRY_ERROR_CODES); - static final RpcCall READ = - new RpcCall("CloudSpannerOperation.ExecuteStreamingRead", DEFAULT_RETRY_ERROR_CODES); + private static final String CREATE_SESSION = "CloudSpannerOperation.CreateSession"; + static final String DELETE_SESSION = "CloudSpannerOperation.DeleteSession"; + static final String BEGIN_TRANSACTION = "CloudSpannerOperation.BeginTransaction"; + static final String COMMIT = "CloudSpannerOperation.Commit"; + static final String QUERY = "CloudSpannerOperation.ExecuteStreamingQuery"; + static final String READ = "CloudSpannerOperation.ExecuteStreamingRead"; static { - TraceUtil.exportSpans( - CREATE_SESSION.method, - DELETE_SESSION.method, - BEGIN_TRANSACTION.method, - COMMIT.method, - QUERY.method, - READ.method); + TraceUtil.exportSpans(CREATE_SESSION, DELETE_SESSION, BEGIN_TRANSACTION, COMMIT, QUERY, READ); } private final Random random = new Random(); private final SpannerRpc gapicRpc; - private final RetrySettings retrySettings; - private final ApiClock clock; @GuardedBy("this") private final Map dbClients = new HashMap<>(); @@ -134,8 +97,6 @@ private RpcCall(String method, Set retryOnErrorCodes) { SpannerImpl(SpannerRpc gapicRpc, SpannerOptions options) { super(options); this.gapicRpc = gapicRpc; - this.retrySettings = options.getRetrySettings(); - this.clock = options.getClock(); this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), this); this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), this, dbAdminClient); } @@ -197,24 +158,27 @@ public void cancelled(Context context) { } } - private static class SpannerRetryAlgorithm extends BasicResultRetryAlgorithm { - private final Set retryOnErrorCodes; - - private SpannerRetryAlgorithm(Set retryOnErrorCodes) { - this.retryOnErrorCodes = retryOnErrorCodes; - } - - @Override - public boolean shouldRetry(Throwable prevThrowable, T prevResponse) { - if (prevThrowable != null) { - if (prevThrowable instanceof SpannerException) { - SpannerException spannerException = (SpannerException) prevThrowable; - if (retryOnErrorCodes.contains(spannerException.getErrorCode())) { - return spannerException.isRetryable(); - } + T runWithRetries(final Callable callable) { + try { + return callable.call(); + } catch (SpannerException e) { + throw e; + } catch (TimeoutException e) { + throw SpannerExceptionFactory.newSpannerException(ErrorCode.DEADLINE_EXCEEDED, e.getMessage(), e); + } catch (RetryHelperException e) { + if(e.getCause() != null) { + Throwables.throwIfUnchecked(e.getCause()); + } + throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e); + } catch (Exception e) { + if(e.getCause() != null) { + Throwable cause = e.getCause(); + if(cause instanceof RetryHelperException) { + cause = cause.getCause(); } + Throwables.throwIfUnchecked(cause); } - return false; + throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e); } } @@ -223,7 +187,7 @@ public boolean shouldRetry(Throwable prevThrowable, T prevResponse) { * *

TODO: Consider replacing with RetryHelper from gcloud-core. */ - T runWithRetries(Callable callable, Set retryOnErrorCodes) { + T runWithRetries_WithContext(Callable callable, Set retryOnErrorCodes) { // Use same backoff setting as abort, somewhat arbitrarily. Span span = tracer.getCurrentSpan(); ExponentialBackOff backOff = newBackOff(); @@ -235,8 +199,9 @@ T runWithRetries(Callable callable, Set retryOnErrorCodes) { span.addAnnotation( "Starting operation", ImmutableMap.of("Attempt", AttributeValue.longAttributeValue(attempt))); - return runWithRetries( - callable, retrySettings, new SpannerRetryAlgorithm<>(retryOnErrorCodes), clock); + return null; +// return runWithRetries( +// callable, retrySettings, new SpannerRetryAlgorithm<>(retryOnErrorCodes), clock); } catch (SpannerException e) { if (!e.isRetryable()) { throw e; @@ -255,36 +220,6 @@ T runWithRetries(Callable callable, Set retryOnErrorCodes) { } } - private static V runWithRetries( - Callable callable, - RetrySettings retrySettings, - ResultRetryAlgorithm resultRetryAlgorithm, - ApiClock clock) { - try { - @SuppressWarnings("unchecked") - ResultRetryAlgorithm algorithm = (ResultRetryAlgorithm) resultRetryAlgorithm; - return run(callable, new ExponentialRetryAlgorithm(retrySettings, clock), algorithm); - } catch (ExecutionException e) { - Throwables.throwIfUnchecked(e.getCause()); - throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e.getCause()); - } catch (InterruptedException e) { - throw newSpannerException(ErrorCode.CANCELLED, "Operation cancelled.", e); - } - } - - private static V run( - Callable callable, - TimedRetryAlgorithm timedAlgorithm, - ResultRetryAlgorithm resultAlgorithm) - throws ExecutionException, InterruptedException { - RetryAlgorithm retryAlgorithm = new RetryAlgorithm<>(resultAlgorithm, timedAlgorithm); - RetryingExecutor executor = new DirectRetryingExecutor<>(retryAlgorithm); - - RetryingFuture retryingFuture = executor.createFuture(callable); - executor.submit(retryingFuture); - return retryingFuture.get(); - } - /** Returns the {@link SpannerRpc} of this {@link SpannerImpl} instance. */ SpannerRpc getRpc() { return gapicRpc; @@ -298,7 +233,7 @@ int getDefaultPrefetchChunks() { SessionImpl createSession(final DatabaseId db) throws SpannerException { final Map options = optionMap(SessionOption.channelHint(random.nextLong())); - Span span = tracer.spanBuilder(CREATE_SESSION.method).startSpan(); + Span span = tracer.spanBuilder(CREATE_SESSION).startSpan(); try (Scope s = tracer.withSpan(span)) { com.google.spanner.v1.Session session = runWithRetries( @@ -308,7 +243,7 @@ public com.google.spanner.v1.Session call() throws Exception { return gapicRpc.createSession( db.getName(), getOptions().getSessionLabels(), options); } - }, CREATE_SESSION.retryOnErrorCodes); + }); span.end(); return new SessionImpl(this, session.getName(), options); } catch (RuntimeException e) { @@ -434,12 +369,10 @@ Object value() { /** Helper class for gRPC calls that can return paginated results. */ abstract static class PageFetcher implements NextPageFetcher { private final SpannerImpl spanner; - private final Set retryOnErrorCodes; private String nextPageToken; - PageFetcher(SpannerImpl spanner, Set retryOnErrorCodes) { + PageFetcher(SpannerImpl spanner) { this.spanner = spanner; - this.retryOnErrorCodes = retryOnErrorCodes; } @Override @@ -451,7 +384,7 @@ public Page getNextPage() { public Paginated call() { return getNextPage(nextPageToken); } - }, retryOnErrorCodes); + }); this.nextPageToken = nextPage.getNextPageToken(); List results = new ArrayList<>(); for (T proto : nextPage.getResults()) { diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index b5457f814d26..8b89e2ba7003 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -17,7 +17,13 @@ package com.google.cloud.spanner; import com.google.api.core.ApiFunction; +import java.io.IOException; +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Map; +import java.util.Set; import com.google.api.gax.grpc.GrpcInterceptorProvider; +import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.TransportChannelProvider; import com.google.cloud.ServiceDefaults; import com.google.cloud.ServiceOptions; @@ -27,6 +33,7 @@ import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -57,6 +64,7 @@ public class SpannerOptions extends ServiceOptions { private final int prefetchChunks; private final int numChannels; private final ImmutableMap sessionLabels; + private final SpannerStubSettings stubSettings; /** Default implementation of {@code SpannerFactory}. */ private static class DefaultSpannerFactory implements SpannerFactory { @@ -96,6 +104,11 @@ private SpannerOptions(Builder builder) { : SessionPoolOptions.newBuilder().build(); prefetchChunks = builder.prefetchChunks; sessionLabels = builder.sessionLabels; + try { + stubSettings = builder.stubSettingsBuilder.build(); + } catch (IOException e) { + throw SpannerExceptionFactory.newSpannerException(e); + } } /** Builder for {@link SpannerOptions} instances. */ @@ -115,6 +128,7 @@ public static class Builder private int prefetchChunks = DEFAULT_PREFETCH_CHUNKS; private SessionPoolOptions sessionPoolOptions; private ImmutableMap sessionLabels; + private SpannerStubSettings.Builder stubSettingsBuilder = SpannerStubSettings.newBuilder(); private Builder() {} @@ -124,6 +138,7 @@ private Builder() {} this.sessionPoolOptions = options.sessionPoolOptions; this.prefetchChunks = options.prefetchChunks; this.sessionLabels = options.sessionLabels; + this.stubSettingsBuilder = options.stubSettings.toBuilder(); this.channelProvider = options.channelProvider; this.channelConfigurator = options.channelConfigurator; this.interceptorProvider = options.interceptorProvider; @@ -202,6 +217,16 @@ public Builder setSessionLabels(Map sessionLabels) { return this; } + @Override + public Builder setRetrySettings(RetrySettings retrySettings) { + throw new UnsupportedOperationException("SpannerOptions does not support setting global retry settings. " + + "Call stubSettingsBuilder().Settings().setRetrySettings(RetrySettings) instead."); + } + + public SpannerStubSettings.Builder stubSettingsBuilder() { + return stubSettingsBuilder; + } + /** * Specifying this will allow the client to prefetch up to {@code prefetchChunks} {@code * PartialResultSet} chunks for each read and query. The data size of each chunk depends on the @@ -258,6 +283,10 @@ public Map getSessionLabels() { return sessionLabels; } + public SpannerStubSettings getStubSettings() { + return stubSettings; + } + public int getPrefetchChunks() { return prefetchChunks; } diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index ad0e2d11f854..3e166f26a8cd 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -122,7 +122,7 @@ void commit() { mutations = null; } final CommitRequest commitRequest = builder.build(); - Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT.method, span).startSpan(); + Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT, span).startSpan(); try (Scope s = tracer.withSpan(opSpan)) { CommitResponse commitResponse = spanner.runWithRetries( @@ -131,7 +131,7 @@ void commit() { public CommitResponse call() throws Exception { return spanner.getRpc().commit(commitRequest, session.getOptions()); } - }, SpannerImpl.COMMIT.retryOnErrorCodes); + }); if (!commitResponse.hasCommitTimestamp()) { throw newSpannerException( @@ -245,7 +245,7 @@ public long executeUpdate(Statement statement) { public com.google.spanner.v1.ResultSet call() throws Exception { return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); } - }, SpannerImpl.QUERY.retryOnErrorCodes); + }); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "DML response missing stats possibly due to non-DML statement as input"); @@ -265,7 +265,7 @@ public long[] batchUpdate(Iterable statements) { public com.google.spanner.v1.ExecuteBatchDmlResponse call() throws Exception { return spanner.getRpc().executeBatchDml(builder.build(), session.getOptions()); } - }, SpannerImpl.QUERY.retryOnErrorCodes); + }); long[] results = new long[response.getResultSetsCount()]; for (int i = 0; i < response.getResultSetsCount(); ++i) { results[i] = response.getResultSets(i).getStats().getRowCountExact(); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 4aaf8e1e6298..64deaf014a1a 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -33,7 +33,6 @@ import com.google.api.gax.rpc.InstantiatingWatchdogProvider; import com.google.api.gax.rpc.OperationCallable; import com.google.api.gax.rpc.ResponseObserver; -import com.google.api.gax.rpc.StatusCode; import com.google.api.gax.rpc.StreamController; import com.google.api.gax.rpc.TransportChannelProvider; import com.google.api.gax.rpc.UnaryCallSettings; @@ -50,13 +49,11 @@ import com.google.cloud.spanner.admin.instance.v1.stub.GrpcInstanceAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; -import com.google.cloud.spanner.spi.v1.SpannerRpc.Option; import com.google.cloud.spanner.v1.stub.GrpcSpannerStub; import com.google.cloud.spanner.v1.stub.SpannerStub; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.longrunning.GetOperationRequest; import com.google.longrunning.Operation; @@ -187,7 +184,7 @@ public static GapicSpannerRpc create(SpannerOptions options) { return new GapicSpannerRpc(options); } - public GapicSpannerRpc(SpannerOptions options) { + public GapicSpannerRpc(final SpannerOptions options) { this.projectId = options.getProjectId(); this.projectName = PROJECT_NAME_TEMPLATE.instantiate("project", this.projectId); @@ -255,27 +252,25 @@ public GapicSpannerRpc(SpannerOptions options) { .withCheckInterval(checkInterval) .withClock(NanoClock.getDefaultClock()); - // Disabling retry for now because spanner handles retry in SpannerImpl. - // We will finally want to improve gax but for smooth transitioning we - // preserve the retry in SpannerImpl try { // TODO: bump the version of gax and remove this try-catch block // applyToAllUnaryMethods does not throw exception in the latest version + SpannerStubSettings.Builder builder = options.getStubSettings().toBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) +// .applyToAllUnaryMethods( +// new ApiFunction, Void>() { +// @Override +// public Void apply(UnaryCallSettings.Builder builder) { +// builder.setRetrySettings(options.getRetrySettings()); +// builder.setRetryableCodes(options.getRetryableCodes()); +// return null; +// } +// }) + ; this.spannerStub = - GrpcSpannerStub.create( - SpannerStubSettings.newBuilder() - .setTransportChannelProvider(channelProvider) - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) - .applyToAllUnaryMethods( - new ApiFunction, Void>() { - @Override - public Void apply(UnaryCallSettings.Builder builder) { - builder.setRetryableCodes(ImmutableSet.of()); - return null; - } - }) - .build()); + GrpcSpannerStub.create(builder.build()); this.instanceAdminStub = GrpcInstanceAdminStub.create( @@ -283,14 +278,14 @@ public Void apply(UnaryCallSettings.Builder builder) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .applyToAllUnaryMethods( - new ApiFunction, Void>() { - @Override - public Void apply(UnaryCallSettings.Builder builder) { - builder.setRetryableCodes(ImmutableSet.of()); - return null; - } - }) +// .applyToAllUnaryMethods( +// new ApiFunction, Void>() { +// @Override +// public Void apply(UnaryCallSettings.Builder builder) { +// builder.setRetryableCodes(options.getRetryableCodes()); +// return null; +// } +// }) .build()); this.databaseAdminStub = GrpcDatabaseAdminStub.create( @@ -298,14 +293,14 @@ public Void apply(UnaryCallSettings.Builder builder) { .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) - .applyToAllUnaryMethods( - new ApiFunction, Void>() { - @Override - public Void apply(UnaryCallSettings.Builder builder) { - builder.setRetryableCodes(ImmutableSet.of()); - return null; - } - }) +// .applyToAllUnaryMethods( +// new ApiFunction, Void>() { +// @Override +// public Void apply(UnaryCallSettings.Builder builder) { +// builder.setRetryableCodes(options.getRetryableCodes()); +// return null; +// } +// }) .build()); } catch (Exception e) { throw newSpannerException(e); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java new file mode 100644 index 000000000000..ba9a0bbb8e79 --- /dev/null +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java @@ -0,0 +1,352 @@ +/* + * 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 + * + * https://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 static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import javax.net.ssl.SSLHandshakeException; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.threeten.bp.Duration; +import com.google.api.core.ApiFunction; +import com.google.api.gax.core.NoCredentialsProvider; +import com.google.api.gax.grpc.testing.LocalChannelProvider; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.rpc.UnaryCallSettings.Builder; +import com.google.cloud.NoCredentials; +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.v1.SpannerClient; +import com.google.cloud.spanner.v1.SpannerSettings; +import com.google.common.collect.ImmutableSet; +import com.google.protobuf.ListValue; +import com.google.spanner.v1.ResultSetMetadata; +import com.google.spanner.v1.StructType; +import com.google.spanner.v1.StructType.Field; +import com.google.spanner.v1.TypeCode; +import io.grpc.Server; +import io.grpc.StatusRuntimeException; +import io.grpc.inprocess.InProcessServerBuilder; + +@RunWith(Parameterized.class) +public class SpannerImplGaxRetryTest { + private static final ResultSetMetadata READ_METADATA = ResultSetMetadata.newBuilder() + .setRowType(StructType.newBuilder().addFields(Field.newBuilder().setName("BAR") + .setType(com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.INT64).build()).build()) + .build()) + .build(); + private static final com.google.spanner.v1.ResultSet READ_RESULTSET = + com.google.spanner.v1.ResultSet.newBuilder().addRows(ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()).build()) + .addRows(ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) + .build()) + .setMetadata(READ_METADATA).build(); + private static final com.google.spanner.v1.ResultSet READ_ROW_RESULTSET = + com.google.spanner.v1.ResultSet.newBuilder() + .addRows(ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()) + .build()) + .setMetadata(READ_METADATA).build(); + private static final Statement SELECT1AND2 = + Statement.of("SELECT 1 AS COL1 UNION ALL SELECT 2 AS COL1"); + private static final ResultSetMetadata SELECT1AND2_METADATA = ResultSetMetadata.newBuilder() + .setRowType(StructType.newBuilder().addFields(Field.newBuilder().setName("COL1") + .setType(com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.INT64).build()).build()) + .build()) + .build(); + private static final com.google.spanner.v1.ResultSet SELECT1_RESULTSET = + com.google.spanner.v1.ResultSet.newBuilder().addRows(ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()).build()) + .addRows(ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) + .build()) + .setMetadata(SELECT1AND2_METADATA).build(); + private static final Statement UPDATE_STATEMENT = + Statement.of("UPDATE FOO SET BAR=1 WHERE BAZ=2"); + private static final long UPDATE_COUNT = 1L; + private static final SimulatedExecutionTime ONE_SECOND = + SimulatedExecutionTime.ofMinimumAndRandomTime(1000, 0); + private static final StatusRuntimeException UNAVAILABLE = + io.grpc.Status.UNAVAILABLE.withDescription("TEST").asRuntimeException(); + private static MockSpannerServiceImpl mockSpanner; + private static Server server; + private static LocalChannelProvider channelProvider; + private static SpannerClient spannerClient; + private static Spanner spanner; + private static DatabaseClient client; + + @Parameter(0) + public boolean enableGaxRetries; + + @Parameters(name = "enable GAX retries = {0}") + public static Collection data() { + List params = new ArrayList<>(); + params.add(new Object[] {true}); + params.add(new Object[] {false}); + return params; + } + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @BeforeClass + public static void startStaticServer() throws IOException { + mockSpanner = new MockSpannerServiceImpl(); + mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. + mockSpanner.putStatementResult( + StatementResult.read("FOO", KeySet.all(), Arrays.asList("BAR"), READ_RESULTSET)); + mockSpanner.putStatementResult(StatementResult.read("FOO", KeySet.singleKey(Key.of()), + Arrays.asList("BAR"), READ_ROW_RESULTSET)); + mockSpanner.putStatementResult(StatementResult.query(SELECT1AND2, SELECT1_RESULTSET)); + mockSpanner.putStatementResult(StatementResult.update(UPDATE_STATEMENT, UPDATE_COUNT)); + + String uniqueName = InProcessServerBuilder.generateName(); + server = InProcessServerBuilder.forName(uniqueName) + // We need to use a real executor for timeouts to occur. + .scheduledExecutorService(new ScheduledThreadPoolExecutor(4)).addService(mockSpanner) + .build().start(); + channelProvider = LocalChannelProvider.create(uniqueName); + + SpannerSettings settings = + SpannerSettings.newBuilder().setTransportChannelProvider(channelProvider) + .setCredentialsProvider(NoCredentialsProvider.create()).build(); + spannerClient = SpannerClient.create(settings); + } + + @AfterClass + public static void stopServer() { + spannerClient.close(); + server.shutdown(); + } + + @Before + public void setUp() throws Exception { + mockSpanner.reset(); + mockSpanner.removeAllExecutionTimes(); + final RetrySettings retrySettings = RetrySettings.newBuilder() + .setInitialRpcTimeout(Duration.ofMillis(100L)).setMaxRpcTimeout(Duration.ofMillis(100L)) + .setMaxAttempts(3).setTotalTimeout(Duration.ofMillis(500L)).build(); + SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("[PROJECT]") + .setChannelProvider(channelProvider).setCredentials(NoCredentials.getInstance()); + builder.stubSettingsBuilder() + .applyToAllUnaryMethods(new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetrySettings(retrySettings); + return null; + } + }); + builder.stubSettingsBuilder().executeStreamingSqlSettings().setRetrySettings(retrySettings); + builder.stubSettingsBuilder().streamingReadSettings().setRetrySettings(retrySettings); + if (!enableGaxRetries) { + // Disable retries by removing all retryable codes. + builder.stubSettingsBuilder() + .applyToAllUnaryMethods(new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetryableCodes(ImmutableSet.of()); + return null; + } + }); + builder.stubSettingsBuilder().executeStreamingSqlSettings() + .setRetryableCodes(ImmutableSet.of()); + builder.stubSettingsBuilder().streamingReadSettings() + .setRetryableCodes(ImmutableSet.of()); + } + spanner = builder.build().getService(); + client = spanner.getDatabaseClient(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]")); + } + + @After + public void tearDown() throws Exception { + spanner.close(); + } + + @Test + public void singleUse() { + if (enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); + } + mockSpanner.setCreateSessionExecutionTime(ONE_SECOND); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + while (rs.next()) { + } + } + } + + @Test + public void singleUseUnavailable() { + if (!enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); + } + mockSpanner.addException(UNAVAILABLE); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + while (rs.next()) { + } + } + } + + @Test + public void singleUseReadOnlyTransaction() { + if (enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); + } + mockSpanner.setCreateSessionExecutionTime(ONE_SECOND); + try (ResultSet rs = client.singleUseReadOnlyTransaction().executeQuery(SELECT1AND2)) { + while (rs.next()) { + } + } + } + + @Test + public void singleUseReadOnlyTransactionUnavailable() { + if (!enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); + } + mockSpanner.addException(UNAVAILABLE); + try (ResultSet rs = client.singleUseReadOnlyTransaction().executeQuery(SELECT1AND2)) { + while (rs.next()) { + } + } + } + + @Test + public void singleUseExecuteStreamingSqlTimeout() { + // Streaming calls do not timeout. + mockSpanner.setExecuteStreamingSqlExecutionTime(ONE_SECOND); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + while (rs.next()) { + } + } + } + + @Test + public void singleUseExecuteStreamingSqlUnavailable() { + // executeStreamingSql is always retried. + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + mockSpanner.addException(UNAVAILABLE); + while (rs.next()) { + } + } + } + + @Test + public void readWriteTransactionTimeout() { + if (enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); + } + mockSpanner.setBeginTransactionExecutionTime(ONE_SECOND); + TransactionRunner runner = client.readWriteTransaction(); + long updateCount = runner.run(new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); + assertThat(updateCount, is(equalTo(UPDATE_COUNT))); + } + + @Test + public void readWriteTransactionUnavailable() { + if (!enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); + } + mockSpanner.addException(UNAVAILABLE); + TransactionRunner runner = client.readWriteTransaction(); + long updateCount = runner.run(new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); + assertThat(updateCount, is(equalTo(UPDATE_COUNT))); + } + + @SuppressWarnings("resource") + @Test + public void transactionManagerTimeout() { + if (enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); + } + mockSpanner.setBeginTransactionExecutionTime(ONE_SECOND); + try (TransactionManager txManager = client.transactionManager()) { + TransactionContext tx = txManager.begin(); + while (true) { + try { + assertThat(tx.executeUpdate(UPDATE_STATEMENT), is(equalTo(UPDATE_COUNT))); + txManager.commit(); + break; + } catch (AbortedException e) { + tx = txManager.resetForRetry(); + } + } + } + } + + @SuppressWarnings("resource") + @Test + public void transactionManagerUnavailable() { + if (!enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); + } + mockSpanner.addException(UNAVAILABLE); + try (TransactionManager txManager = client.transactionManager()) { + TransactionContext tx = txManager.begin(); + while (true) { + try { + assertThat(tx.executeUpdate(UPDATE_STATEMENT), is(equalTo(UPDATE_COUNT))); + txManager.commit(); + break; + } catch (AbortedException e) { + tx = txManager.resetForRetry(); + } + } + } + } + + //TODO(loite): Check what happens when an SSLHandshakeException is thrown when using GAX. + @Test + @Ignore + public void singleUseSslHandshakeException() { + if (!enableGaxRetries) { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); + } + mockSpanner.addException(new SSLHandshakeException("some SSL handshake exception")); + ((SpannerImpl) spanner).createSession(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]")); + } +} diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java deleted file mode 100644 index f2c5d5842a56..000000000000 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplRetryTest.java +++ /dev/null @@ -1,173 +0,0 @@ -/* - * Copyright 2017 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 static com.google.cloud.spanner.SpannerMatchers.isSpannerException; -import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Mockito.mock; - -import com.google.api.gax.retrying.RetrySettings; -import com.google.cloud.NoCredentials; -import com.google.cloud.spanner.spi.v1.SpannerRpc; -import io.grpc.Context; -import java.util.concurrent.Callable; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Mockito; -import org.threeten.bp.Duration; - -/** Unit tests for {@link SpannerImpl#runWithRetries}. */ -@RunWith(JUnit4.class) -public class SpannerImplRetryTest { - private static final String PROJECT_ID = "Test-Project"; - - interface StringCallable extends Callable { - @Override - String call(); - } - - static class RetryableException extends SpannerException { - RetryableException(ErrorCode code, @Nullable String message) { - // OK to instantiate SpannerException directly for this unit test. - super(DoNotConstructDirectly.ALLOWED, code, true, message, null); - } - } - - static class NonRetryableException extends SpannerException { - NonRetryableException(ErrorCode code, @Nullable String message) { - super(DoNotConstructDirectly.ALLOWED, code, false, message, null); - } - } - - @Rule public ExpectedException expectedException = ExpectedException.none(); - SpannerImpl spanner; - StringCallable callable; - - @Before - public void setUp() { - callable = Mockito.mock(StringCallable.class); - SpannerOptions options = - SpannerOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setProjectId(PROJECT_ID) - .build(); - SpannerRpc rpc = mock(SpannerRpc.class); - spanner = new SpannerImpl(rpc, options); - } - - @Test - public void ok() { - Mockito.when(callable.call()).thenReturn("r"); - assertThat(spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES)) - .isEqualTo("r"); - } - - @Test - public void nonRetryableFailure() { - Mockito.when(callable.call()) - .thenThrow(new NonRetryableException(ErrorCode.FAILED_PRECONDITION, "Failed by test")); - expectedException.expect(isSpannerException(ErrorCode.FAILED_PRECONDITION)); - spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } - - @Test - public void retryableFailure() { - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #2")) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #3")) - .thenReturn("r"); - assertThat(spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES)) - .isEqualTo("r"); - } - - @Test - public void retryableFailureFollowedByPermanentFailure() { - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #2")) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #3")) - .thenThrow(new NonRetryableException(ErrorCode.FAILED_PRECONDITION, "Failed by test")); - expectedException.expect(isSpannerException(ErrorCode.FAILED_PRECONDITION)); - spanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } - - @Test - public void contextCancelled() { - SpannerOptions options = - SpannerOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setProjectId(PROJECT_ID) - .setRetrySettings( - RetrySettings.newBuilder().setTotalTimeout(Duration.ofMillis(5L)).build()) - .build(); - SpannerRpc rpc = mock(SpannerRpc.class); - try (SpannerImpl contextSpanner = new SpannerImpl(rpc, options)) { - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); - Context.CancellableContext context = Context.current().withCancellation(); - Runnable work = - context.wrap( - new Runnable() { - @Override - public void run() { - contextSpanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } - }); - context.cancel(new RuntimeException("Cancelled by test")); - expectedException.expect(isSpannerException(ErrorCode.CANCELLED)); - work.run(); - } - } - - @Test - public void contextDeadlineExceeded() { - SpannerOptions options = - SpannerOptions.newBuilder() - .setCredentials(NoCredentials.getInstance()) - .setProjectId(PROJECT_ID) - .setRetrySettings( - RetrySettings.newBuilder().setTotalTimeout(Duration.ofMillis(5L)).build()) - .build(); - SpannerRpc rpc = mock(SpannerRpc.class); - try (SpannerImpl contextSpanner = new SpannerImpl(rpc, options)) { - ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); - Context.CancellableContext context = - Context.current().withDeadlineAfter(10, TimeUnit.MILLISECONDS, executor); - Mockito.when(callable.call()) - .thenThrow(new RetryableException(ErrorCode.UNAVAILABLE, "Failure #1")); - Runnable work = - context.wrap( - new Runnable() { - @Override - public void run() { - contextSpanner.runWithRetries(callable, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } - }); - expectedException.expect(isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); - work.run(); - } - } -} \ No newline at end of file diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 38e79becf0cc..ff5d5a641309 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -135,55 +135,10 @@ public void exceptionIsTranslated() { public Void call() throws Exception { throw new Exception("Should be translated to SpannerException"); } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); + }); } catch (SpannerException e) { assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL); assertThat(e.getMessage().contains("Unexpected exception thrown")); } } - - @Test - public void sslHandshakeExceptionIsNotRetryable() { - // Verify that a SpannerException with code UNAVAILABLE and cause SSLHandshakeException is not - // retryable. - boolean gotExpectedException = false; - try { - impl.runWithRetries( - new Callable() { - @Override - public Void call() throws Exception { - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.UNAVAILABLE, - "This exception should not be retryable", - new SSLHandshakeException("some SSL handshake exception")); - } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } catch (SpannerException e) { - gotExpectedException = true; - assertThat(e.isRetryable(), is(false)); - assertThat(e.getErrorCode()).isEqualTo(ErrorCode.UNAVAILABLE); - assertThat(e.getMessage().contains("This exception should not be retryable")); - } - assertThat(gotExpectedException, is(true)); - - // Verify that any other SpannerException with code UNAVAILABLE is retryable. - impl.runWithRetries( - new Callable() { - private boolean firstTime = true; - - @Override - public Void call() throws Exception { - // Keep track of whethr this is the first call or a subsequent call to avoid an infinite - // loop. - if (firstTime) { - firstTime = false; - throw SpannerExceptionFactory.newSpannerException( - ErrorCode.UNAVAILABLE, - "This exception should be retryable", - new Exception("some other exception")); - } - return null; - } - }, SpannerImpl.DEFAULT_RETRY_ERROR_CODES); - } } From 081569c2a8939935a49aa0fe2ae4536023aad7a1 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 15 Apr 2019 14:07:54 +0200 Subject: [PATCH 05/26] removed runWithRetries and added access to gax settings on options --- .../cloud/spanner/AbstractReadContext.java | 13 +- .../google/cloud/spanner/BatchClientImpl.java | 19 +- .../spanner/DatabaseAdminClientImpl.java | 40 +- .../spanner/InstanceAdminClientImpl.java | 42 +- .../spanner/PartitionedDMLTransaction.java | 18 +- .../com/google/cloud/spanner/SessionImpl.java | 34 +- .../com/google/cloud/spanner/SpannerImpl.java | 123 +--- .../google/cloud/spanner/SpannerOptions.java | 63 +- .../cloud/spanner/TransactionRunnerImpl.java | 40 +- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 58 +- .../cloud/spanner/BatchClientImplTest.java | 22 +- .../spanner/DatabaseAdminClientImplTest.java | 20 +- .../spanner/InstanceAdminClientImplTest.java | 18 +- .../cloud/spanner/InstanceAdminGaxTest.java | 592 ++++++++++++++++++ .../google/cloud/spanner/SessionImplTest.java | 38 +- ...etryTest.java => SpannerGaxRetryTest.java} | 303 +++++---- .../google/cloud/spanner/SpannerImplTest.java | 29 +- 17 files changed, 976 insertions(+), 496 deletions(-) create mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java rename google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/{SpannerImplGaxRetryTest.java => SpannerGaxRetryTest.java} (58%) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 3a099baa8c4f..7340257f356c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -43,7 +43,6 @@ import io.opencensus.trace.Span; import io.opencensus.trace.Tracing; import java.util.Map; -import java.util.concurrent.Callable; import java.util.concurrent.atomic.AtomicLong; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; @@ -227,13 +226,7 @@ void initTransaction() { .setOptions(options) .build(); Transaction transaction = - spanner.runWithRetries( - new Callable() { - @Override - public Transaction call() throws Exception { - return spanner.getRpc().beginTransaction(request, session.getOptions()); - } - }); + spanner.getRpc().beginTransaction(request, session.getOptions()); if (!transaction.hasReadTimestamp()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, "Missing expected transaction.read_timestamp metadata field"); @@ -426,7 +419,9 @@ CloseableIterator startStream(@Nullable ByteString resumeToken request.setResumeToken(resumeToken); } SpannerRpc.StreamingCall call = - spanner.getRpc().executeQuery(request.build(), stream.consumer(), session.getOptions()); + spanner + .getRpc() + .executeQuery(request.build(), stream.consumer(), session.getOptions()); call.request(prefetchChunks); stream.setCall(call); return stream; diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java index d6f6215cdbbc..fb1757b3bc3c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java @@ -32,7 +32,6 @@ import com.google.spanner.v1.TransactionSelector; import java.util.List; import java.util.Map; -import java.util.concurrent.Callable; /** Default implementation for Batch Client interface. */ public class BatchClientImpl implements BatchClient { @@ -135,14 +134,7 @@ public List partitionReadUsingIndex( builder.setPartitionOptions(pbuilder.build()); final PartitionReadRequest request = builder.build(); - PartitionResponse response = - spanner.runWithRetries( - new Callable() { - @Override - public PartitionResponse call() throws Exception { - return spanner.getRpc().partitionRead(request, options); - } - }); + PartitionResponse response = spanner.getRpc().partitionRead(request, options); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = @@ -180,14 +172,7 @@ public List partitionQuery( builder.setPartitionOptions(pbuilder.build()); final PartitionQueryRequest request = builder.build(); - PartitionResponse response = - spanner.runWithRetries( - new Callable() { - @Override - public PartitionResponse call() throws Exception { - return spanner.getRpc().partitionQuery(request, options); - } - }); + PartitionResponse response = spanner.getRpc().partitionQuery(request, options); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java index 4678fac6a2cd..80bf08aa4e6d 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java @@ -31,7 +31,6 @@ import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; import java.util.List; import java.util.UUID; -import java.util.concurrent.Callable; import javax.annotation.Nullable; /** Default implementation of {@link DatabaseAdminClient}. */ @@ -57,7 +56,8 @@ public OperationFuture createDatabase( String instanceName = getInstanceName(instanceId); String createStatement = "CREATE DATABASE `" + databaseId + "`"; OperationFuture - rawOperationFuture = spanner.getRpc().createDatabase(instanceName, createStatement, statements); + rawOperationFuture = + spanner.getRpc().createDatabase(instanceName, createStatement, statements); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), @@ -82,15 +82,8 @@ public Database apply(Exception e) { @Override public Database getDatabase(String instanceId, String databaseId) throws SpannerException { - final String dbName = getDatabaseName(instanceId, databaseId); - Callable callable = - new Callable() { - @Override - public Database call() throws Exception { - return Database.fromProto(spanner.getRpc().getDatabase(dbName), DatabaseAdminClientImpl.this); - } - }; - return spanner.runWithRetries(callable); + String dbName = getDatabaseName(instanceId, databaseId); + return Database.fromProto(spanner.getRpc().getDatabase(dbName), DatabaseAdminClientImpl.this); } @Override @@ -125,29 +118,14 @@ public Void apply(Exception e) { @Override public void dropDatabase(String instanceId, String databaseId) throws SpannerException { - final String dbName = getDatabaseName(instanceId, databaseId); - Callable callable = - new Callable() { - @Override - public Void call() throws Exception { - spanner.getRpc().dropDatabase(dbName); - return null; - } - }; - spanner.runWithRetries(callable); + String dbName = getDatabaseName(instanceId, databaseId); + spanner.getRpc().dropDatabase(dbName); } @Override public List getDatabaseDdl(String instanceId, String databaseId) { - final String dbName = getDatabaseName(instanceId, databaseId); - Callable> callable = - new Callable>() { - @Override - public List call() throws Exception { - return spanner.getRpc().getDatabaseDdl(dbName); - } - }; - return spanner.runWithRetries(callable); + String dbName = getDatabaseName(instanceId, databaseId); + return spanner.getRpc().getDatabaseDdl(dbName); } @Override @@ -158,7 +136,7 @@ public Page listDatabases(String instanceId, ListOption... options) { !listOptions.hasFilter(), "Filter option is not support by" + "listDatabases"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher(spanner) { + new PageFetcher() { @Override public Paginated getNextPage( String nextPageToken) { diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java index 90b65c25fb51..dd31e165baaf 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java @@ -30,7 +30,6 @@ import com.google.protobuf.FieldMask; import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; -import java.util.concurrent.Callable; /** Default implementation of {@link InstanceAdminClient} */ class InstanceAdminClientImpl implements InstanceAdminClient { @@ -48,15 +47,9 @@ class InstanceAdminClientImpl implements InstanceAdminClient { @Override public InstanceConfig getInstanceConfig(String configId) throws SpannerException { - final String instanceConfigName = new InstanceConfigId(projectId, configId).getName(); - return spanner.runWithRetries( - new Callable() { - @Override - public InstanceConfig call() { - return InstanceConfig.fromProto( - spanner.getRpc().getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); - } - }); + String instanceConfigName = new InstanceConfigId(projectId, configId).getName(); + return InstanceConfig.fromProto( + spanner.getRpc().getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); } @Override @@ -66,7 +59,7 @@ public Page listInstanceConfigs(ListOption... options) { !listOptions.hasFilter(), "Filter option is not supported by listInstanceConfigs"); final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; PageFetcher pageFetcher = - new PageFetcher(spanner) { + new PageFetcher() { @Override public Paginated getNextPage( String nextPageToken) { @@ -91,7 +84,9 @@ public OperationFuture createInstance(Instance String projectName = PROJECT_NAME_TEMPLATE.instantiate("project", projectId); OperationFuture rawOperationFuture = - spanner.getRpc().createInstance(projectName, instance.getId().getInstance(), instance.toProto()); + spanner + .getRpc() + .createInstance(projectName, instance.getId().getInstance(), instance.toProto()); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), @@ -118,15 +113,9 @@ public Instance apply(Exception e) { @Override public Instance getInstance(String instanceId) throws SpannerException { - final String instanceName = new InstanceId(projectId, instanceId).getName(); - return spanner.runWithRetries( - new Callable() { - @Override - public Instance call() { - return Instance.fromProto( - spanner.getRpc().getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); - } - }); + String instanceName = new InstanceId(projectId, instanceId).getName(); + return Instance.fromProto( + spanner.getRpc().getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); } @Override @@ -135,7 +124,7 @@ public Page listInstances(ListOption... options) throws SpannerExcepti final int pageSize = listOptions.hasPageSize() ? listOptions.pageSize() : 0; final String filter = listOptions.filter(); PageFetcher pageFetcher = - new PageFetcher(spanner) { + new PageFetcher() { @Override public Paginated getNextPage( String nextPageToken) { @@ -155,14 +144,7 @@ public Instance fromProto(com.google.spanner.admin.instance.v1.Instance proto) { @Override public void deleteInstance(final String instanceId) throws SpannerException { - spanner.runWithRetries( - new Callable() { - @Override - public Void call() { - spanner.getRpc().deleteInstance(new InstanceId(projectId, instanceId).getName()); - return null; - } - }); + spanner.getRpc().deleteInstance(new InstanceId(projectId, instanceId).getName()); } @Override diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java index 237af748c9c7..0ded8849830c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java @@ -27,7 +27,6 @@ import com.google.spanner.v1.TransactionOptions; import com.google.spanner.v1.TransactionSelector; import java.util.Map; -import java.util.concurrent.Callable; /** Partitioned DML transaction for bulk updates and deletes. */ class PartitionedDMLTransaction implements SessionTransaction { @@ -50,14 +49,7 @@ private ByteString initTransaction() { TransactionOptions.newBuilder() .setPartitionedDml(TransactionOptions.PartitionedDml.getDefaultInstance())) .build(); - Transaction txn = - spanner.runWithRetries( - new Callable() { - @Override - public Transaction call() throws Exception { - return spanner.getRpc().beginTransaction(request, session.getOptions()); - } - }); + Transaction txn = spanner.getRpc().beginTransaction(request, session.getOptions()); if (txn.getId().isEmpty()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, @@ -83,13 +75,7 @@ long executePartitionedUpdate(Statement statement) { } } com.google.spanner.v1.ResultSet resultSet = - spanner.runWithRetries( - new Callable() { - @Override - public com.google.spanner.v1.ResultSet call() throws Exception { - return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); - } - }); + spanner.getRpc().executeQuery(builder.build(), session.getOptions()); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "Partitioned DML response missing stats possibly due to non-DML statement as input"); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index e446782b76c9..443146767e98 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -40,7 +40,6 @@ import java.util.Collection; import java.util.List; import java.util.Map; -import java.util.concurrent.Callable; import javax.annotation.Nullable; /** @@ -137,14 +136,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.runWithRetries( - new Callable() { - @Override - public CommitResponse call() throws Exception { - return spanner.getRpc().commit(request, options); - } - }); + CommitResponse response = spanner.getRpc().commit(request, options); Timestamp t = Timestamp.fromProto(response.getCommitTimestamp()); span.end(); return t; @@ -176,8 +168,7 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() { @Override public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) { return setActive( - new SingleUseReadOnlyTransaction( - this, bound, spanner, spanner.getDefaultPrefetchChunks())); + new SingleUseReadOnlyTransaction(this, bound, spanner, spanner.getDefaultPrefetchChunks())); } @Override @@ -188,8 +179,7 @@ public ReadOnlyTransaction readOnlyTransaction() { @Override public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) { return setActive( - new MultiUseReadOnlyTransaction( - this, bound, spanner, spanner.getDefaultPrefetchChunks())); + new MultiUseReadOnlyTransaction(this, bound, spanner, spanner.getDefaultPrefetchChunks())); } @Override @@ -208,14 +198,7 @@ public void prepareReadWriteTransaction() { public void close() { Span span = tracer.spanBuilder(SpannerImpl.DELETE_SESSION).startSpan(); try (Scope s = tracer.withSpan(span)) { - spanner.runWithRetries( - new Callable() { - @Override - public Void call() throws Exception { - spanner.getRpc().deleteSession(name, options); - return null; - } - }); + spanner.getRpc().deleteSession(name, options); span.end(); } catch (RuntimeException e) { TraceUtil.endSpanWithFailure(span, e); @@ -233,14 +216,7 @@ ByteString beginTransaction() { TransactionOptions.newBuilder() .setReadWrite(TransactionOptions.ReadWrite.getDefaultInstance())) .build(); - Transaction txn = - spanner.runWithRetries( - new Callable() { - @Override - public Transaction call() throws Exception { - return spanner.getRpc().beginTransaction(request, options); - } - }); + Transaction txn = spanner.getRpc().beginTransaction(request, options); if (txn.getId().isEmpty()) { throw newSpannerException(ErrorCode.INTERNAL, "Missing id in transaction\n" + getName()); } diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 69302aab5c53..5e239b8a5d6d 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -20,24 +20,7 @@ import static com.google.cloud.spanner.SpannerExceptionFactory.newSpannerExceptionForCancellation; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.Random; -import java.util.Set; -import java.util.concurrent.Callable; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Executor; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.annotation.Nullable; -import javax.annotation.concurrent.GuardedBy; + import com.google.api.client.util.BackOff; import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.paging.Page; @@ -45,12 +28,10 @@ import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; -import com.google.cloud.RetryHelper.RetryHelperException; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.util.concurrent.Futures; @@ -61,6 +42,21 @@ import io.opencensus.trace.Span; import io.opencensus.trace.Tracer; import io.opencensus.trace.Tracing; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Random; +import java.util.concurrent.CountDownLatch; +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; /** Default implementation of the Cloud Spanner interface. */ class SpannerImpl extends BaseService implements Spanner { @@ -158,68 +154,6 @@ public void cancelled(Context context) { } } - T runWithRetries(final Callable callable) { - try { - return callable.call(); - } catch (SpannerException e) { - throw e; - } catch (TimeoutException e) { - throw SpannerExceptionFactory.newSpannerException(ErrorCode.DEADLINE_EXCEEDED, e.getMessage(), e); - } catch (RetryHelperException e) { - if(e.getCause() != null) { - Throwables.throwIfUnchecked(e.getCause()); - } - throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e); - } catch (Exception e) { - if(e.getCause() != null) { - Throwable cause = e.getCause(); - if(cause instanceof RetryHelperException) { - cause = cause.getCause(); - } - Throwables.throwIfUnchecked(cause); - } - throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e); - } - } - - /** - * Helper to execute some work, retrying with backoff on retryable errors. - * - *

TODO: Consider replacing with RetryHelper from gcloud-core. - */ - T runWithRetries_WithContext(Callable callable, Set retryOnErrorCodes) { - // Use same backoff setting as abort, somewhat arbitrarily. - Span span = tracer.getCurrentSpan(); - ExponentialBackOff backOff = newBackOff(); - Context context = Context.current(); - int attempt = 0; - while (true) { - attempt++; - try { - span.addAnnotation( - "Starting operation", - ImmutableMap.of("Attempt", AttributeValue.longAttributeValue(attempt))); - return null; -// return runWithRetries( -// callable, retrySettings, new SpannerRetryAlgorithm<>(retryOnErrorCodes), clock); - } catch (SpannerException e) { - if (!e.isRetryable()) { - throw e; - } - logger.log(Level.FINE, "Retryable exception, will sleep and retry", e); - long delay = e.getRetryDelayInMillis(); - if (delay != -1) { - backoffSleep(context, delay); - } else { - backoffSleep(context, backOff); - } - } catch (Exception e) { - Throwables.throwIfUnchecked(e); - throw newSpannerException(ErrorCode.INTERNAL, "Unexpected exception thrown", e); - } - } - } - /** Returns the {@link SpannerRpc} of this {@link SpannerImpl} instance. */ SpannerRpc getRpc() { return gapicRpc; @@ -236,14 +170,7 @@ SessionImpl createSession(final DatabaseId db) throws SpannerException { Span span = tracer.spanBuilder(CREATE_SESSION).startSpan(); try (Scope s = tracer.withSpan(span)) { com.google.spanner.v1.Session session = - runWithRetries( - new Callable() { - @Override - public com.google.spanner.v1.Session call() throws Exception { - return gapicRpc.createSession( - db.getName(), getOptions().getSessionLabels(), options); - } - }); + gapicRpc.createSession(db.getName(), getOptions().getSessionLabels(), options); span.end(); return new SessionImpl(this, session.getName(), options); } catch (RuntimeException e) { @@ -368,23 +295,11 @@ Object value() { /** Helper class for gRPC calls that can return paginated results. */ abstract static class PageFetcher implements NextPageFetcher { - private final SpannerImpl spanner; private String nextPageToken; - PageFetcher(SpannerImpl spanner) { - this.spanner = spanner; - } - @Override public Page getNextPage() { - Paginated nextPage = - spanner.runWithRetries( - new Callable>() { - @Override - public Paginated call() { - return getNextPage(nextPageToken); - } - }); + Paginated nextPage = getNextPage(nextPageToken); this.nextPageToken = nextPage.getNextPageToken(); List results = new ArrayList<>(); for (T proto : nextPage.getResults()) { @@ -410,4 +325,4 @@ public void execute(Runnable command) { command.run(); } } -} \ No newline at end of file +} diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 8b89e2ba7003..8c60a37d5249 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -30,6 +30,8 @@ import com.google.cloud.ServiceRpc; import com.google.cloud.TransportOptions; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; @@ -38,6 +40,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import io.grpc.ManagedChannelBuilder; +import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; import java.util.Map; @@ -64,7 +67,9 @@ public class SpannerOptions extends ServiceOptions { private final int prefetchChunks; private final int numChannels; private final ImmutableMap sessionLabels; - private final SpannerStubSettings stubSettings; + private final SpannerStubSettings spannerStubSettings; + private final InstanceAdminStubSettings instanceAdminStubSettings; + private final DatabaseAdminStubSettings databaseAdminStubSettings; /** Default implementation of {@code SpannerFactory}. */ private static class DefaultSpannerFactory implements SpannerFactory { @@ -105,7 +110,9 @@ private SpannerOptions(Builder builder) { prefetchChunks = builder.prefetchChunks; sessionLabels = builder.sessionLabels; try { - stubSettings = builder.stubSettingsBuilder.build(); + spannerStubSettings = builder.spannerStubSettingsBuilder.build(); + instanceAdminStubSettings = builder.instanceAdminStubSettingsBuilder.build(); + databaseAdminStubSettings = builder.databaseAdminStubSettingsBuilder.build(); } catch (IOException e) { throw SpannerExceptionFactory.newSpannerException(e); } @@ -128,7 +135,12 @@ public static class Builder private int prefetchChunks = DEFAULT_PREFETCH_CHUNKS; private SessionPoolOptions sessionPoolOptions; private ImmutableMap sessionLabels; - private SpannerStubSettings.Builder stubSettingsBuilder = SpannerStubSettings.newBuilder(); + private SpannerStubSettings.Builder spannerStubSettingsBuilder = + SpannerStubSettings.newBuilder(); + private InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder = + InstanceAdminStubSettings.newBuilder(); + private DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder = + DatabaseAdminStubSettings.newBuilder(); private Builder() {} @@ -138,7 +150,9 @@ private Builder() {} this.sessionPoolOptions = options.sessionPoolOptions; this.prefetchChunks = options.prefetchChunks; this.sessionLabels = options.sessionLabels; - this.stubSettingsBuilder = options.stubSettings.toBuilder(); + this.spannerStubSettingsBuilder = options.spannerStubSettings.toBuilder(); + this.instanceAdminStubSettingsBuilder = options.instanceAdminStubSettings.toBuilder(); + this.databaseAdminStubSettingsBuilder = options.databaseAdminStubSettings.toBuilder(); this.channelProvider = options.channelProvider; this.channelConfigurator = options.channelConfigurator; this.interceptorProvider = options.interceptorProvider; @@ -219,12 +233,33 @@ public Builder setSessionLabels(Map sessionLabels) { @Override public Builder setRetrySettings(RetrySettings retrySettings) { - throw new UnsupportedOperationException("SpannerOptions does not support setting global retry settings. " - + "Call stubSettingsBuilder().Settings().setRetrySettings(RetrySettings) instead."); + throw new UnsupportedOperationException( + "SpannerOptions does not support setting global retry settings. " + + "Call stubSettingsBuilder().Settings().setRetrySettings(RetrySettings) instead."); + } + + /** + * Returns the {@link SpannerStubSettings.Builder} that will be used to build the {@link + * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + */ + public SpannerStubSettings.Builder spannerStubSettingsBuilder() { + return spannerStubSettingsBuilder; } - public SpannerStubSettings.Builder stubSettingsBuilder() { - return stubSettingsBuilder; + /** + * Returns the {@link InstanceAdminStubSettings.Builder} that will be used to build the {@link + * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + */ + public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { + return instanceAdminStubSettingsBuilder; + } + + /** + * Returns the {@link DatabaseAdminStubSettings.Builder} that will be used to build the {@link + * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + */ + public DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder() { + return databaseAdminStubSettingsBuilder; } /** @@ -283,8 +318,16 @@ public Map getSessionLabels() { return sessionLabels; } - public SpannerStubSettings getStubSettings() { - return stubSettings; + public SpannerStubSettings getSpannerStubSettings() { + return spannerStubSettings; + } + + public InstanceAdminStubSettings getInstanceAdminStubSettings() { + return instanceAdminStubSettings; + } + + public DatabaseAdminStubSettings getDatabaseAdminStubSettings() { + return databaseAdminStubSettings; } public int getPrefetchChunks() { diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index 3e166f26a8cd..b9c5b8e02299 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -44,7 +44,6 @@ import io.opencensus.trace.Tracing; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.Callable; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -125,14 +124,7 @@ void commit() { Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT, span).startSpan(); try (Scope s = tracer.withSpan(opSpan)) { CommitResponse commitResponse = - spanner.runWithRetries( - new Callable() { - @Override - public CommitResponse call() throws Exception { - return spanner.getRpc().commit(commitRequest, session.getOptions()); - } - }); - + spanner.getRpc().commit(commitRequest, session.getOptions()); if (!commitResponse.hasCommitTimestamp()) { throw newSpannerException( ErrorCode.INTERNAL, "Missing commitTimestamp:\n" + session.getName()); @@ -178,12 +170,14 @@ void rollback() { // response. Normally, the next thing that will happen is that we will make a fresh // transaction attempt, which should implicitly abort this one. span.addAnnotation("Starting Rollback"); - spanner.getRpc().rollback( - RollbackRequest.newBuilder() - .setSession(session.getName()) - .setTransactionId(transactionId) - .build(), - session.getOptions()); + spanner + .getRpc() + .rollback( + RollbackRequest.newBuilder() + .setSession(session.getName()) + .setTransactionId(transactionId) + .build(), + session.getOptions()); span.addAnnotation("Rollback Done"); } catch (SpannerException e) { txnLogger.log(Level.FINE, "Exception during rollback", e); @@ -239,13 +233,7 @@ public long executeUpdate(Statement statement) { final ExecuteSqlRequest.Builder builder = getExecuteSqlRequestBuilder(statement, QueryMode.NORMAL); com.google.spanner.v1.ResultSet resultSet = - spanner.runWithRetries( - new Callable() { - @Override - public com.google.spanner.v1.ResultSet call() throws Exception { - return spanner.getRpc().executeQuery(builder.build(), session.getOptions()); - } - }); + spanner.getRpc().executeQuery(builder.build(), session.getOptions()); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "DML response missing stats possibly due to non-DML statement as input"); @@ -259,13 +247,7 @@ public long[] batchUpdate(Iterable statements) { beforeReadOrQuery(); final ExecuteBatchDmlRequest.Builder builder = getExecuteBatchDmlRequestBuilder(statements); com.google.spanner.v1.ExecuteBatchDmlResponse response = - spanner.runWithRetries( - new Callable() { - @Override - public com.google.spanner.v1.ExecuteBatchDmlResponse call() throws Exception { - return spanner.getRpc().executeBatchDml(builder.build(), session.getOptions()); - } - }); + spanner.getRpc().executeBatchDml(builder.build(), session.getOptions()); long[] results = new long[response.getResultSetsCount()]; for (int i = 0; i < response.getResultSetsCount(); ++i) { results[i] = response.getResultSets(i).getStats().getRowCountExact(); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 64deaf014a1a..a6ca0e003ac5 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -18,7 +18,6 @@ import static com.google.cloud.spanner.SpannerExceptionFactory.newSpannerException; -import com.google.api.core.ApiFunction; import com.google.api.core.NanoClock; import com.google.api.gax.core.CredentialsProvider; import com.google.api.gax.core.ExecutorProvider; @@ -35,7 +34,6 @@ import com.google.api.gax.rpc.ResponseObserver; import com.google.api.gax.rpc.StreamController; import com.google.api.gax.rpc.TransportChannelProvider; -import com.google.api.gax.rpc.UnaryCallSettings; import com.google.api.gax.rpc.WatchdogProvider; import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.ServiceOptions; @@ -44,14 +42,11 @@ import com.google.cloud.spanner.SpannerExceptionFactory; import com.google.cloud.spanner.SpannerOptions; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStub; -import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; import com.google.cloud.spanner.admin.database.v1.stub.GrpcDatabaseAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.GrpcInstanceAdminStub; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStub; -import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.v1.stub.GrpcSpannerStub; import com.google.cloud.spanner.v1.stub.SpannerStub; -import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.base.MoreObjects; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ThreadFactoryBuilder; @@ -103,7 +98,6 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -253,54 +247,34 @@ public GapicSpannerRpc(final SpannerOptions options) { .withClock(NanoClock.getDefaultClock()); try { - // TODO: bump the version of gax and remove this try-catch block - // applyToAllUnaryMethods does not throw exception in the latest version - SpannerStubSettings.Builder builder = options.getStubSettings().toBuilder() - .setTransportChannelProvider(channelProvider) - .setCredentialsProvider(credentialsProvider) - .setStreamWatchdogProvider(watchdogProvider) -// .applyToAllUnaryMethods( -// new ApiFunction, Void>() { -// @Override -// public Void apply(UnaryCallSettings.Builder builder) { -// builder.setRetrySettings(options.getRetrySettings()); -// builder.setRetryableCodes(options.getRetryableCodes()); -// return null; -// } -// }) - ; this.spannerStub = - GrpcSpannerStub.create(builder.build()); + GrpcSpannerStub.create( + options + .getSpannerStubSettings() + .toBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(credentialsProvider) + .setStreamWatchdogProvider(watchdogProvider) + .build()); this.instanceAdminStub = GrpcInstanceAdminStub.create( - InstanceAdminStubSettings.newBuilder() + options + .getInstanceAdminStubSettings() + .toBuilder() .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) -// .applyToAllUnaryMethods( -// new ApiFunction, Void>() { -// @Override -// public Void apply(UnaryCallSettings.Builder builder) { -// builder.setRetryableCodes(options.getRetryableCodes()); -// return null; -// } -// }) .build()); + this.databaseAdminStub = GrpcDatabaseAdminStub.create( - DatabaseAdminStubSettings.newBuilder() + options + .getDatabaseAdminStubSettings() + .toBuilder() .setTransportChannelProvider(channelProvider) .setCredentialsProvider(credentialsProvider) .setStreamWatchdogProvider(watchdogProvider) -// .applyToAllUnaryMethods( -// new ApiFunction, Void>() { -// @Override -// public Void apply(UnaryCallSettings.Builder builder) { -// builder.setRetryableCodes(options.getRetryableCodes()); -// return null; -// } -// }) .build()); } catch (Exception e) { throw newSpannerException(e); @@ -610,7 +584,7 @@ private static T get(final Future future) throws SpannerException { // We are the sole consumer of the future, so cancel it. future.cancel(true); throw SpannerExceptionFactory.propagateInterrupt(e); - } catch (ExecutionException | CancellationException e) { + } catch (Exception e) { throw newSpannerException(context, e); } } diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index b0ab15e981c0..74a2214d68c1 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -21,15 +21,7 @@ import static org.mockito.Matchers.eq; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import java.util.Map; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.Mock; -import org.mockito.Mockito; + import com.google.api.core.NanoClock; import com.google.api.gax.retrying.RetrySettings; import com.google.cloud.Timestamp; @@ -39,6 +31,15 @@ import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.Session; import com.google.spanner.v1.Transaction; +import java.util.Map; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; /** Unit tests for {@link com.google.cloud.spanner.BatchClientImpl}. */ @RunWith(JUnit4.class) @@ -63,7 +64,8 @@ public void setUp() { when(spannerOptions.getPrefetchChunks()).thenReturn(1); when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); - SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); client = new BatchClientImpl(db, spanner); + SpannerImpl spanner = new SpannerImpl(gapicRpc, spannerOptions); + client = new BatchClientImpl(db, spanner); } @Test diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java index d1e971adf3f0..15c586327ce5 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java @@ -20,14 +20,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import java.util.Collections; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Mock; -import org.mockito.Mockito; + import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; import com.google.api.gax.retrying.RetrySettings; @@ -41,6 +34,14 @@ import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; import com.google.spanner.admin.database.v1.Database; import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; +import java.util.Collections; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.DatabaseAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -63,7 +64,8 @@ public void setUp() { when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); - client = new DatabaseAdminClientImpl(PROJECT_ID, spanner); } + client = new DatabaseAdminClientImpl(PROJECT_ID, spanner); + } private Database getDatabaseProto() { return Database.newBuilder().setName(DB_NAME).setState(Database.State.READY).build(); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java index fa01a252fa05..46338e2c5be6 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java @@ -20,13 +20,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import java.util.List; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.Mock; -import org.mockito.Mockito; + import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; import com.google.api.gax.retrying.RetrySettings; @@ -38,6 +32,13 @@ import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; import com.google.spanner.admin.instance.v1.InstanceConfig; import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; +import java.util.List; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.Mockito; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.InstanceAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -61,7 +62,8 @@ public void setUp() { when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); - client = new InstanceAdminClientImpl(PROJECT_ID, spanner, dbClient); } + client = new InstanceAdminClientImpl(PROJECT_ID, spanner, dbClient); + } @Test public void getInstanceConfig() { diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java new file mode 100644 index 000000000000..f099a783d498 --- /dev/null +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java @@ -0,0 +1,592 @@ +/* + * 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 + * + * https://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.api.core.ApiFunction; +import com.google.api.gax.grpc.testing.LocalChannelProvider; +import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.paging.Page; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.rpc.UnaryCallSettings.Builder; +import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.admin.instance.v1.MockInstanceAdminImpl; +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; +import com.google.protobuf.AbstractMessage; +import com.google.protobuf.Any; +import com.google.protobuf.Empty; +import com.google.spanner.admin.instance.v1.CreateInstanceMetadata; +import com.google.spanner.admin.instance.v1.InstanceConfig; +import com.google.spanner.admin.instance.v1.InstanceConfigName; +import com.google.spanner.admin.instance.v1.InstanceName; +import com.google.spanner.admin.instance.v1.ListInstanceConfigsRequest; +import com.google.spanner.admin.instance.v1.ListInstanceConfigsResponse; +import com.google.spanner.admin.instance.v1.ListInstancesRequest; +import com.google.spanner.admin.instance.v1.ListInstancesResponse; +import com.google.spanner.admin.instance.v1.ProjectName; +import com.google.spanner.admin.instance.v1.UpdateInstanceMetadata; +import io.grpc.Server; +import io.grpc.StatusRuntimeException; +import io.grpc.inprocess.InProcessServerBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.threeten.bp.Duration; + +@RunWith(Parameterized.class) +public class InstanceAdminGaxTest { + public static class DelayedStatusRuntimeException extends RuntimeException { + private final long millis; + private boolean hasWaited = false; + + public DelayedStatusRuntimeException(StatusRuntimeException cause, long millis) { + super(cause); + this.millis = millis; + } + + @Override + public synchronized Throwable getCause() { + if (!hasWaited) { + try { + Thread.sleep(millis); + hasWaited = true; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + return super.getCause(); + } + } + + private static final String PROJECT = "PROJECT"; + private static final StatusRuntimeException UNAVAILABLE = + io.grpc.Status.UNAVAILABLE.withDescription("Retryable test exception.").asRuntimeException(); + private static final StatusRuntimeException FAILED_PRECONDITION = + io.grpc.Status.FAILED_PRECONDITION + .withDescription("Non-retryable test exception.") + .asRuntimeException(); + + private static Exception createDelayedInternal() { + return new DelayedStatusRuntimeException( + io.grpc.Status.INTERNAL.withDescription("Delayed test exception.").asRuntimeException(), + 500L); + } + + public static enum ExceptionType { + RETRYABLE { + @Override + public Exception getException() { + return UNAVAILABLE; + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return null; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.UNAVAILABLE; + } + + @Override + public boolean isRetryable() { + return true; + } + }, + NON_RETRYABLE { + @Override + public Exception getException() { + return FAILED_PRECONDITION; + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return ErrorCode.FAILED_PRECONDITION; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.FAILED_PRECONDITION; + } + + @Override + public boolean isRetryable() { + return false; + } + }, + DELAYED { + @Override + public Exception getException() { + return createDelayedInternal(); + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return ErrorCode.DEADLINE_EXCEEDED; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.INTERNAL; + } + + @Override + public boolean isRetryable() { + return true; + } + }; + + public abstract Exception getException(); + + public abstract ErrorCode getExpectedErrorCodeWithGax(); + + public abstract ErrorCode getExpectedErrorCodeWithoutGax(); + + public abstract boolean isRetryable(); + } + + private static MockInstanceAdminImpl mockInstanceAdmin; + private static Server server; + private static Spanner spanner; + private static InstanceAdminClient client; + private static LocalChannelProvider channelProvider; + + @Parameter(0) + public boolean enableGaxRetries; + + @Parameter(1) + public int exceptionAtCall; + + @Parameter(2) + public ExceptionType exceptionType; + + @Parameters(name = "enable GAX retries = {0}, exception at call = {1}, exception type = {2}") + public static Collection data() { + List params = new ArrayList<>(); + for (boolean enableRetries : new boolean[] {true, false}) { + for (int exceptionAtCall : new int[] {0, 1}) { + for (ExceptionType exceptionType : ExceptionType.values()) { + params.add(new Object[] {enableRetries, exceptionAtCall, exceptionType}); + } + } + } + return params; + } + + @Rule public ExpectedException expectedException = ExpectedException.none(); + + @BeforeClass + public static void startStaticServer() throws IOException { + mockInstanceAdmin = new MockInstanceAdminImpl(); + String uniqueName = InProcessServerBuilder.generateName(); + server = + InProcessServerBuilder.forName(uniqueName) + // We need to use a real executor for timeouts to occur. + .scheduledExecutorService(new ScheduledThreadPoolExecutor(1)) + .addService(mockInstanceAdmin) + .build() + .start(); + channelProvider = LocalChannelProvider.create(uniqueName); + } + + @AfterClass + public static void stopServer() { + server.shutdown(); + } + + @Before + public void setUp() throws Exception { + mockInstanceAdmin.reset(); + final RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(Duration.ofMillis(50L)) + .setMaxRpcTimeout(Duration.ofMillis(50L)) + .setMaxAttempts(3) + .setTotalTimeout(Duration.ofMillis(250L)) + .build(); + SpannerOptions.Builder builder = + SpannerOptions.newBuilder() + .setProjectId(PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()); + builder + .instanceAdminStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetrySettings(retrySettings); + return null; + } + }); + builder + .instanceAdminStubSettingsBuilder() + .createInstanceOperationSettings() + .setInitialCallSettings( + builder + .instanceAdminStubSettingsBuilder() + .createInstanceOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .build()); + builder + .instanceAdminStubSettingsBuilder() + .updateInstanceOperationSettings() + .setInitialCallSettings( + builder + .instanceAdminStubSettingsBuilder() + .updateInstanceOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .build()); + if (!enableGaxRetries) { + // Disable retries by removing all retryable codes. + builder + .instanceAdminStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetryableCodes(ImmutableSet.of()); + return null; + } + }); + builder + .instanceAdminStubSettingsBuilder() + .createInstanceOperationSettings() + .setInitialCallSettings( + builder + .instanceAdminStubSettingsBuilder() + .createInstanceOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of()) + .build()); + builder + .instanceAdminStubSettingsBuilder() + .updateInstanceOperationSettings() + .setInitialCallSettings( + builder + .instanceAdminStubSettingsBuilder() + .updateInstanceOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of()) + .build()); + } + spanner = builder.build().getService(); + client = spanner.getInstanceAdminClient(); + } + + @After + public void tearDown() throws Exception { + spanner.close(); + } + + private Exception setupException() { + if (exceptionType.isRetryable()) { + if (!enableGaxRetries) { + expectedException.expect( + SpannerMatchers.isSpannerException(exceptionType.getExpectedErrorCodeWithoutGax())); + } + } else { + expectedException.expect( + SpannerMatchers.isSpannerException(exceptionType.getExpectedErrorCodeWithGax())); + } + return exceptionType.getException(); + } + + @Test + public void listInstanceConfigsTest() { + Exception exception = setupException(); + String nextPageToken = "token%d"; + List configs = new ArrayList<>(2); + for (int i = 0; i < 2; i++) { + configs.add( + InstanceConfig.newBuilder() + .setDisplayName(String.format("TEST%d", i)) + .setName(String.format("projects/%s/instanceConfigs/test%d", PROJECT, i)) + .build()); + } + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + for (int i = 0; i < 2; i++) { + ListInstanceConfigsResponse.Builder builder = + ListInstanceConfigsResponse.newBuilder() + .addAllInstanceConfigs(Arrays.asList(configs.get(i))); + if (i < (configs.size() - 1)) { + builder.setNextPageToken(String.format(nextPageToken, i)); + } + if (exceptionAtCall == (i + 1)) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(builder.build()); + } + ProjectName parent = ProjectName.of(PROJECT); + Page pagedListResponse = client.listInstanceConfigs(); + List resources = + Lists.newArrayList(pagedListResponse.iterateAll()); + Assert.assertEquals(2, resources.size()); + + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + ListInstanceConfigsRequest actualRequest = (ListInstanceConfigsRequest) actualRequests.get(0); + + Assert.assertEquals(parent, ProjectName.parse(actualRequest.getParent())); + } + + @Test + public void getInstanceConfigTest() { + Exception exception = setupException(); + for (int i = 0; i < 2; i++) { + InstanceConfigName name2 = InstanceConfigName.of(PROJECT, "INSTANCE_CONFIG"); + String displayName = "displayName1615086568"; + InstanceConfig expectedResponse = + InstanceConfig.newBuilder().setName(name2.toString()).setDisplayName(displayName).build(); + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(expectedResponse); + if (exceptionAtCall == 1) { + mockInstanceAdmin.addException(exception); + } + + InstanceConfigName name = InstanceConfigName.of(PROJECT, "INSTANCE_CONFIG"); + com.google.cloud.spanner.InstanceConfig actualResponse = + client.getInstanceConfig(name.toString()); + + Assert.assertEquals(displayName, actualResponse.getDisplayName()); + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(i + 1, actualRequests.size()); + } + } + + @Test + public void listInstancesTest() { + Exception exception = setupException(); + String nextPageToken = "token%d"; + List instances = new ArrayList<>(2); + for (int i = 0; i < 2; i++) { + instances.add( + com.google.spanner.admin.instance.v1.Instance.newBuilder() + .setDisplayName(String.format("TEST%d", i)) + .setName(String.format("projects/%s/instances/test%d", PROJECT, i)) + .setConfig(String.format("projects/%s/instanceConfigs/test%d", PROJECT, i)) + .build()); + } + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + for (int i = 0; i < 2; i++) { + ListInstancesResponse.Builder builder = + ListInstancesResponse.newBuilder().addAllInstances(Arrays.asList(instances.get(i))); + if (i < (instances.size() - 1)) { + builder.setNextPageToken(String.format(nextPageToken, i)); + } + if (exceptionAtCall == (i + 1)) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(builder.build()); + } + + ProjectName parent = ProjectName.of(PROJECT); + Page pagedListResponse = client.listInstances(); + + List resources = Lists.newArrayList(pagedListResponse.iterateAll()); + Assert.assertEquals(2, resources.size()); + + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + ListInstancesRequest actualRequest = (ListInstancesRequest) actualRequests.get(0); + + Assert.assertEquals(parent, ProjectName.parse(actualRequest.getParent())); + } + + @Test + public void getInstanceTest() { + Exception exception = setupException(); + InstanceName name2 = InstanceName.of(PROJECT, "INSTANCE"); + String displayName = "displayName1615086568"; + InstanceConfigName config = InstanceConfigName.of(PROJECT, "INSTANCE_CONFIG"); + com.google.spanner.admin.instance.v1.Instance expectedResponse = + com.google.spanner.admin.instance.v1.Instance.newBuilder() + .setName(name2.toString()) + .setConfig(config.toString()) + .setDisplayName(displayName) + .setNodeCount(3) + .build(); + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(expectedResponse); + if (exceptionAtCall == 1) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(expectedResponse); + + InstanceName name = InstanceName.of(PROJECT, "INSTANCE"); + for (int i = 0; i < 2; i++) { + Instance actualResponse = client.getInstance(name.toString()); + Assert.assertEquals(displayName, actualResponse.getDisplayName()); + } + + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void createInstanceTest() throws Exception { + Exception exception = setupException(); + InstanceName name = InstanceName.of(PROJECT, "INSTANCE"); + InstanceConfigName config = InstanceConfigName.of(PROJECT, "INSTANCE_CONFIG"); + String displayName = "displayName1615086568"; + int nodeCount = 1539922066; + com.google.spanner.admin.instance.v1.Instance expectedResponse = + com.google.spanner.admin.instance.v1.Instance.newBuilder() + .setName(name.toString()) + .setConfig(config.toString()) + .setDisplayName(displayName) + .setNodeCount(nodeCount) + .build(); + com.google.longrunning.Operation resultOperation = + com.google.longrunning.Operation.newBuilder() + .setName("createInstanceTest") + .setDone(true) + .setResponse(Any.pack(expectedResponse)) + .build(); + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(resultOperation); + if (exceptionAtCall == 1) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(resultOperation); + + for (int i = 0; i < 2; i++) { + OperationFuture actualResponse = + client.createInstance( + InstanceInfo.newBuilder(InstanceId.of(PROJECT, "INSTANCE")) + .setDisplayName(displayName) + .setNodeCount(nodeCount) + .build()); + try { + Instance returnedInstance = actualResponse.get(); + Assert.assertEquals(displayName, returnedInstance.getDisplayName()); + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + throw e; + } + } + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void updateInstanceTest() throws Exception { + Exception exception = setupException(); + InstanceName name = InstanceName.of(PROJECT, "INSTANCE"); + InstanceConfigName config = InstanceConfigName.of(PROJECT, "INSTANCE_CONFIG"); + String displayName = "displayName1615086568"; + int nodeCount = 1539922066; + com.google.spanner.admin.instance.v1.Instance expectedResponse = + com.google.spanner.admin.instance.v1.Instance.newBuilder() + .setName(name.toString()) + .setConfig(config.toString()) + .setDisplayName(displayName) + .setNodeCount(nodeCount) + .build(); + com.google.longrunning.Operation resultOperation = + com.google.longrunning.Operation.newBuilder() + .setName("updateInstanceTest") + .setDone(true) + .setResponse(Any.pack(expectedResponse)) + .build(); + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(resultOperation); + if (exceptionAtCall == 1) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(resultOperation); + + for (int i = 0; i < 2; i++) { + OperationFuture actualResponse = + client.updateInstance( + InstanceInfo.newBuilder(InstanceId.of(PROJECT, "INSTANCE")) + .setDisplayName(displayName) + .setNodeCount(nodeCount) + .build()); + try { + Instance returnedInstance = actualResponse.get(); + Assert.assertEquals(displayName, returnedInstance.getDisplayName()); + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + throw e; + } + } + + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void deleteInstanceTest() { + Exception exception = setupException(); + Empty expectedResponse = Empty.newBuilder().build(); + if (exceptionAtCall == 0) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(expectedResponse); + if (exceptionAtCall == 1) { + mockInstanceAdmin.addException(exception); + } + mockInstanceAdmin.addResponse(expectedResponse); + for (int i = 0; i < 2; i++) { + client.deleteInstance("INSTANCE"); + } + + List actualRequests = mockInstanceAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } +} diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java index ad62c2f3667a..cbed03e5bdfc 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionImplTest.java @@ -19,6 +19,24 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; import static org.mockito.Mockito.when; + +import com.google.api.core.NanoClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.Timestamp; +import com.google.cloud.spanner.TransactionRunner.TransactionCallable; +import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.protobuf.ByteString; +import com.google.protobuf.ListValue; +import com.google.protobuf.util.Timestamps; +import com.google.spanner.v1.BeginTransactionRequest; +import com.google.spanner.v1.CommitRequest; +import com.google.spanner.v1.CommitResponse; +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.Transaction; import java.text.ParseException; import java.util.Arrays; import java.util.Calendar; @@ -40,23 +58,6 @@ import org.mockito.MockitoAnnotations; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; -import com.google.api.core.NanoClock; -import com.google.api.gax.retrying.RetrySettings; -import com.google.cloud.Timestamp; -import com.google.cloud.spanner.TransactionRunner.TransactionCallable; -import com.google.cloud.spanner.spi.v1.SpannerRpc; -import com.google.protobuf.ByteString; -import com.google.protobuf.ListValue; -import com.google.protobuf.util.Timestamps; -import com.google.spanner.v1.BeginTransactionRequest; -import com.google.spanner.v1.CommitRequest; -import com.google.spanner.v1.CommitResponse; -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.Transaction; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.SessionImpl}. */ @RunWith(JUnit4.class) @@ -77,7 +78,8 @@ public void setUp() { when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); @SuppressWarnings("resource") - SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); String dbName = "projects/p1/instances/i1/databases/d1"; + SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); + String dbName = "projects/p1/instances/i1/databases/d1"; String sessionName = dbName + "/sessions/s1"; DatabaseId db = DatabaseId.of(dbName); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java similarity index 58% rename from google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java rename to google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index ba9a0bbb8e79..aac653b16a82 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -19,26 +19,7 @@ import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.is; import static org.junit.Assert.assertThat; -import java.io.IOException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.concurrent.ScheduledThreadPoolExecutor; -import javax.net.ssl.SSLHandshakeException; -import org.junit.After; -import org.junit.AfterClass; -import org.junit.Before; -import org.junit.BeforeClass; -import org.junit.Ignore; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; -import org.junit.runners.Parameterized.Parameter; -import org.junit.runners.Parameterized.Parameters; -import org.threeten.bp.Duration; + import com.google.api.core.ApiFunction; import com.google.api.gax.core.NoCredentialsProvider; import com.google.api.gax.grpc.testing.LocalChannelProvider; @@ -61,48 +42,102 @@ import io.grpc.Server; import io.grpc.StatusRuntimeException; import io.grpc.inprocess.InProcessServerBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import javax.net.ssl.SSLHandshakeException; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.threeten.bp.Duration; @RunWith(Parameterized.class) -public class SpannerImplGaxRetryTest { - private static final ResultSetMetadata READ_METADATA = ResultSetMetadata.newBuilder() - .setRowType(StructType.newBuilder().addFields(Field.newBuilder().setName("BAR") - .setType(com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.INT64).build()).build()) - .build()) - .build(); +public class SpannerGaxRetryTest { + private static final ResultSetMetadata READ_METADATA = + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("BAR") + .setType( + com.google.spanner.v1.Type.newBuilder() + .setCode(TypeCode.INT64) + .build()) + .build()) + .build()) + .build(); private static final com.google.spanner.v1.ResultSet READ_RESULTSET = - com.google.spanner.v1.ResultSet.newBuilder().addRows(ListValue.newBuilder() - .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()).build()) - .addRows(ListValue.newBuilder() - .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) - .build()) - .setMetadata(READ_METADATA).build(); + com.google.spanner.v1.ResultSet.newBuilder() + .addRows( + ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) + .build()) + .setMetadata(READ_METADATA) + .build(); private static final com.google.spanner.v1.ResultSet READ_ROW_RESULTSET = com.google.spanner.v1.ResultSet.newBuilder() - .addRows(ListValue.newBuilder() - .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()) - .build()) - .setMetadata(READ_METADATA).build(); + .addRows( + ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()) + .build()) + .setMetadata(READ_METADATA) + .build(); private static final Statement SELECT1AND2 = Statement.of("SELECT 1 AS COL1 UNION ALL SELECT 2 AS COL1"); - private static final ResultSetMetadata SELECT1AND2_METADATA = ResultSetMetadata.newBuilder() - .setRowType(StructType.newBuilder().addFields(Field.newBuilder().setName("COL1") - .setType(com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.INT64).build()).build()) - .build()) - .build(); + private static final ResultSetMetadata SELECT1AND2_METADATA = + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + Field.newBuilder() + .setName("COL1") + .setType( + com.google.spanner.v1.Type.newBuilder() + .setCode(TypeCode.INT64) + .build()) + .build()) + .build()) + .build(); private static final com.google.spanner.v1.ResultSet SELECT1_RESULTSET = - com.google.spanner.v1.ResultSet.newBuilder().addRows(ListValue.newBuilder() - .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()).build()) - .addRows(ListValue.newBuilder() - .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) - .build()) - .setMetadata(SELECT1AND2_METADATA).build(); + com.google.spanner.v1.ResultSet.newBuilder() + .addRows( + ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("1").build()) + .build()) + .addRows( + ListValue.newBuilder() + .addValues(com.google.protobuf.Value.newBuilder().setStringValue("2").build()) + .build()) + .setMetadata(SELECT1AND2_METADATA) + .build(); private static final Statement UPDATE_STATEMENT = Statement.of("UPDATE FOO SET BAR=1 WHERE BAZ=2"); private static final long UPDATE_COUNT = 1L; private static final SimulatedExecutionTime ONE_SECOND = SimulatedExecutionTime.ofMinimumAndRandomTime(1000, 0); private static final StatusRuntimeException UNAVAILABLE = - io.grpc.Status.UNAVAILABLE.withDescription("TEST").asRuntimeException(); + io.grpc.Status.UNAVAILABLE.withDescription("Retryable test exception.").asRuntimeException(); + private static final StatusRuntimeException FAILED_PRECONDITION = + io.grpc.Status.FAILED_PRECONDITION + .withDescription("Non-retryable test exception.") + .asRuntimeException(); private static MockSpannerServiceImpl mockSpanner; private static Server server; private static LocalChannelProvider channelProvider; @@ -121,8 +156,7 @@ public static Collection data() { return params; } - @Rule - public ExpectedException expectedException = ExpectedException.none(); + @Rule public ExpectedException expectedException = ExpectedException.none(); @BeforeClass public static void startStaticServer() throws IOException { @@ -130,21 +164,27 @@ public static void startStaticServer() throws IOException { mockSpanner.setAbortProbability(0.0D); // We don't want any unpredictable aborted transactions. mockSpanner.putStatementResult( StatementResult.read("FOO", KeySet.all(), Arrays.asList("BAR"), READ_RESULTSET)); - mockSpanner.putStatementResult(StatementResult.read("FOO", KeySet.singleKey(Key.of()), - Arrays.asList("BAR"), READ_ROW_RESULTSET)); + mockSpanner.putStatementResult( + StatementResult.read( + "FOO", KeySet.singleKey(Key.of()), Arrays.asList("BAR"), READ_ROW_RESULTSET)); mockSpanner.putStatementResult(StatementResult.query(SELECT1AND2, SELECT1_RESULTSET)); mockSpanner.putStatementResult(StatementResult.update(UPDATE_STATEMENT, UPDATE_COUNT)); String uniqueName = InProcessServerBuilder.generateName(); - server = InProcessServerBuilder.forName(uniqueName) - // We need to use a real executor for timeouts to occur. - .scheduledExecutorService(new ScheduledThreadPoolExecutor(4)).addService(mockSpanner) - .build().start(); + server = + InProcessServerBuilder.forName(uniqueName) + // We need to use a real executor for timeouts to occur. + .scheduledExecutorService(new ScheduledThreadPoolExecutor(1)) + .addService(mockSpanner) + .build() + .start(); channelProvider = LocalChannelProvider.create(uniqueName); SpannerSettings settings = - SpannerSettings.newBuilder().setTransportChannelProvider(channelProvider) - .setCredentialsProvider(NoCredentialsProvider.create()).build(); + SpannerSettings.newBuilder() + .setTransportChannelProvider(channelProvider) + .setCredentialsProvider(NoCredentialsProvider.create()) + .build(); spannerClient = SpannerClient.create(settings); } @@ -158,34 +198,52 @@ public static void stopServer() { public void setUp() throws Exception { mockSpanner.reset(); mockSpanner.removeAllExecutionTimes(); - final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(100L)).setMaxRpcTimeout(Duration.ofMillis(100L)) - .setMaxAttempts(3).setTotalTimeout(Duration.ofMillis(500L)).build(); - SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("[PROJECT]") - .setChannelProvider(channelProvider).setCredentials(NoCredentials.getInstance()); - builder.stubSettingsBuilder() - .applyToAllUnaryMethods(new ApiFunction, Void>() { - @Override - public Void apply(Builder input) { - input.setRetrySettings(retrySettings); - return null; - } - }); - builder.stubSettingsBuilder().executeStreamingSqlSettings().setRetrySettings(retrySettings); - builder.stubSettingsBuilder().streamingReadSettings().setRetrySettings(retrySettings); + final RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(Duration.ofMillis(100L)) + .setMaxRpcTimeout(Duration.ofMillis(100L)) + .setMaxAttempts(3) + .setTotalTimeout(Duration.ofMillis(500L)) + .build(); + SpannerOptions.Builder builder = + SpannerOptions.newBuilder() + .setProjectId("[PROJECT]") + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()); + builder + .spannerStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetrySettings(retrySettings); + return null; + } + }); + builder + .spannerStubSettingsBuilder() + .executeStreamingSqlSettings() + .setRetrySettings(retrySettings); + builder.spannerStubSettingsBuilder().streamingReadSettings().setRetrySettings(retrySettings); if (!enableGaxRetries) { // Disable retries by removing all retryable codes. - builder.stubSettingsBuilder() - .applyToAllUnaryMethods(new ApiFunction, Void>() { - @Override - public Void apply(Builder input) { - input.setRetryableCodes(ImmutableSet.of()); - return null; - } - }); - builder.stubSettingsBuilder().executeStreamingSqlSettings() + builder + .spannerStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetryableCodes(ImmutableSet.of()); + return null; + } + }); + builder + .spannerStubSettingsBuilder() + .executeStreamingSqlSettings() .setRetryableCodes(ImmutableSet.of()); - builder.stubSettingsBuilder().streamingReadSettings() + builder + .spannerStubSettingsBuilder() + .streamingReadSettings() .setRetryableCodes(ImmutableSet.of()); } spanner = builder.build().getService(); @@ -198,14 +256,13 @@ public void tearDown() throws Exception { } @Test - public void singleUse() { + public void singleUseTimeout() { if (enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); } mockSpanner.setCreateSessionExecutionTime(ONE_SECOND); try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { - while (rs.next()) { - } + while (rs.next()) {} } } @@ -216,20 +273,45 @@ public void singleUseUnavailable() { } mockSpanner.addException(UNAVAILABLE); try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { - while (rs.next()) { - } + while (rs.next()) {} + } + } + + @Test + public void singleUseNonRetryableError() { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.FAILED_PRECONDITION)); + mockSpanner.addException(FAILED_PRECONDITION); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + while (rs.next()) {} } } @Test - public void singleUseReadOnlyTransaction() { + public void singleUseNonRetryableErrorOnNext() { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.FAILED_PRECONDITION)); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + mockSpanner.addException(FAILED_PRECONDITION); + while (rs.next()) {} + } + } + + @Test + public void singleUseInternal() { + expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.INTERNAL)); + mockSpanner.addException(new IllegalArgumentException()); + try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { + while (rs.next()) {} + } + } + + @Test + public void singleUseReadOnlyTransactionTimeout() { if (enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); } mockSpanner.setCreateSessionExecutionTime(ONE_SECOND); try (ResultSet rs = client.singleUseReadOnlyTransaction().executeQuery(SELECT1AND2)) { - while (rs.next()) { - } + while (rs.next()) {} } } @@ -240,8 +322,7 @@ public void singleUseReadOnlyTransactionUnavailable() { } mockSpanner.addException(UNAVAILABLE); try (ResultSet rs = client.singleUseReadOnlyTransaction().executeQuery(SELECT1AND2)) { - while (rs.next()) { - } + while (rs.next()) {} } } @@ -250,8 +331,7 @@ public void singleUseExecuteStreamingSqlTimeout() { // Streaming calls do not timeout. mockSpanner.setExecuteStreamingSqlExecutionTime(ONE_SECOND); try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { - while (rs.next()) { - } + while (rs.next()) {} } } @@ -260,8 +340,7 @@ public void singleUseExecuteStreamingSqlUnavailable() { // executeStreamingSql is always retried. try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { mockSpanner.addException(UNAVAILABLE); - while (rs.next()) { - } + while (rs.next()) {} } } @@ -272,12 +351,14 @@ public void readWriteTransactionTimeout() { } mockSpanner.setBeginTransactionExecutionTime(ONE_SECOND); TransactionRunner runner = client.readWriteTransaction(); - long updateCount = runner.run(new TransactionCallable() { - @Override - public Long run(TransactionContext transaction) throws Exception { - return transaction.executeUpdate(UPDATE_STATEMENT); - } - }); + long updateCount = + runner.run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); assertThat(updateCount, is(equalTo(UPDATE_COUNT))); } @@ -288,12 +369,14 @@ public void readWriteTransactionUnavailable() { } mockSpanner.addException(UNAVAILABLE); TransactionRunner runner = client.readWriteTransaction(); - long updateCount = runner.run(new TransactionCallable() { - @Override - public Long run(TransactionContext transaction) throws Exception { - return transaction.executeUpdate(UPDATE_STATEMENT); - } - }); + long updateCount = + runner.run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); assertThat(updateCount, is(equalTo(UPDATE_COUNT))); } @@ -339,7 +422,7 @@ public void transactionManagerUnavailable() { } } - //TODO(loite): Check what happens when an SSLHandshakeException is thrown when using GAX. + // TODO(loite): Check what happens when an SSLHandshakeException is thrown when using GAX. @Test @Ignore public void singleUseSslHandshakeException() { diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index ff5d5a641309..dd8d3068ac9c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -17,14 +17,15 @@ package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.mockito.Mockito.when; + +import com.google.api.core.NanoClock; +import com.google.api.gax.retrying.RetrySettings; +import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import java.util.HashMap; import java.util.Map; -import java.util.concurrent.Callable; -import javax.net.ssl.SSLHandshakeException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -34,10 +35,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; -import com.google.api.core.NanoClock; -import com.google.api.gax.retrying.RetrySettings; -import com.google.cloud.grpc.GrpcTransportOptions; -import com.google.cloud.spanner.spi.v1.SpannerRpc; /** Unit tests for {@link SpannerImpl}. */ @RunWith(JUnit4.class) @@ -125,20 +122,4 @@ public void getDbclientAfterCloseThrows() { assertThat(e.getMessage()).contains("Cloud Spanner client has been closed"); } } - - @Test - public void exceptionIsTranslated() { - try { - impl.runWithRetries( - new Callable() { - @Override - public Void call() throws Exception { - throw new Exception("Should be translated to SpannerException"); - } - }); - } catch (SpannerException e) { - assertThat(e.getErrorCode()).isEqualTo(ErrorCode.INTERNAL); - assertThat(e.getMessage().contains("Unexpected exception thrown")); - } - } } From e99b45d1b9adb9ff9946ab9827f0c582448e7420 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 15 Apr 2019 14:46:36 +0200 Subject: [PATCH 06/26] try to fix flaky test --- .../java/com/google/cloud/spanner/SpannerGaxRetryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index aac653b16a82..d0d64198b1a9 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,8 +200,8 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(100L)) - .setMaxRpcTimeout(Duration.ofMillis(100L)) + .setInitialRpcTimeout(Duration.ofMillis(200L)) + .setMaxRpcTimeout(Duration.ofMillis(200L)) .setMaxAttempts(3) .setTotalTimeout(Duration.ofMillis(500L)) .build(); From de22aed7a61b35d9779f3d7365301cb08b2850ca Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 15 Apr 2019 14:47:01 +0200 Subject: [PATCH 07/26] added gax config for DatabaseAdminClient --- .../google/cloud/spanner/SpannerOptions.java | 14 + .../cloud/spanner/DatabaseAdminGaxTest.java | 489 ++++++++++++++++++ 2 files changed, 503 insertions(+) create mode 100644 google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 8c60a37d5249..a59d24ff4f3e 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -30,11 +30,13 @@ import com.google.cloud.ServiceRpc; import com.google.cloud.TransportOptions; import com.google.cloud.grpc.GrpcTransportOptions; +import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc; +import com.google.cloud.spanner.v1.SpannerSettings; import com.google.cloud.spanner.v1.stub.SpannerStubSettings; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; @@ -241,6 +243,10 @@ public Builder setRetrySettings(RetrySettings retrySettings) { /** * Returns the {@link SpannerStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * + *

The library will automatically use sensible defaults if no custom settings are set. The + * defaults are the same as the defaults that are used by {@link SpannerSettings}. Retries are + * configured for idempotent methods but not for non-idempotent methods. */ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { return spannerStubSettingsBuilder; @@ -249,6 +255,10 @@ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { /** * Returns the {@link InstanceAdminStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * + *

The library will automatically use sensible defaults if no custom settings are set. The + * defaults are the same as the defaults that are used by {@link InstanceAdminSettings}. Retries + * are configured for idempotent methods but not for non-idempotent methods. */ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { return instanceAdminStubSettingsBuilder; @@ -257,6 +267,10 @@ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { /** * Returns the {@link DatabaseAdminStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * + *

The library will automatically use sensible defaults if no custom settings are set. The + * defaults are the same as the defaults that are used by {@link DatabaseAdminSettings}. Retries + * are configured for idempotent methods but not for non-idempotent methods. */ public DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder() { return databaseAdminStubSettingsBuilder; diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java new file mode 100644 index 000000000000..49b8ab311a60 --- /dev/null +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java @@ -0,0 +1,489 @@ +/* + * 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 + * + * https://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.api.core.ApiFunction; +import com.google.api.gax.grpc.testing.LocalChannelProvider; +import com.google.api.gax.longrunning.OperationFuture; +import com.google.api.gax.paging.Page; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.StatusCode; +import com.google.api.gax.rpc.UnaryCallSettings; +import com.google.api.gax.rpc.UnaryCallSettings.Builder; +import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.admin.database.v1.MockDatabaseAdminImpl; +import com.google.common.base.Throwables; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; +import com.google.protobuf.AbstractMessage; +import com.google.protobuf.Any; +import com.google.protobuf.Empty; +import com.google.spanner.admin.database.v1.CreateDatabaseMetadata; +import com.google.spanner.admin.database.v1.DatabaseName; +import com.google.spanner.admin.database.v1.ListDatabasesRequest; +import com.google.spanner.admin.database.v1.ListDatabasesResponse; +import com.google.spanner.admin.database.v1.UpdateDatabaseDdlMetadata; +import com.google.spanner.admin.instance.v1.InstanceName; +import io.grpc.Server; +import io.grpc.StatusRuntimeException; +import io.grpc.inprocess.InProcessServerBuilder; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import org.junit.After; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; +import org.junit.runners.Parameterized.Parameters; +import org.threeten.bp.Duration; + +@RunWith(Parameterized.class) +public class DatabaseAdminGaxTest { + public static class DelayedStatusRuntimeException extends RuntimeException { + private final long millis; + private boolean hasWaited = false; + + public DelayedStatusRuntimeException(StatusRuntimeException cause, long millis) { + super(cause); + this.millis = millis; + } + + @Override + public synchronized Throwable getCause() { + if (!hasWaited) { + try { + Thread.sleep(millis); + hasWaited = true; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + return super.getCause(); + } + } + + private static final String PROJECT = "PROJECT"; + private static final String INSTANCE = "INSTANCE"; + private static final StatusRuntimeException UNAVAILABLE = + io.grpc.Status.UNAVAILABLE.withDescription("Retryable test exception.").asRuntimeException(); + private static final StatusRuntimeException FAILED_PRECONDITION = + io.grpc.Status.FAILED_PRECONDITION + .withDescription("Non-retryable test exception.") + .asRuntimeException(); + + private static Exception createDelayedInternal() { + return new DelayedStatusRuntimeException( + io.grpc.Status.INTERNAL.withDescription("Delayed test exception.").asRuntimeException(), + 500L); + } + + public static enum ExceptionType { + RETRYABLE { + @Override + public Exception getException() { + return UNAVAILABLE; + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return null; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.UNAVAILABLE; + } + + @Override + public boolean isRetryable() { + return true; + } + }, + NON_RETRYABLE { + @Override + public Exception getException() { + return FAILED_PRECONDITION; + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return ErrorCode.FAILED_PRECONDITION; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.FAILED_PRECONDITION; + } + + @Override + public boolean isRetryable() { + return false; + } + }, + DELAYED { + @Override + public Exception getException() { + return createDelayedInternal(); + } + + @Override + public ErrorCode getExpectedErrorCodeWithGax() { + return ErrorCode.DEADLINE_EXCEEDED; + } + + @Override + public ErrorCode getExpectedErrorCodeWithoutGax() { + return ErrorCode.INTERNAL; + } + + @Override + public boolean isRetryable() { + return true; + } + }; + + public abstract Exception getException(); + + public abstract ErrorCode getExpectedErrorCodeWithGax(); + + public abstract ErrorCode getExpectedErrorCodeWithoutGax(); + + public abstract boolean isRetryable(); + } + + private static MockDatabaseAdminImpl mockDatabaseAdmin; + private static Server server; + private static Spanner spanner; + private static DatabaseAdminClient client; + private static LocalChannelProvider channelProvider; + + @Parameter(0) + public boolean enableGaxRetries; + + @Parameter(1) + public int exceptionAtCall; + + @Parameter(2) + public ExceptionType exceptionType; + + @Parameters(name = "enable GAX retries = {0}, exception at call = {1}, exception type = {2}") + public static Collection data() { + List params = new ArrayList<>(); + for (boolean enableRetries : new boolean[] {true, false}) { + for (int exceptionAtCall : new int[] {0, 1}) { + for (ExceptionType exceptionType : ExceptionType.values()) { + params.add(new Object[] {enableRetries, exceptionAtCall, exceptionType}); + } + } + } + return params; + } + + @Rule public ExpectedException expectedException = ExpectedException.none(); + + @BeforeClass + public static void startStaticServer() throws IOException { + mockDatabaseAdmin = new MockDatabaseAdminImpl(); + String uniqueName = InProcessServerBuilder.generateName(); + server = + InProcessServerBuilder.forName(uniqueName) + // We need to use a real executor for timeouts to occur. + .scheduledExecutorService(new ScheduledThreadPoolExecutor(1)) + .addService(mockDatabaseAdmin) + .build() + .start(); + channelProvider = LocalChannelProvider.create(uniqueName); + } + + @AfterClass + public static void stopServer() { + server.shutdown(); + } + + @Before + public void setUp() throws Exception { + mockDatabaseAdmin.reset(); + final RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(Duration.ofMillis(50L)) + .setMaxRpcTimeout(Duration.ofMillis(50L)) + .setMaxAttempts(3) + .setTotalTimeout(Duration.ofMillis(250L)) + .build(); + SpannerOptions.Builder builder = + SpannerOptions.newBuilder() + .setProjectId(PROJECT) + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()); + builder + .databaseAdminStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetrySettings(retrySettings); + return null; + } + }); + builder + .databaseAdminStubSettingsBuilder() + .createDatabaseOperationSettings() + .setInitialCallSettings( + builder + .databaseAdminStubSettingsBuilder() + .createDatabaseOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .build()); + builder + .databaseAdminStubSettingsBuilder() + .updateDatabaseDdlOperationSettings() + .setInitialCallSettings( + builder + .databaseAdminStubSettingsBuilder() + .updateDatabaseDdlOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .build()); + if (!enableGaxRetries) { + // Disable retries by removing all retryable codes. + builder + .databaseAdminStubSettingsBuilder() + .applyToAllUnaryMethods( + new ApiFunction, Void>() { + @Override + public Void apply(Builder input) { + input.setRetryableCodes(ImmutableSet.of()); + return null; + } + }); + builder + .databaseAdminStubSettingsBuilder() + .createDatabaseOperationSettings() + .setInitialCallSettings( + builder + .databaseAdminStubSettingsBuilder() + .createDatabaseOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of()) + .build()); + builder + .databaseAdminStubSettingsBuilder() + .updateDatabaseDdlOperationSettings() + .setInitialCallSettings( + builder + .databaseAdminStubSettingsBuilder() + .updateDatabaseDdlOperationSettings() + .getInitialCallSettings() + .toBuilder() + .setRetrySettings(retrySettings) + .setRetryableCodes(ImmutableSet.of()) + .build()); + } + spanner = builder.build().getService(); + client = spanner.getDatabaseAdminClient(); + } + + @After + public void tearDown() throws Exception { + spanner.close(); + } + + private Exception setupException() { + if (exceptionType.isRetryable()) { + if (!enableGaxRetries) { + expectedException.expect( + SpannerMatchers.isSpannerException(exceptionType.getExpectedErrorCodeWithoutGax())); + } + } else { + expectedException.expect( + SpannerMatchers.isSpannerException(exceptionType.getExpectedErrorCodeWithGax())); + } + return exceptionType.getException(); + } + + @Test + public void listDatabasesTest() { + Exception exception = setupException(); + String nextPageToken = "token%d"; + List databases = new ArrayList<>(2); + for (int i = 0; i < 2; i++) { + databases.add( + com.google.spanner.admin.database.v1.Database.newBuilder() + .setName( + String.format("projects/%s/instances/%s/databases/test%d", PROJECT, INSTANCE, i)) + .build()); + } + if (exceptionAtCall == 0) { + mockDatabaseAdmin.addException(exception); + } + for (int i = 0; i < 2; i++) { + ListDatabasesResponse.Builder builder = + ListDatabasesResponse.newBuilder().addAllDatabases(Arrays.asList(databases.get(i))); + if (i < (databases.size() - 1)) { + builder.setNextPageToken(String.format(nextPageToken, i)); + } + if (exceptionAtCall == (i + 1)) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(builder.build()); + } + + InstanceName parent = InstanceName.of(PROJECT, INSTANCE); + Page pagedListResponse = client.listDatabases(INSTANCE); + + List resources = Lists.newArrayList(pagedListResponse.iterateAll()); + Assert.assertEquals(2, resources.size()); + + List actualRequests = mockDatabaseAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + ListDatabasesRequest actualRequest = (ListDatabasesRequest) actualRequests.get(0); + + Assert.assertEquals(parent, InstanceName.parse(actualRequest.getParent())); + } + + @Test + public void getDatabaseTest() { + Exception exception = setupException(); + DatabaseName name2 = DatabaseName.of(PROJECT, INSTANCE, "DATABASE"); + com.google.spanner.admin.database.v1.Database expectedResponse = + com.google.spanner.admin.database.v1.Database.newBuilder() + .setName(name2.toString()) + .build(); + if (exceptionAtCall == 0) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(expectedResponse); + if (exceptionAtCall == 1) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(expectedResponse); + + for (int i = 0; i < 2; i++) { + Database actualResponse = client.getDatabase(INSTANCE, "DATABASE"); + Assert.assertEquals(name2.toString(), actualResponse.getId().getName()); + } + + List actualRequests = mockDatabaseAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void createDatabaseTest() throws Exception { + Exception exception = setupException(); + DatabaseName name = DatabaseName.of(PROJECT, INSTANCE, "DATABASE"); + com.google.spanner.admin.database.v1.Database expectedResponse = + com.google.spanner.admin.database.v1.Database.newBuilder().setName(name.toString()).build(); + com.google.longrunning.Operation resultOperation = + com.google.longrunning.Operation.newBuilder() + .setName("createDatabaseTest") + .setDone(true) + .setResponse(Any.pack(expectedResponse)) + .build(); + if (exceptionAtCall == 0) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(resultOperation); + if (exceptionAtCall == 1) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(resultOperation); + + for (int i = 0; i < 2; i++) { + OperationFuture actualResponse = + client.createDatabase(INSTANCE, "DATABASE", Arrays.asList()); + try { + Database returnedInstance = actualResponse.get(); + Assert.assertEquals(name.toString(), returnedInstance.getId().getName()); + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + throw e; + } + } + List actualRequests = mockDatabaseAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void updateDatabaseDdlTest() throws Exception { + Exception exception = setupException(); + com.google.longrunning.Operation resultOperation = + com.google.longrunning.Operation.newBuilder() + .setName("updateDatabaseDdlTest") + .setDone(true) + .setResponse(Any.pack(Empty.getDefaultInstance())) + .build(); + if (exceptionAtCall == 0) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(resultOperation); + if (exceptionAtCall == 1) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(resultOperation); + + for (int i = 0; i < 2; i++) { + OperationFuture actualResponse = + client.updateDatabaseDdl( + INSTANCE, "DATABASE", Arrays.asList("CREATE TABLE FOO"), "updateDatabaseDdlTest"); + try { + actualResponse.get(); + } catch (ExecutionException e) { + Throwables.throwIfUnchecked(e.getCause()); + throw e; + } + } + + List actualRequests = mockDatabaseAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } + + @Test + public void deleteInstanceTest() { + Exception exception = setupException(); + Empty expectedResponse = Empty.newBuilder().build(); + if (exceptionAtCall == 0) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(expectedResponse); + if (exceptionAtCall == 1) { + mockDatabaseAdmin.addException(exception); + } + mockDatabaseAdmin.addResponse(expectedResponse); + for (int i = 0; i < 2; i++) { + client.dropDatabase(INSTANCE, "DATABASE"); + } + + List actualRequests = mockDatabaseAdmin.getRequests(); + Assert.assertEquals(2, actualRequests.size()); + } +} From eb9f4490a45ee9405be0652a3a58f33cfc9a6c79 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 15 Apr 2019 15:06:31 +0200 Subject: [PATCH 08/26] increase timeouts to reduce chance of false test failure --- .../java/com/google/cloud/spanner/DatabaseAdminGaxTest.java | 6 +++--- .../java/com/google/cloud/spanner/InstanceAdminGaxTest.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java index 49b8ab311a60..8d0911304989 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java @@ -230,10 +230,10 @@ public void setUp() throws Exception { mockDatabaseAdmin.reset(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(50L)) - .setMaxRpcTimeout(Duration.ofMillis(50L)) + .setInitialRpcTimeout(Duration.ofMillis(200L)) + .setMaxRpcTimeout(Duration.ofMillis(200L)) .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(250L)) + .setTotalTimeout(Duration.ofMillis(500L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java index f099a783d498..1f79fe2c9b90 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java @@ -233,10 +233,10 @@ public void setUp() throws Exception { mockInstanceAdmin.reset(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(50L)) - .setMaxRpcTimeout(Duration.ofMillis(50L)) + .setInitialRpcTimeout(Duration.ofMillis(200L)) + .setMaxRpcTimeout(Duration.ofMillis(200L)) .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(250L)) + .setTotalTimeout(Duration.ofMillis(500L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() From a791085735428712b253bd4387ec13b141afe168 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 15 Apr 2019 16:01:13 +0200 Subject: [PATCH 09/26] increase timeout to avoid false test failures --- .../java/com/google/cloud/spanner/SpannerGaxRetryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index d0d64198b1a9..532525fafc18 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,8 +200,8 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(200L)) - .setMaxRpcTimeout(Duration.ofMillis(200L)) + .setInitialRpcTimeout(Duration.ofMillis(300L)) + .setMaxRpcTimeout(Duration.ofMillis(500L)) .setMaxAttempts(3) .setTotalTimeout(Duration.ofMillis(500L)) .build(); From d3a01c3533b2f04a4c2ca651f38e48e20f3145a7 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 13:22:05 +0200 Subject: [PATCH 10/26] changed read/write tests to use warmed up session pool --- .../cloud/spanner/SpannerGaxRetryTest.java | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 532525fafc18..0ec024c8b2a5 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,10 +200,10 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(300L)) - .setMaxRpcTimeout(Duration.ofMillis(500L)) + .setInitialRpcTimeout(Duration.ofMillis(50L)) + .setMaxRpcTimeout(Duration.ofMillis(100L)) .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(500L)) + .setTotalTimeout(Duration.ofMillis(200L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() @@ -255,6 +255,26 @@ public void tearDown() throws Exception { spanner.close(); } + private void warmUpSessionPool() { + for(int i=0; i<10; i++) { + TransactionRunner runner = client.readWriteTransaction(); + long updateCount = + runner.run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); + assertThat(updateCount, is(equalTo(UPDATE_COUNT))); + } + // Wait a little to allow the session pool to prepare read/write sessions. + try { + Thread.sleep(500L); + } catch (InterruptedException e) { + } + } + @Test public void singleUseTimeout() { if (enableGaxRetries) { @@ -346,6 +366,7 @@ public void singleUseExecuteStreamingSqlUnavailable() { @Test public void readWriteTransactionTimeout() { + warmUpSessionPool(); if (enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); } @@ -364,6 +385,7 @@ public Long run(TransactionContext transaction) throws Exception { @Test public void readWriteTransactionUnavailable() { + warmUpSessionPool(); if (!enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); } @@ -383,6 +405,7 @@ public Long run(TransactionContext transaction) throws Exception { @SuppressWarnings("resource") @Test public void transactionManagerTimeout() { + warmUpSessionPool(); if (enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.DEADLINE_EXCEEDED)); } @@ -404,6 +427,7 @@ public void transactionManagerTimeout() { @SuppressWarnings("resource") @Test public void transactionManagerUnavailable() { + warmUpSessionPool(); if (!enableGaxRetries) { expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); } From 08e276896e126580acd6aa90033056b9530aa0fe Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 15:57:59 +0200 Subject: [PATCH 11/26] increase timeout to avoid timeouts in warmUpSessionPool --- .../java/com/google/cloud/spanner/SpannerGaxRetryTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 0ec024c8b2a5..2d180d9dbdc4 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,10 +200,10 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(50L)) - .setMaxRpcTimeout(Duration.ofMillis(100L)) + .setInitialRpcTimeout(Duration.ofMillis(100L)) + .setMaxRpcTimeout(Duration.ofMillis(200L)) .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(200L)) + .setTotalTimeout(Duration.ofMillis(500L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() From e28da860316e2e5badc79d72541a9f707303e92c Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 16:35:35 +0200 Subject: [PATCH 12/26] fixed formatting --- .../com/google/cloud/spanner/SpannerGaxRetryTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 2d180d9dbdc4..efdae5267581 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,10 +200,10 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(100L)) - .setMaxRpcTimeout(Duration.ofMillis(200L)) + .setInitialRpcTimeout(Duration.ofMillis(200L)) + .setMaxRpcTimeout(Duration.ofMillis(250L)) .setMaxAttempts(3) - .setTotalTimeout(Duration.ofMillis(500L)) + .setTotalTimeout(Duration.ofMillis(1000L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() @@ -256,7 +256,7 @@ public void tearDown() throws Exception { } private void warmUpSessionPool() { - for(int i=0; i<10; i++) { + for (int i = 0; i < 10; i++) { TransactionRunner runner = client.readWriteTransaction(); long updateCount = runner.run( From 344f86ec6d777602f4b77d10ba23dd003a16f5d9 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 17:23:24 +0200 Subject: [PATCH 13/26] set specific retry settings for commit --- .../com/google/cloud/spanner/SpannerGaxRetryTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index efdae5267581..4df8a71d97b6 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -205,6 +205,13 @@ public void setUp() throws Exception { .setMaxAttempts(3) .setTotalTimeout(Duration.ofMillis(1000L)) .build(); + RetrySettings commitRetrySettings = + RetrySettings.newBuilder() + .setInitialRpcTimeout(Duration.ofMillis(5000L)) + .setMaxRpcTimeout(Duration.ofMillis(10000L)) + .setMaxAttempts(1) + .setTotalTimeout(Duration.ofMillis(20000L)) + .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder() .setProjectId("[PROJECT]") @@ -220,6 +227,7 @@ public Void apply(Builder input) { return null; } }); + builder.spannerStubSettingsBuilder().commitSettings().setRetrySettings(commitRetrySettings); builder .spannerStubSettingsBuilder() .executeStreamingSqlSettings() From e9e6c56736b6a05f3b1f166bc5198cbf142c88d0 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 18:18:37 +0200 Subject: [PATCH 14/26] increase initial rpc timeout to avoid slow build to timeout --- .../java/com/google/cloud/spanner/SpannerGaxRetryTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 4df8a71d97b6..de55b0528248 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -200,8 +200,8 @@ public void setUp() throws Exception { mockSpanner.removeAllExecutionTimes(); final RetrySettings retrySettings = RetrySettings.newBuilder() - .setInitialRpcTimeout(Duration.ofMillis(200L)) - .setMaxRpcTimeout(Duration.ofMillis(250L)) + .setInitialRpcTimeout(Duration.ofMillis(500L)) + .setMaxRpcTimeout(Duration.ofMillis(500L)) .setMaxAttempts(3) .setTotalTimeout(Duration.ofMillis(1000L)) .build(); From b684a0d44873845b988e56a213b2b8b57edd0867 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 16 Apr 2019 20:43:36 +0200 Subject: [PATCH 15/26] added tests for default and setting custom retry settings --- .../cloud/spanner/SpannerOptionsTest.java | 271 ++++++++++++++++++ 1 file changed, 271 insertions(+) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 1cc406b8a851..fc6045810c43 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -18,8 +18,16 @@ import static com.google.common.truth.Truth.assertThat; +import com.google.api.gax.retrying.RetrySettings; +import com.google.api.gax.rpc.ServerStreamingCallSettings; +import com.google.api.gax.rpc.UnaryCallSettings; import com.google.cloud.TransportOptions; +import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; +import com.google.cloud.spanner.v1.stub.SpannerStubSettings; +import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.junit.Rule; import org.junit.Test; @@ -27,6 +35,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mockito; +import org.threeten.bp.Duration; /** Unit tests for {@link com.google.cloud.spanner.SpannerOptions}. */ @RunWith(JUnit4.class) @@ -63,6 +72,268 @@ public void builder() { assertThat(options.getSessionLabels()).containsExactlyEntriesIn(labels); } + @Test + public void testSpannerDefaultRetrySettings() { + RetrySettings defaultRetrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofSeconds(1L)) + .setRetryDelayMultiplier(1.3D) + .setMaxRetryDelay(Duration.ofSeconds(32L)) + .setInitialRpcTimeout(Duration.ofSeconds(60L)) + .setRpcTimeoutMultiplier(1.0D) + .setMaxRpcTimeout(Duration.ofSeconds(60L)) + .setTotalTimeout(Duration.ofSeconds(600L)) + .build(); + RetrySettings streamingRetrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(1000L)) + .setRetryDelayMultiplier(1.3) + .setMaxRetryDelay(Duration.ofMillis(32000L)) + .setInitialRpcTimeout(Duration.ofMillis(120000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(120000L)) + .setTotalTimeout(Duration.ofMillis(1200000L)) + .build(); + RetrySettings longRunningRetrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(1000L)) + .setRetryDelayMultiplier(1.3) + .setMaxRetryDelay(Duration.ofMillis(32000L)) + .setInitialRpcTimeout(Duration.ofMillis(3600000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(3600000L)) + .setTotalTimeout(Duration.ofMillis(3600000L)) + .build(); + SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + SpannerStubSettings stubSettings = options.getSpannerStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.beginTransactionSettings(), + stubSettings.createSessionSettings(), + stubSettings.deleteSessionSettings(), + stubSettings.executeBatchDmlSettings(), + stubSettings.executeSqlSettings(), + stubSettings.getSessionSettings(), + stubSettings.listSessionsSettings(), + stubSettings.partitionQuerySettings(), + stubSettings.partitionReadSettings(), + stubSettings.readSettings(), + stubSettings.rollbackSettings()); + List> callsWithStreamingSettings = + Arrays.asList( + stubSettings.executeStreamingSqlSettings(), stubSettings.streamingReadSettings()); + List> callsWithLongRunningSettings = + Arrays.asList(stubSettings.commitSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(defaultRetrySettings); + } + for (ServerStreamingCallSettings callSettings : callsWithStreamingSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(streamingRetrySettings); + } + for (UnaryCallSettings callSettings : callsWithLongRunningSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(longRunningRetrySettings); + } + } + + @Test + public void testSpannerCustomRetrySettings() { + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofSeconds(9999L)) + .setRetryDelayMultiplier(9999.99D) + .setMaxRetryDelay(Duration.ofSeconds(9999L)) + .setInitialRpcTimeout(Duration.ofSeconds(9999L)) + .setRpcTimeoutMultiplier(9999.99D) + .setMaxRpcTimeout(Duration.ofSeconds(9999L)) + .setTotalTimeout(Duration.ofSeconds(9999L)) + .build(); + SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); + SpannerStubSettings.Builder stubSettingsBuilder = builder.spannerStubSettingsBuilder(); + List> unaryCallSettingsBuilders = + Arrays.asList( + stubSettingsBuilder.beginTransactionSettings(), + stubSettingsBuilder.createSessionSettings(), + stubSettingsBuilder.deleteSessionSettings(), + stubSettingsBuilder.executeBatchDmlSettings(), + stubSettingsBuilder.executeSqlSettings(), + stubSettingsBuilder.getSessionSettings(), + stubSettingsBuilder.listSessionsSettings(), + stubSettingsBuilder.partitionQuerySettings(), + stubSettingsBuilder.partitionReadSettings(), + stubSettingsBuilder.readSettings(), + stubSettingsBuilder.rollbackSettings(), + stubSettingsBuilder.commitSettings()); + for (UnaryCallSettings.Builder callSettingsBuilder : unaryCallSettingsBuilders) { + callSettingsBuilder.setRetrySettings(retrySettings); + } + List> streamingCallSettingsBuilders = + Arrays.asList( + stubSettingsBuilder.executeStreamingSqlSettings(), + stubSettingsBuilder.streamingReadSettings()); + for (ServerStreamingCallSettings.Builder callSettingsBuilder : + streamingCallSettingsBuilders) { + callSettingsBuilder.setRetrySettings(retrySettings); + } + + SpannerOptions options = builder.build(); + SpannerStubSettings stubSettings = options.getSpannerStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.beginTransactionSettings(), + stubSettings.createSessionSettings(), + stubSettings.deleteSessionSettings(), + stubSettings.executeBatchDmlSettings(), + stubSettings.executeSqlSettings(), + stubSettings.getSessionSettings(), + stubSettings.listSessionsSettings(), + stubSettings.partitionQuerySettings(), + stubSettings.partitionReadSettings(), + stubSettings.readSettings(), + stubSettings.rollbackSettings(), + stubSettings.commitSettings()); + List> callsWithStreamingSettings = + Arrays.asList( + stubSettings.executeStreamingSqlSettings(), stubSettings.streamingReadSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(retrySettings); + } + for (ServerStreamingCallSettings callSettings : callsWithStreamingSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(retrySettings); + } + } + + @Test + public void testDatabaseAdminDefaultRetrySettings() { + RetrySettings defaultRetrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(1000L)) + .setRetryDelayMultiplier(1.3) + .setMaxRetryDelay(Duration.ofMillis(32000L)) + .setInitialRpcTimeout(Duration.ofMillis(60000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(60000L)) + .setTotalTimeout(Duration.ofMillis(600000L)) + .build(); + SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + DatabaseAdminStubSettings stubSettings = options.getDatabaseAdminStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.dropDatabaseSettings(), + stubSettings.getDatabaseDdlSettings(), + stubSettings.getDatabaseSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(defaultRetrySettings); + } + } + + @Test + public void testDatabaseAdminCustomRetrySettings() { + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofSeconds(9999L)) + .setRetryDelayMultiplier(9999.99D) + .setMaxRetryDelay(Duration.ofSeconds(9999L)) + .setInitialRpcTimeout(Duration.ofSeconds(9999L)) + .setRpcTimeoutMultiplier(9999.99D) + .setMaxRpcTimeout(Duration.ofSeconds(9999L)) + .setTotalTimeout(Duration.ofSeconds(9999L)) + .build(); + SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); + DatabaseAdminStubSettings.Builder stubSettingsBuilder = + builder.databaseAdminStubSettingsBuilder(); + List> unaryCallSettingsBuilders = + Arrays.asList( + stubSettingsBuilder.dropDatabaseSettings(), + stubSettingsBuilder.getDatabaseDdlSettings(), + stubSettingsBuilder.getDatabaseSettings()); + for (UnaryCallSettings.Builder callSettingsBuilder : unaryCallSettingsBuilders) { + callSettingsBuilder.setRetrySettings(retrySettings); + } + + SpannerOptions options = builder.build(); + DatabaseAdminStubSettings stubSettings = options.getDatabaseAdminStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.dropDatabaseSettings(), + stubSettings.getDatabaseDdlSettings(), + stubSettings.getDatabaseSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(retrySettings); + } + } + + @Test + public void testInstanceAdminDefaultRetrySettings() { + RetrySettings defaultRetrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofMillis(1000L)) + .setRetryDelayMultiplier(1.3) + .setMaxRetryDelay(Duration.ofMillis(32000L)) + .setInitialRpcTimeout(Duration.ofMillis(60000L)) + .setRpcTimeoutMultiplier(1.0) + .setMaxRpcTimeout(Duration.ofMillis(60000L)) + .setTotalTimeout(Duration.ofMillis(600000L)) + .build(); + SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build(); + InstanceAdminStubSettings stubSettings = options.getInstanceAdminStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.getInstanceConfigSettings(), + stubSettings.listInstanceConfigsSettings(), + stubSettings.deleteInstanceSettings(), + stubSettings.getInstanceSettings(), + stubSettings.listInstancesSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(defaultRetrySettings); + } + } + + @Test + public void testInstanceAdminCustomRetrySettings() { + RetrySettings retrySettings = + RetrySettings.newBuilder() + .setInitialRetryDelay(Duration.ofSeconds(9999L)) + .setRetryDelayMultiplier(9999.99D) + .setMaxRetryDelay(Duration.ofSeconds(9999L)) + .setInitialRpcTimeout(Duration.ofSeconds(9999L)) + .setRpcTimeoutMultiplier(9999.99D) + .setMaxRpcTimeout(Duration.ofSeconds(9999L)) + .setTotalTimeout(Duration.ofSeconds(9999L)) + .build(); + SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); + InstanceAdminStubSettings.Builder stubSettingsBuilder = + builder.instanceAdminStubSettingsBuilder(); + List> unaryCallSettingsBuilders = + Arrays.asList( + stubSettingsBuilder.deleteInstanceSettings(), + stubSettingsBuilder.getInstanceConfigSettings(), + stubSettingsBuilder.getInstanceSettings(), + stubSettingsBuilder.listInstanceConfigsSettings(), + stubSettingsBuilder.listInstancesSettings()); + for (UnaryCallSettings.Builder callSettingsBuilder : unaryCallSettingsBuilders) { + callSettingsBuilder.setRetrySettings(retrySettings); + } + + SpannerOptions options = builder.build(); + InstanceAdminStubSettings stubSettings = options.getInstanceAdminStubSettings(); + List> callsWithDefaultSettings = + Arrays.asList( + stubSettings.getInstanceConfigSettings(), + stubSettings.listInstanceConfigsSettings(), + stubSettings.deleteInstanceSettings(), + stubSettings.getInstanceSettings(), + stubSettings.listInstancesSettings()); + + for (UnaryCallSettings callSettings : callsWithDefaultSettings) { + assertThat(callSettings.getRetrySettings()).isEqualTo(retrySettings); + } + } + @Test public void testInvalidTransport() { thrown.expect(IllegalArgumentException.class); From e945c64875d2a1c29c221f1f5d997ce5b667183d Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 17 Apr 2019 07:43:26 +0200 Subject: [PATCH 16/26] add retry loop in warmUpSessionPool --- .../cloud/spanner/SpannerGaxRetryTest.java | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index de55b0528248..b96448b98813 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -265,16 +265,27 @@ public void tearDown() throws Exception { private void warmUpSessionPool() { for (int i = 0; i < 10; i++) { - TransactionRunner runner = client.readWriteTransaction(); - long updateCount = - runner.run( - new TransactionCallable() { - @Override - public Long run(TransactionContext transaction) throws Exception { - return transaction.executeUpdate(UPDATE_STATEMENT); - } - }); - assertThat(updateCount, is(equalTo(UPDATE_COUNT))); + while (true) { + try { + TransactionRunner runner = client.readWriteTransaction(); + long updateCount = + runner.run( + new TransactionCallable() { + @Override + public Long run(TransactionContext transaction) throws Exception { + return transaction.executeUpdate(UPDATE_STATEMENT); + } + }); + assertThat(updateCount, is(equalTo(UPDATE_COUNT))); + break; + } catch (SpannerException e) { + // On slow systems there is a chance of DEADLINE_EXCEEDED errors. + // These should be retried. + if (e.getErrorCode() != ErrorCode.DEADLINE_EXCEEDED) { + throw e; + } + } + } } // Wait a little to allow the session pool to prepare read/write sessions. try { From d30497826c0c0c41a80ddc4aa31e89739ef8de71 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 17 Apr 2019 08:27:14 +0200 Subject: [PATCH 17/26] added additional documentation --- .../google/cloud/spanner/SpannerOptions.java | 61 +++++++++++++++++-- .../cloud/spanner/SpannerGaxRetryTest.java | 7 ++- 2 files changed, 62 insertions(+), 6 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index a59d24ff4f3e..ef5901d79f24 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -233,20 +233,43 @@ public Builder setSessionLabels(Map sessionLabels) { return this; } + /** + * {@link SpannerOptions.Builder} does not support global retry settings, as it creates three + * different gRPC clients: {@link Spanner}, {@link DatabaseAdminClient} and {@link + * InstanceAdminClient}. Instead of calling this method, you should set specific {@link + * RetrySettings} for each of the underlying gRPC clients by calling respectively {@link + * #spannerStubSettingsBuilder()}, {@link #databaseAdminStubSettingsBuilder()} or {@link + * #instanceAdminStubSettingsBuilder()}. + */ @Override public Builder setRetrySettings(RetrySettings retrySettings) { throw new UnsupportedOperationException( "SpannerOptions does not support setting global retry settings. " - + "Call stubSettingsBuilder().Settings().setRetrySettings(RetrySettings) instead."); + + "Call spannerStubSettingsBuilder().Settings().setRetrySettings(RetrySettings) instead."); } /** * Returns the {@link SpannerStubSettings.Builder} that will be used to build the {@link - * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * *

The library will automatically use sensible defaults if no custom settings are set. The * defaults are the same as the defaults that are used by {@link SpannerSettings}. Retries are * configured for idempotent methods but not for non-idempotent methods. + * + *

You can set the same {@link RetrySettings} for all unary methods by calling this: + * + * 

{@code
+     * builder
+     *     .spannerStubSettingsBuilder()
+     *     .applyToAllUnaryMethods(
+     *         new ApiFunction, Void>() {
+     *           @Override
+     *           public Void apply(Builder input) {
+     *             input.setRetrySettings(retrySettings);
+     *             return null;
+     *           }
+     *         });
+     * }
*/ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { return spannerStubSettingsBuilder; @@ -254,11 +277,26 @@ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { /** * Returns the {@link InstanceAdminStubSettings.Builder} that will be used to build the {@link - * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * *

The library will automatically use sensible defaults if no custom settings are set. The * defaults are the same as the defaults that are used by {@link InstanceAdminSettings}. Retries * are configured for idempotent methods but not for non-idempotent methods. + * + *

You can set the same {@link RetrySettings} for all unary methods by calling this: + * + * 

{@code
+     * builder
+     *     .instanceAdminStubSettingsBuilder()
+     *     .applyToAllUnaryMethods(
+     *         new ApiFunction, Void>() {
+     *           @Override
+     *           public Void apply(Builder input) {
+     *             input.setRetrySettings(retrySettings);
+     *             return null;
+     *           }
+     *         });
+     * }
*/ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { return instanceAdminStubSettingsBuilder; @@ -266,11 +304,26 @@ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { /** * Returns the {@link DatabaseAdminStubSettings.Builder} that will be used to build the {@link - * SpannerRpc}. Use this to set custom {@link RetrySettings} for gRPC calls. + * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * *

The library will automatically use sensible defaults if no custom settings are set. The * defaults are the same as the defaults that are used by {@link DatabaseAdminSettings}. Retries * are configured for idempotent methods but not for non-idempotent methods. + * + *

You can set the same {@link RetrySettings} for all unary methods by calling this: + * + * 

{@code
+     * builder
+     *     .databaseAdminStubSettingsBuilder()
+     *     .applyToAllUnaryMethods(
+     *         new ApiFunction, Void>() {
+     *           @Override
+     *           public Void apply(Builder input) {
+     *             input.setRetrySettings(retrySettings);
+     *             return null;
+     *           }
+     *         });
+     * }
*/ public DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder() { return databaseAdminStubSettingsBuilder; diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index b96448b98813..6c708ed0d814 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -265,6 +265,7 @@ public void tearDown() throws Exception { private void warmUpSessionPool() { for (int i = 0; i < 10; i++) { + int retryCount = 0; while (true) { try { TransactionRunner runner = client.readWriteTransaction(); @@ -281,7 +282,8 @@ public Long run(TransactionContext transaction) throws Exception { } catch (SpannerException e) { // On slow systems there is a chance of DEADLINE_EXCEEDED errors. // These should be retried. - if (e.getErrorCode() != ErrorCode.DEADLINE_EXCEEDED) { + retryCount++; + if (e.getErrorCode() != ErrorCode.DEADLINE_EXCEEDED || retryCount > 10) { throw e; } } @@ -376,7 +378,8 @@ public void singleUseExecuteStreamingSqlTimeout() { @Test public void singleUseExecuteStreamingSqlUnavailable() { - // executeStreamingSql is always retried. + // executeStreamingSql is always retried by the Spanner library, even if gax retries have been + // disabled. try (ResultSet rs = client.singleUse().executeQuery(SELECT1AND2)) { mockSpanner.addException(UNAVAILABLE); while (rs.next()) {} From f78df4dee1b57a823f6a7f0bde081a0ef6a1ed96 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 17 Apr 2019 11:04:11 +0200 Subject: [PATCH 18/26] removed unnecessary passing around SpannerImpl --- .../cloud/spanner/AbstractReadContext.java | 35 +++++++++---------- .../google/cloud/spanner/BatchClientImpl.java | 8 ++--- .../spanner/DatabaseAdminClientImpl.java | 20 +++++------ .../spanner/InstanceAdminClientImpl.java | 23 ++++++------ .../spanner/PartitionedDMLTransaction.java | 11 +++--- .../com/google/cloud/spanner/SessionImpl.java | 12 ++++--- .../com/google/cloud/spanner/SpannerImpl.java | 5 +-- .../cloud/spanner/TransactionRunnerImpl.java | 25 ++++++------- .../cloud/spanner/spi/v1/GapicSpannerRpc.java | 1 - .../spanner/DatabaseAdminClientImplTest.java | 9 +---- .../spanner/InstanceAdminClientImplTest.java | 9 +---- 11 files changed, 70 insertions(+), 88 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java index 7340257f356c..cb2f217c496f 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractReadContext.java @@ -64,8 +64,8 @@ static class SingleReadContext extends AbstractReadContext { private boolean used; SingleReadContext( - SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { - super(session, spanner, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { + super(session, rpc, defaultPrefetchChunks); this.bound = bound; } @@ -100,8 +100,8 @@ static class SingleUseReadOnlyTransaction extends SingleReadContext private Timestamp timestamp; SingleUseReadOnlyTransaction( - SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { - super(session, bound, spanner, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { + super(session, bound, rpc, defaultPrefetchChunks); } @Override @@ -149,8 +149,8 @@ static class MultiUseReadOnlyTransaction extends AbstractReadContext private ByteString transactionId; MultiUseReadOnlyTransaction( - SessionImpl session, TimestampBound bound, SpannerImpl spanner, int defaultPrefetchChunks) { - super(session, spanner, defaultPrefetchChunks); + SessionImpl session, TimestampBound bound, SpannerRpc rpc, int defaultPrefetchChunks) { + super(session, rpc, defaultPrefetchChunks); checkArgument( bound.getMode() != TimestampBound.Mode.MAX_STALENESS && bound.getMode() != TimestampBound.Mode.MIN_READ_TIMESTAMP, @@ -164,9 +164,9 @@ static class MultiUseReadOnlyTransaction extends AbstractReadContext SessionImpl session, ByteString transactionId, Timestamp timestamp, - SpannerImpl spanner, + SpannerRpc rpc, int defaultPrefetchChunks) { - super(session, spanner, defaultPrefetchChunks); + super(session, rpc, defaultPrefetchChunks); this.transactionId = transactionId; this.timestamp = timestamp; } @@ -225,8 +225,7 @@ void initTransaction() { .setSession(session.getName()) .setOptions(options) .build(); - Transaction transaction = - spanner.getRpc().beginTransaction(request, session.getOptions()); + Transaction transaction = rpc.beginTransaction(request, session.getOptions()); if (!transaction.hasReadTimestamp()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, "Missing expected transaction.read_timestamp metadata field"); @@ -254,7 +253,7 @@ void initTransaction() { final Object lock = new Object(); final SessionImpl session; - final SpannerImpl spanner; + final SpannerRpc rpc; final Span span; private final int defaultPrefetchChunks; @@ -272,14 +271,14 @@ void initTransaction() { // much more frequently. private static final int MAX_BUFFERED_CHUNKS = 512; - AbstractReadContext(SessionImpl session, SpannerImpl spanner, int defaultPrefetchChunks) { - this(session, spanner, defaultPrefetchChunks, Tracing.getTracer().getCurrentSpan()); + AbstractReadContext(SessionImpl session, SpannerRpc rpc, int defaultPrefetchChunks) { + this(session, rpc, defaultPrefetchChunks, Tracing.getTracer().getCurrentSpan()); } private AbstractReadContext( - SessionImpl session, SpannerImpl spanner, int defaultPrefetchChunks, Span span) { + SessionImpl session, SpannerRpc rpc, int defaultPrefetchChunks, Span span) { this.session = session; - this.spanner = spanner; + this.rpc = rpc; this.defaultPrefetchChunks = defaultPrefetchChunks; this.span = span; } @@ -419,9 +418,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken request.setResumeToken(resumeToken); } SpannerRpc.StreamingCall call = - spanner - .getRpc() - .executeQuery(request.build(), stream.consumer(), session.getOptions()); + rpc.executeQuery(request.build(), stream.consumer(), session.getOptions()); call.request(prefetchChunks); stream.setCall(call); return stream; @@ -528,7 +525,7 @@ CloseableIterator startStream(@Nullable ByteString resumeToken builder.setResumeToken(resumeToken); } SpannerRpc.StreamingCall call = - spanner.getRpc().read(builder.build(), stream.consumer(), session.getOptions()); + rpc.read(builder.build(), stream.consumer(), session.getOptions()); call.request(prefetchChunks); stream.setCall(call); return stream; diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java index fb1757b3bc3c..7bcc60653ffc 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/BatchClientImpl.java @@ -64,7 +64,7 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa super( checkNotNull(session), checkNotNull(bound), - checkNotNull(spanner), + checkNotNull(spanner).getOptions().getSpannerRpcV1(), spanner.getOptions().getPrefetchChunks()); this.sessionName = session.getName(); this.options = session.getOptions(); @@ -77,7 +77,7 @@ private static class BatchReadOnlyTransactionImpl extends MultiUseReadOnlyTransa checkNotNull(session), checkNotNull(batchTransactionId).getTransactionId(), batchTransactionId.getTimestamp(), - checkNotNull(spanner), + checkNotNull(spanner).getOptions().getSpannerRpcV1(), spanner.getOptions().getPrefetchChunks()); this.sessionName = session.getName(); this.options = session.getOptions(); @@ -134,7 +134,7 @@ public List partitionReadUsingIndex( builder.setPartitionOptions(pbuilder.build()); final PartitionReadRequest request = builder.build(); - PartitionResponse response = spanner.getRpc().partitionRead(request, options); + PartitionResponse response = rpc.partitionRead(request, options); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = @@ -172,7 +172,7 @@ public List partitionQuery( builder.setPartitionOptions(pbuilder.build()); final PartitionQueryRequest request = builder.build(); - PartitionResponse response = spanner.getRpc().partitionQuery(request, options); + PartitionResponse response = rpc.partitionQuery(request, options); ImmutableList.Builder partitions = ImmutableList.builder(); for (com.google.spanner.v1.Partition p : response.getPartitionsList()) { Partition partition = diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java index 80bf08aa4e6d..cd5589c6c4d9 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/DatabaseAdminClientImpl.java @@ -24,6 +24,7 @@ import com.google.api.gax.paging.Page; import com.google.cloud.spanner.Options.ListOption; import com.google.cloud.spanner.SpannerImpl.PageFetcher; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.base.Preconditions; import com.google.protobuf.Empty; @@ -36,11 +37,11 @@ /** Default implementation of {@link DatabaseAdminClient}. */ class DatabaseAdminClientImpl implements DatabaseAdminClient { private final String projectId; - private final SpannerImpl spanner; + private final SpannerRpc rpc; - DatabaseAdminClientImpl(String projectId, SpannerImpl spanner) { + DatabaseAdminClientImpl(String projectId, SpannerRpc rpc) { this.projectId = projectId; - this.spanner = spanner; + this.rpc = rpc; } /** Generates a random operation id for long-running database operations. */ @@ -56,8 +57,7 @@ public OperationFuture createDatabase( String instanceName = getInstanceName(instanceId); String createStatement = "CREATE DATABASE `" + databaseId + "`"; OperationFuture - rawOperationFuture = - spanner.getRpc().createDatabase(instanceName, createStatement, statements); + rawOperationFuture = rpc.createDatabase(instanceName, createStatement, statements); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), @@ -83,7 +83,7 @@ public Database apply(Exception e) { @Override public Database getDatabase(String instanceId, String databaseId) throws SpannerException { String dbName = getDatabaseName(instanceId, databaseId); - return Database.fromProto(spanner.getRpc().getDatabase(dbName), DatabaseAdminClientImpl.this); + return Database.fromProto(rpc.getDatabase(dbName), DatabaseAdminClientImpl.this); } @Override @@ -96,7 +96,7 @@ public OperationFuture updateDatabaseDdl( final String dbName = getDatabaseName(instanceId, databaseId); final String opId = operationId != null ? operationId : randomOperationId(); OperationFuture rawOperationFuture = - spanner.getRpc().updateDatabaseDdl(dbName, statements, opId); + rpc.updateDatabaseDdl(dbName, statements, opId); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), @@ -119,13 +119,13 @@ public Void apply(Exception e) { @Override public void dropDatabase(String instanceId, String databaseId) throws SpannerException { String dbName = getDatabaseName(instanceId, databaseId); - spanner.getRpc().dropDatabase(dbName); + rpc.dropDatabase(dbName); } @Override public List getDatabaseDdl(String instanceId, String databaseId) { String dbName = getDatabaseName(instanceId, databaseId); - return spanner.getRpc().getDatabaseDdl(dbName); + return rpc.getDatabaseDdl(dbName); } @Override @@ -140,7 +140,7 @@ public Page listDatabases(String instanceId, ListOption... options) { @Override public Paginated getNextPage( String nextPageToken) { - return spanner.getRpc().listDatabases(instanceName, pageSize, nextPageToken); + return rpc.listDatabases(instanceName, pageSize, nextPageToken); } @Override diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java index dd31e165baaf..7a8406db9cee 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/InstanceAdminClientImpl.java @@ -25,6 +25,7 @@ import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.spanner.Options.ListOption; import com.google.cloud.spanner.SpannerImpl.PageFetcher; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.base.Preconditions; import com.google.protobuf.FieldMask; @@ -37,11 +38,11 @@ class InstanceAdminClientImpl implements InstanceAdminClient { PathTemplate.create("projects/{project}"); private final DatabaseAdminClient dbClient; private final String projectId; - private final SpannerImpl spanner; + private final SpannerRpc rpc; - InstanceAdminClientImpl(String projectId, SpannerImpl spanner, DatabaseAdminClient dbClient) { + InstanceAdminClientImpl(String projectId, SpannerRpc rpc, DatabaseAdminClient dbClient) { this.projectId = projectId; - this.spanner = spanner; + this.rpc = rpc; this.dbClient = dbClient; } @@ -49,7 +50,7 @@ class InstanceAdminClientImpl implements InstanceAdminClient { public InstanceConfig getInstanceConfig(String configId) throws SpannerException { String instanceConfigName = new InstanceConfigId(projectId, configId).getName(); return InstanceConfig.fromProto( - spanner.getRpc().getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); + rpc.getInstanceConfig(instanceConfigName), InstanceAdminClientImpl.this); } @Override @@ -63,7 +64,7 @@ public Page listInstanceConfigs(ListOption... options) { @Override public Paginated getNextPage( String nextPageToken) { - return spanner.getRpc().listInstanceConfigs(pageSize, nextPageToken); + return rpc.listInstanceConfigs(pageSize, nextPageToken); } @Override @@ -84,9 +85,7 @@ public OperationFuture createInstance(Instance String projectName = PROJECT_NAME_TEMPLATE.instantiate("project", projectId); OperationFuture rawOperationFuture = - spanner - .getRpc() - .createInstance(projectName, instance.getId().getInstance(), instance.toProto()); + rpc.createInstance(projectName, instance.getId().getInstance(), instance.toProto()); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), @@ -115,7 +114,7 @@ public Instance apply(Exception e) { public Instance getInstance(String instanceId) throws SpannerException { String instanceName = new InstanceId(projectId, instanceId).getName(); return Instance.fromProto( - spanner.getRpc().getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); + rpc.getInstance(instanceName), InstanceAdminClientImpl.this, dbClient); } @Override @@ -128,7 +127,7 @@ public Page listInstances(ListOption... options) throws SpannerExcepti @Override public Paginated getNextPage( String nextPageToken) { - return spanner.getRpc().listInstances(pageSize, nextPageToken, filter); + return rpc.listInstances(pageSize, nextPageToken, filter); } @Override @@ -144,7 +143,7 @@ public Instance fromProto(com.google.spanner.admin.instance.v1.Instance proto) { @Override public void deleteInstance(final String instanceId) throws SpannerException { - spanner.getRpc().deleteInstance(new InstanceId(projectId, instanceId).getName()); + rpc.deleteInstance(new InstanceId(projectId, instanceId).getName()); } @Override @@ -156,7 +155,7 @@ public OperationFuture updateInstance( : InstanceInfo.InstanceField.toFieldMask(fieldsToUpdate); OperationFuture - rawOperationFuture = spanner.getRpc().updateInstance(instance.toProto(), fieldMask); + rawOperationFuture = rpc.updateInstance(instance.toProto(), fieldMask); return new OperationFutureImpl( rawOperationFuture.getPollingFuture(), rawOperationFuture.getInitialFuture(), diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java index 0ded8849830c..fa8344d003b1 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/PartitionedDMLTransaction.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkState; import com.google.cloud.spanner.SessionImpl.SessionTransaction; +import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.protobuf.ByteString; import com.google.spanner.v1.BeginTransactionRequest; import com.google.spanner.v1.ExecuteSqlRequest; @@ -32,12 +33,12 @@ class PartitionedDMLTransaction implements SessionTransaction { private final ByteString transactionId; private final SessionImpl session; - private final SpannerImpl spanner; + private final SpannerRpc rpc; private volatile boolean isValid = true; - PartitionedDMLTransaction(SessionImpl session, SpannerImpl spanner) { + PartitionedDMLTransaction(SessionImpl session, SpannerRpc rpc) { this.session = session; - this.spanner = spanner; + this.rpc = rpc; this.transactionId = initTransaction(); } @@ -49,7 +50,7 @@ private ByteString initTransaction() { TransactionOptions.newBuilder() .setPartitionedDml(TransactionOptions.PartitionedDml.getDefaultInstance())) .build(); - Transaction txn = spanner.getRpc().beginTransaction(request, session.getOptions()); + Transaction txn = rpc.beginTransaction(request, session.getOptions()); if (txn.getId().isEmpty()) { throw SpannerExceptionFactory.newSpannerException( ErrorCode.INTERNAL, @@ -75,7 +76,7 @@ long executePartitionedUpdate(Statement statement) { } } com.google.spanner.v1.ResultSet resultSet = - spanner.getRpc().executeQuery(builder.build(), session.getOptions()); + rpc.executeQuery(builder.build(), session.getOptions()); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "Partitioned DML response missing stats possibly due to non-DML statement as input"); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java index 443146767e98..ef514fba1a9c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionImpl.java @@ -99,7 +99,7 @@ public String getName() { @Override public long executePartitionedUpdate(Statement stmt) { setActive(null); - PartitionedDMLTransaction txn = new PartitionedDMLTransaction(this, spanner); + PartitionedDMLTransaction txn = new PartitionedDMLTransaction(this, spanner.getRpc()); return txn.executePartitionedUpdate(stmt); } @@ -157,7 +157,7 @@ public ReadContext singleUse() { @Override public ReadContext singleUse(TimestampBound bound) { return setActive( - new SingleReadContext(this, bound, spanner, spanner.getDefaultPrefetchChunks())); + new SingleReadContext(this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); } @Override @@ -168,7 +168,8 @@ public ReadOnlyTransaction singleUseReadOnlyTransaction() { @Override public ReadOnlyTransaction singleUseReadOnlyTransaction(TimestampBound bound) { return setActive( - new SingleUseReadOnlyTransaction(this, bound, spanner, spanner.getDefaultPrefetchChunks())); + new SingleUseReadOnlyTransaction( + this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); } @Override @@ -179,7 +180,8 @@ public ReadOnlyTransaction readOnlyTransaction() { @Override public ReadOnlyTransaction readOnlyTransaction(TimestampBound bound) { return setActive( - new MultiUseReadOnlyTransaction(this, bound, spanner, spanner.getDefaultPrefetchChunks())); + new MultiUseReadOnlyTransaction( + this, bound, spanner.getRpc(), spanner.getDefaultPrefetchChunks())); } @Override @@ -231,7 +233,7 @@ ByteString beginTransaction() { TransactionContextImpl newTransaction() { TransactionContextImpl txn = new TransactionContextImpl( - this, readyTransactionId, spanner, spanner.getDefaultPrefetchChunks()); + this, readyTransactionId, spanner.getRpc(), spanner.getDefaultPrefetchChunks()); return txn; } diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 5e239b8a5d6d..4dbe82b7ca96 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -93,8 +93,9 @@ class SpannerImpl extends BaseService implements Spanner { SpannerImpl(SpannerRpc gapicRpc, SpannerOptions options) { super(options); this.gapicRpc = gapicRpc; - this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), this); - this.instanceClient = new InstanceAdminClientImpl(options.getProjectId(), this, dbAdminClient); + this.dbAdminClient = new DatabaseAdminClientImpl(options.getProjectId(), gapicRpc); + this.instanceClient = + new InstanceAdminClientImpl(options.getProjectId(), gapicRpc, dbAdminClient); } SpannerImpl(SpannerOptions options) { diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java index b9c5b8e02299..0c85a1197d57 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/TransactionRunnerImpl.java @@ -72,9 +72,9 @@ static class TransactionContextImpl extends AbstractReadContext implements Trans TransactionContextImpl( SessionImpl session, @Nullable ByteString transactionId, - SpannerImpl spanner, + SpannerRpc rpc, int defaultPrefetchChunks) { - super(session, spanner, defaultPrefetchChunks); + super(session, rpc, defaultPrefetchChunks); this.transactionId = transactionId; } @@ -123,8 +123,7 @@ void commit() { final CommitRequest commitRequest = builder.build(); Span opSpan = tracer.spanBuilderWithExplicitParent(SpannerImpl.COMMIT, span).startSpan(); try (Scope s = tracer.withSpan(opSpan)) { - CommitResponse commitResponse = - spanner.getRpc().commit(commitRequest, session.getOptions()); + CommitResponse commitResponse = rpc.commit(commitRequest, session.getOptions()); if (!commitResponse.hasCommitTimestamp()) { throw newSpannerException( ErrorCode.INTERNAL, "Missing commitTimestamp:\n" + session.getName()); @@ -170,14 +169,12 @@ void rollback() { // response. Normally, the next thing that will happen is that we will make a fresh // transaction attempt, which should implicitly abort this one. span.addAnnotation("Starting Rollback"); - spanner - .getRpc() - .rollback( - RollbackRequest.newBuilder() - .setSession(session.getName()) - .setTransactionId(transactionId) - .build(), - session.getOptions()); + rpc.rollback( + RollbackRequest.newBuilder() + .setSession(session.getName()) + .setTransactionId(transactionId) + .build(), + session.getOptions()); span.addAnnotation("Rollback Done"); } catch (SpannerException e) { txnLogger.log(Level.FINE, "Exception during rollback", e); @@ -233,7 +230,7 @@ public long executeUpdate(Statement statement) { final ExecuteSqlRequest.Builder builder = getExecuteSqlRequestBuilder(statement, QueryMode.NORMAL); com.google.spanner.v1.ResultSet resultSet = - spanner.getRpc().executeQuery(builder.build(), session.getOptions()); + rpc.executeQuery(builder.build(), session.getOptions()); if (!resultSet.hasStats()) { throw new IllegalArgumentException( "DML response missing stats possibly due to non-DML statement as input"); @@ -247,7 +244,7 @@ public long[] batchUpdate(Iterable statements) { beforeReadOrQuery(); final ExecuteBatchDmlRequest.Builder builder = getExecuteBatchDmlRequestBuilder(statements); com.google.spanner.v1.ExecuteBatchDmlResponse response = - spanner.getRpc().executeBatchDml(builder.build(), session.getOptions()); + rpc.executeBatchDml(builder.build(), session.getOptions()); long[] results = new long[response.getResultSetsCount()]; for (int i = 0; i < response.getResultSetsCount(); ++i) { results[i] = response.getResultSets(i).getStats().getRowCountExact(); diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index a6ca0e003ac5..950922e33e0d 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -598,7 +598,6 @@ private GrpcCallContext newCallContext(@Nullable Map options, String return context.withStreamWaitTimeout(waitTimeout).withStreamIdleTimeout(idleTimeout); } - @Override public void shutdown() { this.spannerStub.close(); this.instanceAdminStub.close(); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java index 15c586327ce5..3315faa8d03c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminClientImplTest.java @@ -21,9 +21,7 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; -import com.google.api.gax.retrying.RetrySettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.collect.ImmutableList; @@ -41,7 +39,6 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mock; -import org.mockito.Mockito; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.DatabaseAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -60,11 +57,7 @@ public class DatabaseAdminClientImplTest { @Before public void setUp() { initMocks(this); - SpannerOptions spannerOptions = Mockito.mock(SpannerOptions.class); - when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); - when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); - SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); - client = new DatabaseAdminClientImpl(PROJECT_ID, spanner); + client = new DatabaseAdminClientImpl(PROJECT_ID, rpc); } private Database getDatabaseProto() { diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java index 46338e2c5be6..0170ada64bd9 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminClientImplTest.java @@ -21,9 +21,7 @@ import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; -import com.google.api.core.NanoClock; import com.google.api.gax.longrunning.OperationFuture; -import com.google.api.gax.retrying.RetrySettings; import com.google.cloud.spanner.spi.v1.SpannerRpc; import com.google.cloud.spanner.spi.v1.SpannerRpc.Paginated; import com.google.common.collect.ImmutableList; @@ -38,7 +36,6 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mock; -import org.mockito.Mockito; /** Unit tests for {@link com.google.cloud.spanner.SpannerImpl.InstanceAdminClientImpl}. */ @RunWith(JUnit4.class) @@ -58,11 +55,7 @@ public class InstanceAdminClientImplTest { @Before public void setUp() { initMocks(this); - SpannerOptions spannerOptions = Mockito.mock(SpannerOptions.class); - when(spannerOptions.getRetrySettings()).thenReturn(RetrySettings.newBuilder().build()); - when(spannerOptions.getClock()).thenReturn(NanoClock.getDefaultClock()); - SpannerImpl spanner = new SpannerImpl(rpc, spannerOptions); - client = new InstanceAdminClientImpl(PROJECT_ID, spanner, dbClient); + client = new InstanceAdminClientImpl(PROJECT_ID, rpc, dbClient); } @Test From ae6b76d661230916925cb88dcc2fba3a4b3ee8ee Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 17 Apr 2019 11:28:15 +0200 Subject: [PATCH 19/26] fixed javadoc build errors --- .../google/cloud/spanner/SpannerOptions.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index ef5901d79f24..e9e4ac500c9d 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -32,6 +32,7 @@ import com.google.cloud.grpc.GrpcTransportOptions; import com.google.cloud.spanner.admin.database.v1.DatabaseAdminSettings; import com.google.cloud.spanner.admin.database.v1.stub.DatabaseAdminStubSettings; +import com.google.cloud.spanner.admin.instance.v1.InstanceAdminSettings; import com.google.cloud.spanner.admin.instance.v1.stub.InstanceAdminStubSettings; import com.google.cloud.spanner.spi.SpannerRpcFactory; import com.google.cloud.spanner.spi.v1.GapicSpannerRpc; @@ -258,18 +259,17 @@ public Builder setRetrySettings(RetrySettings retrySettings) { * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * - * 

{@code
+     * 

      * builder
      *     .spannerStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
-     *         new ApiFunction, Void>() {
-     *           @Override
-     *           public Void apply(Builder input) {
+     *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
+     *           public Void apply(Builder<?, ?> input) {
      *             input.setRetrySettings(retrySettings);
      *             return null;
      *           }
      *         });
-     * }

+     *
*/ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { return spannerStubSettingsBuilder; @@ -285,18 +285,17 @@ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * - * 

{@code
+     * 

      * builder
      *     .instanceAdminStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
-     *         new ApiFunction, Void>() {
-     *           @Override
-     *           public Void apply(Builder input) {
+     *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
+     *           public Void apply(Builder<?, ?> input) {
      *             input.setRetrySettings(retrySettings);
      *             return null;
      *           }
      *         });
-     * }

+     *
*/ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { return instanceAdminStubSettingsBuilder; @@ -312,18 +311,17 @@ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * - * 

{@code
+     * 

      * builder
      *     .databaseAdminStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
-     *         new ApiFunction, Void>() {
-     *           @Override
-     *           public Void apply(Builder input) {
+     *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
+     *           public Void apply(Builder<?, ?> input) {
      *             input.setRetrySettings(retrySettings);
      *             return null;
      *           }
      *         });
-     * }

+     *
*/ public DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder() { return databaseAdminStubSettingsBuilder; From 02dca04a144fb2c646a73e9e6324f5da53d79b07 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Wed, 17 Apr 2019 12:10:20 +0200 Subject: [PATCH 20/26] removed unnecessary import change --- .../java/com/google/cloud/spanner/BatchClientImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java index 74a2214d68c1..8dfe02d70bb7 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BatchClientImplTest.java @@ -17,8 +17,8 @@ package com.google.cloud.spanner; import static com.google.common.truth.Truth.assertThat; -import static org.mockito.Matchers.anyMap; -import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.anyMap; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; From 432c1328b3c8539cd2aa61ba81500a1930b26238 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 22 Apr 2019 18:41:17 +0200 Subject: [PATCH 21/26] renamed stub settings builder methods to get...SettingsBuilder --- .../google/cloud/spanner/SpannerOptions.java | 16 +++++++-------- .../cloud/spanner/DatabaseAdminGaxTest.java | 20 +++++++++---------- .../cloud/spanner/InstanceAdminGaxTest.java | 20 +++++++++---------- .../cloud/spanner/SpannerGaxRetryTest.java | 14 ++++++------- .../cloud/spanner/SpannerOptionsTest.java | 6 +++--- 5 files changed, 38 insertions(+), 38 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index e9e4ac500c9d..a23a187e7ba0 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -239,8 +239,8 @@ public Builder setSessionLabels(Map sessionLabels) { * different gRPC clients: {@link Spanner}, {@link DatabaseAdminClient} and {@link * InstanceAdminClient}. Instead of calling this method, you should set specific {@link * RetrySettings} for each of the underlying gRPC clients by calling respectively {@link - * #spannerStubSettingsBuilder()}, {@link #databaseAdminStubSettingsBuilder()} or {@link - * #instanceAdminStubSettingsBuilder()}. + * #getSpannerStubSettingsBuilder()}, {@link #getDatabaseAdminStubSettingsBuilder()} or {@link + * #getInstanceAdminStubSettingsBuilder()}. */ @Override public Builder setRetrySettings(RetrySettings retrySettings) { @@ -261,7 +261,7 @@ public Builder setRetrySettings(RetrySettings retrySettings) { * * 

      * builder
-     *     .spannerStubSettingsBuilder()
+     *     .getSpannerStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
      *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
      *           public Void apply(Builder<?, ?> input) {
@@ -271,7 +271,7 @@ public Builder setRetrySettings(RetrySettings retrySettings) {
      *         });
      *
*/ - public SpannerStubSettings.Builder spannerStubSettingsBuilder() { + public SpannerStubSettings.Builder getSpannerStubSettingsBuilder() { return spannerStubSettingsBuilder; } @@ -287,7 +287,7 @@ public SpannerStubSettings.Builder spannerStubSettingsBuilder() { * * 

      * builder
-     *     .instanceAdminStubSettingsBuilder()
+     *     .getInstanceAdminStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
      *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
      *           public Void apply(Builder<?, ?> input) {
@@ -297,7 +297,7 @@ public SpannerStubSettings.Builder spannerStubSettingsBuilder() {
      *         });
      *
*/ - public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { + public InstanceAdminStubSettings.Builder getInstanceAdminStubSettingsBuilder() { return instanceAdminStubSettingsBuilder; } @@ -313,7 +313,7 @@ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() { * * 

      * builder
-     *     .databaseAdminStubSettingsBuilder()
+     *     .getDatabaseAdminStubSettingsBuilder()
      *     .applyToAllUnaryMethods(
      *         new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
      *           public Void apply(Builder<?, ?> input) {
@@ -323,7 +323,7 @@ public InstanceAdminStubSettings.Builder instanceAdminStubSettingsBuilder() {
      *         });
      *
*/ - public DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder() { + public DatabaseAdminStubSettings.Builder getDatabaseAdminStubSettingsBuilder() { return databaseAdminStubSettingsBuilder; } diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java index 8d0911304989..ab6dd31d86fb 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseAdminGaxTest.java @@ -241,7 +241,7 @@ public void setUp() throws Exception { .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()); builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -251,22 +251,22 @@ public Void apply(Builder input) { } }); builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .createDatabaseOperationSettings() .setInitialCallSettings( builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .createDatabaseOperationSettings() .getInitialCallSettings() .toBuilder() .setRetrySettings(retrySettings) .build()); builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .updateDatabaseDdlOperationSettings() .setInitialCallSettings( builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .updateDatabaseDdlOperationSettings() .getInitialCallSettings() .toBuilder() @@ -275,7 +275,7 @@ public Void apply(Builder input) { if (!enableGaxRetries) { // Disable retries by removing all retryable codes. builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -285,11 +285,11 @@ public Void apply(Builder input) { } }); builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .createDatabaseOperationSettings() .setInitialCallSettings( builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .createDatabaseOperationSettings() .getInitialCallSettings() .toBuilder() @@ -297,11 +297,11 @@ public Void apply(Builder input) { .setRetryableCodes(ImmutableSet.of()) .build()); builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .updateDatabaseDdlOperationSettings() .setInitialCallSettings( builder - .databaseAdminStubSettingsBuilder() + .getDatabaseAdminStubSettingsBuilder() .updateDatabaseDdlOperationSettings() .getInitialCallSettings() .toBuilder() diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java index 1f79fe2c9b90..a50bdafe7b93 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/InstanceAdminGaxTest.java @@ -244,7 +244,7 @@ public void setUp() throws Exception { .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()); builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -254,22 +254,22 @@ public Void apply(Builder input) { } }); builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .createInstanceOperationSettings() .setInitialCallSettings( builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .createInstanceOperationSettings() .getInitialCallSettings() .toBuilder() .setRetrySettings(retrySettings) .build()); builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .updateInstanceOperationSettings() .setInitialCallSettings( builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .updateInstanceOperationSettings() .getInitialCallSettings() .toBuilder() @@ -278,7 +278,7 @@ public Void apply(Builder input) { if (!enableGaxRetries) { // Disable retries by removing all retryable codes. builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -288,11 +288,11 @@ public Void apply(Builder input) { } }); builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .createInstanceOperationSettings() .setInitialCallSettings( builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .createInstanceOperationSettings() .getInitialCallSettings() .toBuilder() @@ -300,11 +300,11 @@ public Void apply(Builder input) { .setRetryableCodes(ImmutableSet.of()) .build()); builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .updateInstanceOperationSettings() .setInitialCallSettings( builder - .instanceAdminStubSettingsBuilder() + .getInstanceAdminStubSettingsBuilder() .updateInstanceOperationSettings() .getInitialCallSettings() .toBuilder() diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 6c708ed0d814..3325536abb65 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -218,7 +218,7 @@ public void setUp() throws Exception { .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()); builder - .spannerStubSettingsBuilder() + .getSpannerStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -227,16 +227,16 @@ public Void apply(Builder input) { return null; } }); - builder.spannerStubSettingsBuilder().commitSettings().setRetrySettings(commitRetrySettings); + builder.getSpannerStubSettingsBuilder().commitSettings().setRetrySettings(commitRetrySettings); builder - .spannerStubSettingsBuilder() + .getSpannerStubSettingsBuilder() .executeStreamingSqlSettings() .setRetrySettings(retrySettings); - builder.spannerStubSettingsBuilder().streamingReadSettings().setRetrySettings(retrySettings); + builder.getSpannerStubSettingsBuilder().streamingReadSettings().setRetrySettings(retrySettings); if (!enableGaxRetries) { // Disable retries by removing all retryable codes. builder - .spannerStubSettingsBuilder() + .getSpannerStubSettingsBuilder() .applyToAllUnaryMethods( new ApiFunction, Void>() { @Override @@ -246,11 +246,11 @@ public Void apply(Builder input) { } }); builder - .spannerStubSettingsBuilder() + .getSpannerStubSettingsBuilder() .executeStreamingSqlSettings() .setRetryableCodes(ImmutableSet.of()); builder - .spannerStubSettingsBuilder() + .getSpannerStubSettingsBuilder() .streamingReadSettings() .setRetryableCodes(ImmutableSet.of()); } diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index fc6045810c43..dd28ca902808 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -149,7 +149,7 @@ public void testSpannerCustomRetrySettings() { .setTotalTimeout(Duration.ofSeconds(9999L)) .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); - SpannerStubSettings.Builder stubSettingsBuilder = builder.spannerStubSettingsBuilder(); + SpannerStubSettings.Builder stubSettingsBuilder = builder.getSpannerStubSettingsBuilder(); List> unaryCallSettingsBuilders = Arrays.asList( stubSettingsBuilder.beginTransactionSettings(), @@ -243,7 +243,7 @@ public void testDatabaseAdminCustomRetrySettings() { .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); DatabaseAdminStubSettings.Builder stubSettingsBuilder = - builder.databaseAdminStubSettingsBuilder(); + builder.getDatabaseAdminStubSettingsBuilder(); List> unaryCallSettingsBuilders = Arrays.asList( stubSettingsBuilder.dropDatabaseSettings(), @@ -307,7 +307,7 @@ public void testInstanceAdminCustomRetrySettings() { .build(); SpannerOptions.Builder builder = SpannerOptions.newBuilder().setProjectId("test-project"); InstanceAdminStubSettings.Builder stubSettingsBuilder = - builder.instanceAdminStubSettingsBuilder(); + builder.getInstanceAdminStubSettingsBuilder(); List> unaryCallSettingsBuilders = Arrays.asList( stubSettingsBuilder.deleteInstanceSettings(), From d90248e78940372c4f518cbf11df3eb1f0b77b0f Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Mon, 22 Apr 2019 18:48:32 +0200 Subject: [PATCH 22/26] remove unused import --- .../src/main/java/com/google/cloud/spanner/SpannerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 4dbe82b7ca96..988a26253937 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -24,7 +24,6 @@ import com.google.api.client.util.BackOff; import com.google.api.client.util.ExponentialBackOff; import com.google.api.gax.paging.Page; -import com.google.api.pathtemplate.PathTemplate; import com.google.cloud.BaseService; import com.google.cloud.PageImpl; import com.google.cloud.PageImpl.NextPageFetcher; From 38267c6a3d0723ff052cd131660649e2c110bd44 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Tue, 23 Apr 2019 12:17:42 +0200 Subject: [PATCH 23/26] ran code formatter --- .../src/main/java/com/google/cloud/spanner/SpannerImpl.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java index 988a26253937..10672da94083 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerImpl.java @@ -292,7 +292,6 @@ Object value() { return ImmutableMap.copyOf(tmp); } - /** Helper class for gRPC calls that can return paginated results. */ abstract static class PageFetcher implements NextPageFetcher { private String nextPageToken; From df2ed1f7ef8cca9b59c438b299e915a7320001d6 Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 17 May 2019 11:36:51 +0200 Subject: [PATCH 24/26] added references to default values --- .../google/cloud/spanner/SpannerOptions.java | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index a23a187e7ba0..7de40268b89c 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -253,9 +253,11 @@ public Builder setRetrySettings(RetrySettings retrySettings) { * Returns the {@link SpannerStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * - *

The library will automatically use sensible defaults if no custom settings are set. The - * defaults are the same as the defaults that are used by {@link SpannerSettings}. Retries are - * configured for idempotent methods but not for non-idempotent methods. + *

The library will automatically use the defaults defined in {@link SpannerStubSettings} if + * no custom settings are set. The defaults are the same as the defaults that are used by {@link + * SpannerSettings}, and are generated from the file spanner_gapic.yaml. + * Retries are configured for idempotent methods but not for non-idempotent methods. * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * @@ -279,9 +281,11 @@ public SpannerStubSettings.Builder getSpannerStubSettingsBuilder() { * Returns the {@link InstanceAdminStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * - *

The library will automatically use sensible defaults if no custom settings are set. The - * defaults are the same as the defaults that are used by {@link InstanceAdminSettings}. Retries - * are configured for idempotent methods but not for non-idempotent methods. + *

The library will automatically use the defaults defined in {@link + * InstanceAdminStubSettings} if no custom settings are set. The defaults are the same as the + * defaults that are used by {@link InstanceAdminSettings}, and are generated from the file spanner_admin_instance_gapic.yaml. + * Retries are configured for idempotent methods but not for non-idempotent methods. * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * @@ -305,9 +309,11 @@ public InstanceAdminStubSettings.Builder getInstanceAdminStubSettingsBuilder() { * Returns the {@link DatabaseAdminStubSettings.Builder} that will be used to build the {@link * SpannerRpc}. Use this to set custom {@link RetrySettings} for individual gRPC methods. * - *

The library will automatically use sensible defaults if no custom settings are set. The - * defaults are the same as the defaults that are used by {@link DatabaseAdminSettings}. Retries - * are configured for idempotent methods but not for non-idempotent methods. + *

The library will automatically use the defaults defined in {@link + * DatabaseAdminStubSettings} if no custom settings are set. The defaults are the same as the + * defaults that are used by {@link DatabaseAdminSettings}, and are generated from the file spanner_admin_database_gapic.yaml. + * Retries are configured for idempotent methods but not for non-idempotent methods. * *

You can set the same {@link RetrySettings} for all unary methods by calling this: * From 000f3c345b2504b21475d9b54d31d20ec195facc Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 17 May 2019 15:05:50 +0200 Subject: [PATCH 25/26] remove ignored test --- .../com/google/cloud/spanner/SpannerGaxRetryTest.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 3325536abb65..171db3e4f6b0 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -467,15 +467,4 @@ public void transactionManagerUnavailable() { } } } - - // TODO(loite): Check what happens when an SSLHandshakeException is thrown when using GAX. - @Test - @Ignore - public void singleUseSslHandshakeException() { - if (!enableGaxRetries) { - expectedException.expect(SpannerMatchers.isSpannerException(ErrorCode.UNAVAILABLE)); - } - mockSpanner.addException(new SSLHandshakeException("some SSL handshake exception")); - ((SpannerImpl) spanner).createSession(DatabaseId.of("[PROJECT]", "[INSTANCE]", "[DATABASE]")); - } } From 1302c0611f3776a25eeff980bc157e4797b4f04d Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 17 May 2019 15:31:17 +0200 Subject: [PATCH 26/26] ran formatter --- .../main/java/com/google/cloud/spanner/SpannerOptions.java | 5 ----- .../com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java | 1 + .../java/com/google/cloud/spanner/SpannerGaxRetryTest.java | 2 -- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 7de40268b89c..cf3e7676b44e 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -17,11 +17,6 @@ package com.google.cloud.spanner; import com.google.api.core.ApiFunction; -import java.io.IOException; -import java.net.MalformedURLException; -import java.net.URL; -import java.util.Map; -import java.util.Set; import com.google.api.gax.grpc.GrpcInterceptorProvider; import com.google.api.gax.retrying.RetrySettings; import com.google.api.gax.rpc.TransportChannelProvider; diff --git a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java index 950922e33e0d..a6ca0e003ac5 100644 --- a/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java +++ b/google-cloud-clients/google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java @@ -598,6 +598,7 @@ private GrpcCallContext newCallContext(@Nullable Map options, String return context.withStreamWaitTimeout(waitTimeout).withStreamIdleTimeout(idleTimeout); } + @Override public void shutdown() { this.spannerStub.close(); this.instanceAdminStub.close(); diff --git a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java index 171db3e4f6b0..d1535c8d2bc0 100644 --- a/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java +++ b/google-cloud-clients/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerGaxRetryTest.java @@ -48,12 +48,10 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.ScheduledThreadPoolExecutor; -import javax.net.ssl.SSLHandshakeException; import org.junit.After; import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException;