From d2bfda922275cc5098a97548517ef848c2e48c82 Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Thu, 9 Jan 2025 11:20:23 -0800 Subject: [PATCH 1/9] feat: Improve upon the default gRPC Connection Pool size --- .../java/com/google/cloud/datastore/DatastoreOptions.java | 5 ++++- .../google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 7dda76f76..30c4ce7c7 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -19,6 +19,7 @@ import static com.google.cloud.datastore.Validator.validateNamespace; import com.google.api.core.BetaApi; +import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider; import com.google.api.gax.rpc.TransportChannelProvider; import com.google.cloud.ServiceDefaults; @@ -222,7 +223,9 @@ private DatastoreOptions(Builder builder) { builder.channelProvider != null ? builder.channelProvider : GrpcTransportOptions.setUpChannelProvider( - DatastoreSettings.defaultGrpcTransportProviderBuilder(), this); + DatastoreSettings.defaultGrpcTransportProviderBuilder().setChannelPoolSettings( + ChannelPoolSettings.builder().setMinChannelCount(1).setInitialChannelCount(4) + .build()), this); } } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index e8e113736..808b0f46a 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -24,6 +24,7 @@ import com.google.api.core.InternalApi; import com.google.api.gax.core.BackgroundResource; import com.google.api.gax.core.GaxProperties; +import com.google.api.gax.grpc.ChannelPoolSettings; import com.google.api.gax.grpc.GrpcCallContext; import com.google.api.gax.grpc.GrpcTransportChannel; import com.google.api.gax.rpc.ClientContext; @@ -74,9 +75,16 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { ? getClientContextForEmulator(datastoreOptions) : getClientContext(datastoreOptions); + // DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) .applyToAllUnaryMethods(retrySettingSetter(datastoreOptions)) + .setTransportChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder() + .setChannelPoolSettings(ChannelPoolSettings.builder() + .setMinChannelCount(1) + .setMaxChannelCount(4) + .build()) + .build()) .build(); datastoreStub = GrpcDatastoreStub.create(datastoreStubSettings); } catch (IOException e) { From 9b3f24f69d9dfa969941531b0eebc442f3f1862f Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Thu, 9 Jan 2025 11:20:47 -0800 Subject: [PATCH 2/9] feat: Improve upon the default gRPC Connection Pool size --- .../java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 808b0f46a..cafa7a5f6 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -75,7 +75,6 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { ? getClientContextForEmulator(datastoreOptions) : getClientContext(datastoreOptions); - // DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) .applyToAllUnaryMethods(retrySettingSetter(datastoreOptions)) From 2b66204d6c4b155d21c403b0ad3ac589951a705f Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 10:20:31 -0800 Subject: [PATCH 3/9] fix formatting --- .../com/google/cloud/datastore/DatastoreOptions.java | 10 +++++++--- .../cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 12 +++++++----- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 30c4ce7c7..0a17afd8e 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -223,9 +223,13 @@ private DatastoreOptions(Builder builder) { builder.channelProvider != null ? builder.channelProvider : GrpcTransportOptions.setUpChannelProvider( - DatastoreSettings.defaultGrpcTransportProviderBuilder().setChannelPoolSettings( - ChannelPoolSettings.builder().setMinChannelCount(1).setInitialChannelCount(4) - .build()), this); + DatastoreSettings.defaultGrpcTransportProviderBuilder() + .setChannelPoolSettings( + ChannelPoolSettings.builder() + .setMinChannelCount(1) + .setInitialChannelCount(4) + .build()), + this); } } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index cafa7a5f6..0016a99f4 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -78,12 +78,14 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) .applyToAllUnaryMethods(retrySettingSetter(datastoreOptions)) - .setTransportChannelProvider(DatastoreSettings.defaultGrpcTransportProviderBuilder() - .setChannelPoolSettings(ChannelPoolSettings.builder() - .setMinChannelCount(1) - .setMaxChannelCount(4) + .setTransportChannelProvider( + DatastoreSettings.defaultGrpcTransportProviderBuilder() + .setChannelPoolSettings( + ChannelPoolSettings.builder() + .setMinChannelCount(1) + .setMaxChannelCount(4) + .build()) .build()) - .build()) .build(); datastoreStub = GrpcDatastoreStub.create(datastoreStubSettings); } catch (IOException e) { From ed989ea3eb6fcc723039a462d5772f4b2107a83e Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 11:31:36 -0800 Subject: [PATCH 4/9] add comment --- .../main/java/com/google/cloud/datastore/DatastoreOptions.java | 3 ++- .../com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 0a17afd8e..1bf1b11aa 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -219,6 +219,7 @@ private DatastoreOptions(Builder builder) { throw new IllegalArgumentException( "Only gRPC transport allows setting of channel provider or credentials provider"); } else if (getTransportOptions() instanceof GrpcTransportOptions) { + // For grpc transport options, configure default gRPC Connection pool with minChannelCount = 1 and maxChannelCount = 4 this.channelProvider = builder.channelProvider != null ? builder.channelProvider @@ -227,7 +228,7 @@ private DatastoreOptions(Builder builder) { .setChannelPoolSettings( ChannelPoolSettings.builder() .setMinChannelCount(1) - .setInitialChannelCount(4) + .setMaxChannelCount(4) .build()), this); } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index 0016a99f4..4308f9ee3 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -75,6 +75,7 @@ public GrpcDatastoreRpc(DatastoreOptions datastoreOptions) throws IOException { ? getClientContextForEmulator(datastoreOptions) : getClientContext(datastoreOptions); + /* For grpc transport options, configure default gRPC Connection pool with minChannelCount = 1 and maxChannelCount = 4 */ DatastoreStubSettings datastoreStubSettings = DatastoreStubSettings.newBuilder(clientContext) .applyToAllUnaryMethods(retrySettingSetter(datastoreOptions)) From 32888908c1ffa00a3fc245649f119c4b5f9b6a3d Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:21:34 -0800 Subject: [PATCH 5/9] Add test --- .../cloud/datastore/DatastoreOptionsTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index aff771383..9d644bdac 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -103,6 +103,23 @@ public void testDatastore() { assertSame(datastoreRpc, options.build().getRpc()); } + @Test + public void testGrpcDefaultChannelConfigurations() { + DatastoreOptions datastoreOptions = + DatastoreOptions.newBuilder() + .setServiceRpcFactory(datastoreRpcFactory) + .setProjectId(PROJECT_ID) + .setDatabaseId(DATABASE_ID) + .setTransportOptions(GrpcTransportOptions.newBuilder().build()) + .setCredentials(NoCredentials.getInstance()) + .setHost("http://localhost:" + PORT) + .build(); + ChannelPoolSettings channelPoolSettings = ((InstantiatingGrpcChannelProvider) datastoreOptions.getTransportChannelProvider()).getChannelPoolSettings(); + assertEquals(channelPoolSettings.getInitialChannelCount(), 1); + assertEquals(channelPoolSettings.getMinChannelCount(), 1); + assertEquals(channelPoolSettings.getMaxChannelCount(), 4); + } + @Test public void testCustomChannelAndCredentials() { InstantiatingGrpcChannelProvider channelProvider = From 2495036a80b19456a8a43e9705b31ea584540ad3 Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 12:37:19 -0800 Subject: [PATCH 6/9] formatting --- .../java/com/google/cloud/datastore/DatastoreOptions.java | 3 ++- .../java/com/google/cloud/datastore/DatastoreOptionsTest.java | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 1bf1b11aa..617536c28 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -219,7 +219,8 @@ private DatastoreOptions(Builder builder) { throw new IllegalArgumentException( "Only gRPC transport allows setting of channel provider or credentials provider"); } else if (getTransportOptions() instanceof GrpcTransportOptions) { - // For grpc transport options, configure default gRPC Connection pool with minChannelCount = 1 and maxChannelCount = 4 + // For grpc transport options, configure default gRPC Connection pool with minChannelCount = 1 + // and maxChannelCount = 4 this.channelProvider = builder.channelProvider != null ? builder.channelProvider diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java index 9d644bdac..28172cbf3 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreOptionsTest.java @@ -114,7 +114,9 @@ public void testGrpcDefaultChannelConfigurations() { .setCredentials(NoCredentials.getInstance()) .setHost("http://localhost:" + PORT) .build(); - ChannelPoolSettings channelPoolSettings = ((InstantiatingGrpcChannelProvider) datastoreOptions.getTransportChannelProvider()).getChannelPoolSettings(); + ChannelPoolSettings channelPoolSettings = + ((InstantiatingGrpcChannelProvider) datastoreOptions.getTransportChannelProvider()) + .getChannelPoolSettings(); assertEquals(channelPoolSettings.getInitialChannelCount(), 1); assertEquals(channelPoolSettings.getMinChannelCount(), 1); assertEquals(channelPoolSettings.getMaxChannelCount(), 4); From 765f311e1cfed878456de79c7ded0ff87a950937 Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 14:29:40 -0800 Subject: [PATCH 7/9] Add constants for min, max and init channel count --- .../java/com/google/cloud/datastore/DatastoreOptions.java | 8 ++++++-- .../google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java | 5 +++-- .../com/google/cloud/datastore/DatastoreOptionsTest.java | 6 +++--- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java index 617536c28..1ea79298c 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreOptions.java @@ -51,6 +51,9 @@ public class DatastoreOptions extends ServiceOptions Date: Mon, 13 Jan 2025 19:43:31 -0800 Subject: [PATCH 8/9] Fix ci testing environment with java 11 --- .github/workflows/ci.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b91fa381f..7bfe5e3c0 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -59,11 +59,21 @@ jobs: env: JOB_TYPE: test windows: + # Building using Java 8 and run the tests with Java 11 runtime runs-on: windows-latest steps: - name: Support longpaths run: git config --system core.longpaths true - uses: actions/checkout@v4 + - uses: actions/setup-java@v4 + with: + java-version: 11 + distribution: temurin + - name: "Set jvm system property environment variable for surefire plugin (unit tests)" + # Maven surefire plugin (unit tests) allows us to specify JVM to run the tests. + # https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#jvm + run: echo "SUREFIRE_JVM_OPT=-Djvm=${JAVA_HOME}/bin/java" >> $GITHUB_ENV + shell: bash - uses: actions/setup-java@v4 with: distribution: temurin From bd49ae097c001ed712da27a84ea1a17a5654828b Mon Sep 17 00:00:00 2001 From: Cindy Peng <148148319+cindy-peng@users.noreply.github.com> Date: Mon, 13 Jan 2025 19:50:20 -0800 Subject: [PATCH 9/9] Revert "Fix ci testing environment with java 11" This reverts commit 00c0d2146c814d428d8ae06f75d4c93b6f826851. --- .github/workflows/ci.yaml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7bfe5e3c0..b91fa381f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -59,21 +59,11 @@ jobs: env: JOB_TYPE: test windows: - # Building using Java 8 and run the tests with Java 11 runtime runs-on: windows-latest steps: - name: Support longpaths run: git config --system core.longpaths true - uses: actions/checkout@v4 - - uses: actions/setup-java@v4 - with: - java-version: 11 - distribution: temurin - - name: "Set jvm system property environment variable for surefire plugin (unit tests)" - # Maven surefire plugin (unit tests) allows us to specify JVM to run the tests. - # https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#jvm - run: echo "SUREFIRE_JVM_OPT=-Djvm=${JAVA_HOME}/bin/java" >> $GITHUB_ENV - shell: bash - uses: actions/setup-java@v4 with: distribution: temurin