From 41d960f3039c4e8621b027e7feb0d9e49b4e5f4e Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 15 May 2020 09:40:39 +0200 Subject: [PATCH 1/2] tests: add additional tests for client ids --- .../cloud/spanner/DatabaseClientImplTest.java | 32 +++++++++++++++++++ .../google/cloud/spanner/SpannerImplTest.java | 7 ++++ 2 files changed, 39 insertions(+) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 3b61e0cb92..8c0a8bf9f3 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -41,6 +41,7 @@ import io.grpc.StatusRuntimeException; import io.grpc.inprocess.InProcessServerBuilder; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -927,4 +928,35 @@ public void testBackendPartitionQueryOptions() { assertThat(request.getQueryOptions().getOptimizerVersion()).isEqualTo("1"); } } + + @Test + public void testClientIdReusedOnDatabaseNotFound() { + mockSpanner.setBatchCreateSessionsExecutionTime( + SimulatedExecutionTime.ofStickyException( + SpannerExceptionFactoryTest.newStatusResourceNotFoundException( + "my-database", + SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, + "project/my-project/instances/my-instance/databases/my-database"))); + Spanner spanner = + SpannerOptions.newBuilder() + .setProjectId("my-project") + .setChannelProvider(channelProvider) + .setCredentials(NoCredentials.getInstance()) + .build() + .getService(); + DatabaseId databaseId = DatabaseId.of("my-project", "my-instance", "my-database"); + String prevClientId = null; + for (int i = 0; i < 100; i++) { + try { + DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(databaseId); + if (prevClientId != null) { + assertThat(client.clientId).isEqualTo(prevClientId); + } + prevClientId = client.clientId; + client.singleUse().readRow("MyTable", Key.of(0), Arrays.asList("MyColumn")); + } catch (Exception e) { + // ignore + } + } + } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java index 25f3381bd1..25a77016b8 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerImplTest.java @@ -223,6 +223,13 @@ public void testClientId() throws Exception { assertThat(revalidated).isNotSameInstanceAs(databaseClient); assertThat(revalidated.clientId).isEqualTo(databaseClient.clientId); + // Now invalidate the second client and request a new one. + revalidated.pool.setResourceNotFoundException( + new DatabaseNotFoundException(DoNotConstructDirectly.ALLOWED, "not found", null, null)); + DatabaseClientImpl revalidated2 = (DatabaseClientImpl) impl.getDatabaseClient(db); + assertThat(revalidated2).isNotSameInstanceAs(revalidated); + assertThat(revalidated2.clientId).isEqualTo(revalidated.clientId); + // Create a new Spanner instance. This will generate new database clients with new ids. try (Spanner spanner = SpannerOptions.newBuilder() From 682648d85261b2bd7fa75c9758dacf4c1ecdda4b Mon Sep 17 00:00:00 2001 From: Olav Loite Date: Fri, 15 May 2020 16:06:38 +0200 Subject: [PATCH 2/2] fix: close Spanner to prevent resource leak --- .../cloud/spanner/DatabaseClientImplTest.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java index 8c0a8bf9f3..ac57476c12 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/DatabaseClientImplTest.java @@ -937,25 +937,26 @@ public void testClientIdReusedOnDatabaseNotFound() { "my-database", SpannerExceptionFactory.DATABASE_RESOURCE_TYPE, "project/my-project/instances/my-instance/databases/my-database"))); - Spanner spanner = + try (Spanner spanner = SpannerOptions.newBuilder() .setProjectId("my-project") .setChannelProvider(channelProvider) .setCredentials(NoCredentials.getInstance()) .build() - .getService(); - DatabaseId databaseId = DatabaseId.of("my-project", "my-instance", "my-database"); - String prevClientId = null; - for (int i = 0; i < 100; i++) { - try { - DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(databaseId); - if (prevClientId != null) { - assertThat(client.clientId).isEqualTo(prevClientId); + .getService()) { + DatabaseId databaseId = DatabaseId.of("my-project", "my-instance", "my-database"); + String prevClientId = null; + for (int i = 0; i < 100; i++) { + try { + DatabaseClientImpl client = (DatabaseClientImpl) spanner.getDatabaseClient(databaseId); + if (prevClientId != null) { + assertThat(client.clientId).isEqualTo(prevClientId); + } + prevClientId = client.clientId; + client.singleUse().readRow("MyTable", Key.of(0), Arrays.asList("MyColumn")); + } catch (Exception e) { + // ignore } - prevClientId = client.clientId; - client.singleUse().readRow("MyTable", Key.of(0), Arrays.asList("MyColumn")); - } catch (Exception e) { - // ignore } } }