From e7ec96ec09a9d273d4f576356d3e4c6cbbb6de9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 18 Mar 2021 06:33:32 +0100 Subject: [PATCH 1/3] feat!: add closeAsync() method to Connection (#984) Co-authored-by: Thiago Nunes --- .../clirr-ignored-differences.xml | 7 ++ .../cloud/spanner/connection/Connection.java | 11 +++- .../spanner/connection/ConnectionImpl.java | 64 +++++++++++++------ 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index f340f5e1963..efc5fd4de36 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -571,4 +571,11 @@ com/google/cloud/spanner/BackupInfo$Builder com.google.cloud.spanner.BackupInfo$Builder setEncryptionConfig(com.google.cloud.spanner.encryption.BackupEncryptionConfig) + + + + 7012 + com/google/cloud/spanner/connection/Connection + com.google.api.core.ApiFuture closeAsync() + diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java index bd1214d6a19..4a0bcf27019 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java @@ -139,10 +139,19 @@ */ @InternalApi public interface Connection extends AutoCloseable { - /** Closes this connection. This is a no-op if the {@link Connection} has alread been closed. */ + + /** Closes this connection. This is a no-op if the {@link Connection} has already been closed. */ @Override void close(); + /** + * Closes this connection without blocking. This is a no-op if the {@link Connection} has already + * been closed. The {@link Connection} is no longer usable directly after calling this method. The + * returned {@link ApiFuture} is done when the running statement(s) (if any) on the connection + * have finished. + */ + ApiFuture closeAsync(); + /** @return true if this connection has been closed. */ boolean isClosed(); diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java index 4f9703807a8..41fca0fcfbc 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java @@ -18,6 +18,7 @@ import static com.google.cloud.spanner.SpannerApiFutures.get; +import com.google.api.core.ApiFunction; import com.google.api.core.ApiFuture; import com.google.api.core.ApiFutures; import com.google.cloud.Timestamp; @@ -42,6 +43,7 @@ import com.google.cloud.spanner.connection.UnitOfWork.UnitOfWorkState; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.util.concurrent.MoreExecutors; import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions; import java.util.ArrayList; import java.util.Collections; @@ -49,8 +51,11 @@ import java.util.LinkedList; import java.util.List; import java.util.Stack; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.threeten.bp.Instant; /** Implementation for {@link Connection}, the generic Spanner connection API (not JDBC). */ @@ -257,28 +262,49 @@ private DdlClient createDdlClient() { @Override public void close() { + try { + closeAsync().get(10L, TimeUnit.SECONDS); + } catch (SpannerException | InterruptedException | ExecutionException | TimeoutException e) { + // ignore and continue to close the connection. + } finally { + statementExecutor.shutdownNow(); + } + } + + public ApiFuture closeAsync() { if (!isClosed()) { - try { - if (isTransactionStarted()) { - try { - rollback(); - } catch (Exception e) { - // Ignore as we are closing the connection. - } - } - // Try to wait for the current statement to finish (if any) before we actually close the - // connection. - this.closed = true; - statementExecutor.shutdown(); - leakedException = null; - spannerPool.removeConnection(options, this); - statementExecutor.awaitTermination(10L, TimeUnit.SECONDS); - } catch (InterruptedException e) { - // ignore and continue to close the connection. - } finally { - statementExecutor.shutdownNow(); + List> futures = new ArrayList<>(); + if (isTransactionStarted()) { + futures.add(rollbackAsync()); } + // Try to wait for the current statement to finish (if any) before we actually close the + // connection. + this.closed = true; + // Add a no-op statement to the execute. Once this has been executed, we know that all + // preceeding statements have also been executed, as the executor is single-threaded and + // executes all statements in order of submitting. + futures.add( + statementExecutor.submit( + new Callable() { + @Override + public Void call() throws Exception { + return null; + } + })); + statementExecutor.shutdown(); + leakedException = null; + spannerPool.removeConnection(options, this); + return ApiFutures.transform( + ApiFutures.allAsList(futures), + new ApiFunction, Void>() { + @Override + public Void apply(List input) { + return null; + } + }, + MoreExecutors.directExecutor()); } + return ApiFutures.immediateFuture(null); } /** Get the current unit-of-work type of this connection. */ From fc305b7cbf59cbdeecf1d0486b7f5ca86925148e Mon Sep 17 00:00:00 2001 From: Yoshi Automation Bot Date: Wed, 17 Mar 2021 22:34:03 -0700 Subject: [PATCH 2/3] chore: regenerate README (#993) This PR was generated using Autosynth. :rainbow:
Log from Synthtool ``` 2021-03-18 05:00:56,730 synthtool [DEBUG] > Executing /root/.cache/synthtool/java-spanner/.github/readme/synth.py. On branch autosynth-readme nothing to commit, working tree clean 2021-03-18 05:00:57,698 synthtool [DEBUG] > Wrote metadata to .github/readme/synth.metadata/synth.metadata. ```
Full log will be available here: https://source.cloud.google.com/results/invocations/4feedb3a-2e39-4824-b0f1-48e0dce90a14/targets - [ ] To automatically regenerate this PR, check this box. --- .github/readme/synth.metadata/synth.metadata | 2 +- README.md | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/readme/synth.metadata/synth.metadata b/.github/readme/synth.metadata/synth.metadata index f2f3015d804..dac2f0ec35a 100644 --- a/.github/readme/synth.metadata/synth.metadata +++ b/.github/readme/synth.metadata/synth.metadata @@ -4,7 +4,7 @@ "git": { "name": ".", "remote": "https://github.com/googleapis/java-spanner.git", - "sha": "8eab323893fda4a464f66cffa87f553f6888c8f8" + "sha": "7af19514dfae5f87ba50572d8867568d2c09daab" } }, { diff --git a/README.md b/README.md index 65772740c1b..c103612f20c 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ Cloud Spanner uses gRPC for the transport layer. ## Java Versions -Java 8 or above is required for using this client. +Java 7 or above is required for using this client. ## Versioning @@ -285,6 +285,7 @@ Apache 2.0 - See [LICENSE][license] for more information. Java Version | Status ------------ | ------ +Java 7 | [![Kokoro CI][kokoro-badge-image-1]][kokoro-badge-link-1] Java 8 | [![Kokoro CI][kokoro-badge-image-2]][kokoro-badge-link-2] Java 8 OSX | [![Kokoro CI][kokoro-badge-image-3]][kokoro-badge-link-3] Java 8 Windows | [![Kokoro CI][kokoro-badge-image-4]][kokoro-badge-link-4] @@ -294,6 +295,8 @@ Java is a registered trademark of Oracle and/or its affiliates. [product-docs]: https://cloud.google.com/spanner/docs/ [javadocs]: https://googleapis.dev/java/google-cloud-spanner/latest/ +[kokoro-badge-image-1]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java7.svg +[kokoro-badge-link-1]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java7.html [kokoro-badge-image-2]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java8.svg [kokoro-badge-link-2]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java8.html [kokoro-badge-image-3]: http://storage.googleapis.com/cloud-devrel-public/java/badges/java-spanner/java8-osx.svg From 459a47756becc49c25551799d76c1771b876978f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Knut=20Olav=20L=C3=B8ite?= Date: Thu, 18 Mar 2021 09:14:02 +0100 Subject: [PATCH 3/3] test: fail if the same token is seen twice (#983) The backup pagination test should fail if the same page token is returned twice by the backend, instead of fetching the same page indefinitely. Further investigation is necessary if that actually happens, as it is not something that is expected. --- .../com/google/cloud/spanner/it/ITBackupTest.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBackupTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBackupTest.java index 53dcc7907f8..6a2742c2d94 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBackupTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/it/ITBackupTest.java @@ -63,8 +63,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; -import java.util.Random; +import java.util.Set; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -100,7 +101,6 @@ public class ITBackupTest { private RemoteSpannerHelper testHelper; private List databases = new ArrayList<>(); private List backups = new ArrayList<>(); - private final Random random = new Random(); private String projectId; private String instanceId; @@ -580,8 +580,15 @@ private void testPagination(int expectedMinimumTotalBackups) { assertThat(page.getValues()).hasSize(1); numBackups++; assertThat(page.hasNextPage()).isTrue(); + Set seenPageTokens = new HashSet<>(); + seenPageTokens.add(""); while (page.hasNextPage()) { - logger.info(String.format("Fetching page %d", numBackups + 1)); + logger.info( + String.format( + "Fetching page %d with page token %s", numBackups + 1, page.getNextPageToken())); + // The backend should not return the same page token twice. + assertThat(seenPageTokens).doesNotContain(page.getNextPageToken()); + seenPageTokens.add(page.getNextPageToken()); page = dbAdminClient.listBackups( instanceId, Options.pageToken(page.getNextPageToken()), Options.pageSize(1));