From 0b60e9ea8291e47d27a66a70c18dd8652ebcce35 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 5 Mar 2025 08:44:06 +0000 Subject: [PATCH 1/7] chore: enable multplexed sessions for experimental host --- .../cloud/spanner/SessionPoolOptions.java | 41 ++++++++++++++----- .../google/cloud/spanner/SpannerOptions.java | 22 +++++----- .../spanner/connection/ConnectionOptions.java | 30 +++++++++++--- .../connection/ConnectionProperties.java | 11 ++++- .../cloud/spanner/SpannerOptionsTest.java | 18 +++++--- .../connection/ConnectionOptionsTest.java | 35 ++++++++++++++++ 6 files changed, 123 insertions(+), 34 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 171c10c9c92..b45569c9325 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -24,6 +24,7 @@ import com.google.cloud.spanner.SessionPool.Position; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import io.grpc.ExperimentalApi; import java.time.Duration; import java.util.Locale; import java.util.Objects; @@ -34,6 +35,7 @@ public class SessionPoolOptions { private static final int DEFAULT_MAX_SESSIONS = 400; private static final int DEFAULT_MIN_SESSIONS = 100; private static final int DEFAULT_INC_STEP = 25; + private static final int EXPERIMENTAL_HOST_MIN_SESSIONS = 0; private static final ActionOnExhaustion DEFAULT_ACTION = ActionOnExhaustion.BLOCK; private final int minSessions; private final int maxSessions; @@ -89,7 +91,10 @@ private SessionPoolOptions(Builder builder) { // minSessions > maxSessions is only possible if the user has only set a value for maxSessions. // We allow that to prevent code that only sets a value for maxSessions to break if the // maxSessions value is less than the default for minSessions. - this.minSessions = Math.min(builder.minSessions, builder.maxSessions); + this.minSessions = + builder.isExperimentalHost + ? EXPERIMENTAL_HOST_MIN_SESSIONS + : Math.min(builder.minSessions, builder.maxSessions); this.maxSessions = builder.maxSessions; this.incStep = builder.incStep; this.maxIdleSessions = builder.maxIdleSessions; @@ -114,26 +119,33 @@ private SessionPoolOptions(Builder builder) { // useMultiplexedSession priority => Environment var > private setter > client default Boolean useMultiplexedSessionFromEnvVariable = getUseMultiplexedSessionFromEnvVariable(); this.useMultiplexedSession = - (useMultiplexedSessionFromEnvVariable != null) - ? useMultiplexedSessionFromEnvVariable - : builder.useMultiplexedSession; + builder.isExperimentalHost + ? true + : ((useMultiplexedSessionFromEnvVariable != null) + ? useMultiplexedSessionFromEnvVariable + : builder.useMultiplexedSession); // useMultiplexedSessionForRW priority => Environment var > private setter > client default Boolean useMultiplexedSessionForRWFromEnvVariable = getUseMultiplexedSessionForRWFromEnvVariable(); this.useMultiplexedSessionForRW = - (useMultiplexedSessionForRWFromEnvVariable != null) - ? useMultiplexedSessionForRWFromEnvVariable - : builder.useMultiplexedSessionForRW; + builder.isExperimentalHost + ? true + : ((useMultiplexedSessionForRWFromEnvVariable != null) + ? useMultiplexedSessionForRWFromEnvVariable + : builder.useMultiplexedSessionForRW); // useMultiplexedSessionPartitionedOps priority => Environment var > private setter > client // default Boolean useMultiplexedSessionFromEnvVariablePartitionedOps = getUseMultiplexedSessionFromEnvVariablePartitionedOps(); this.useMultiplexedSessionForPartitionedOps = - (useMultiplexedSessionFromEnvVariablePartitionedOps != null) - ? useMultiplexedSessionFromEnvVariablePartitionedOps - : builder.useMultiplexedSessionPartitionedOps; + builder.isExperimentalHost + ? true + : ((useMultiplexedSessionFromEnvVariablePartitionedOps != null) + ? useMultiplexedSessionFromEnvVariablePartitionedOps + : builder.useMultiplexedSessionPartitionedOps); this.multiplexedSessionMaintenanceDuration = builder.multiplexedSessionMaintenanceDuration; - this.skipVerifyingBeginTransactionForMuxRW = builder.skipVerifyingBeginTransactionForMuxRW; + this.skipVerifyingBeginTransactionForMuxRW = + builder.isExperimentalHost ? true : builder.skipVerifyingBeginTransactionForMuxRW; } @Override @@ -617,6 +629,7 @@ public static class Builder { private Duration multiplexedSessionMaintenanceDuration = Duration.ofDays(7); private Clock poolMaintainerClock = Clock.INSTANCE; private boolean skipVerifyingBeginTransactionForMuxRW = false; + private boolean isExperimentalHost = false; private static Position getReleaseToPositionFromSystemProperty() { // NOTE: This System property is a beta feature. Support for it can be removed in the future. @@ -813,6 +826,12 @@ public Builder setWarnAndCloseIfInactiveTransactions() { return this; } + @ExperimentalApi("") + public Builder setExperimentalHost() { + this.isExperimentalHost = true; + return this; + } + /** * If there are inactive transactions, release the resources consumed by such transactions. A * transaction is classified as inactive if it executes for more than a system defined duration. diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index 43978a1d04c..dcddbb69212 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -98,7 +98,6 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import java.util.regex.Pattern; import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; @@ -117,10 +116,7 @@ public class SpannerOptions extends ServiceOptions { private static final String API_SHORT_NAME = "Spanner"; private static final String DEFAULT_HOST = "https://spanner.googleapis.com"; - private static final String CLOUD_SPANNER_HOST_FORMAT = ".*\\.googleapis\\.com.*"; - - @VisibleForTesting - static final Pattern CLOUD_SPANNER_HOST_PATTERN = Pattern.compile(CLOUD_SPANNER_HOST_FORMAT); + private static final String EXPERIMENTAL_HOST_PROJECT_ID = "default"; private static final ImmutableSet SCOPES = ImmutableSet.of( @@ -991,7 +987,7 @@ public static class Builder private boolean enableBuiltInMetrics = SpannerOptions.environment.isEnableBuiltInMetrics(); private String monitoringHost = SpannerOptions.environment.getMonitoringHost(); private SslContext mTLSContext = null; - private boolean isExternalHost = false; + private boolean isExperimentalHost = false; private static String createCustomClientLibToken(String token) { return token + " " + ServiceOptions.getGoogApiClientLibName(); @@ -1484,14 +1480,20 @@ public Builder setDecodeMode(DecodeMode decodeMode) { @Override public Builder setHost(String host) { super.setHost(host); - if (!CLOUD_SPANNER_HOST_PATTERN.matcher(host).matches()) { - this.isExternalHost = true; - } // Setting a host should override any SPANNER_EMULATOR_HOST setting. setEmulatorHost(null); return this; } + @ExperimentalApi("") + public Builder setExperimentalHost(String host) { + super.setHost(host); + super.setProjectId(EXPERIMENTAL_HOST_PROJECT_ID); + setSessionPoolOption(SessionPoolOptions.newBuilder().setExperimentalHost().build()); + this.isExperimentalHost = true; + return this; + } + /** * Enables gRPC-GCP extension with the default settings. Do not set * GOOGLE_CLOUD_SPANNER_MULTIPLEXED_SESSIONS to true in combination with this option, as @@ -1657,7 +1659,7 @@ public SpannerOptions build() { this.setChannelConfigurator(ManagedChannelBuilder::usePlaintext); // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); - } else if (isExternalHost && credentials == null) { + } else if (isExperimentalHost && credentials == null) { credentials = environment.getDefaultExternalHostCredentials(); } if (this.numChannels == null) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index b766545a799..71155759058 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -32,6 +32,7 @@ import static com.google.cloud.spanner.connection.ConnectionProperties.ENABLE_EXTENDED_TRACING; import static com.google.cloud.spanner.connection.ConnectionProperties.ENCODED_CREDENTIALS; import static com.google.cloud.spanner.connection.ConnectionProperties.ENDPOINT; +import static com.google.cloud.spanner.connection.ConnectionProperties.IS_EXPERIMENTAL_HOST; import static com.google.cloud.spanner.connection.ConnectionProperties.LENIENT; import static com.google.cloud.spanner.connection.ConnectionProperties.MAX_COMMIT_DELAY; import static com.google.cloud.spanner.connection.ConnectionProperties.MAX_PARTITIONED_PARALLELISM; @@ -221,6 +222,7 @@ public String[] getValidValues() { private static final LocalConnectionChecker LOCAL_CONNECTION_CHECKER = new LocalConnectionChecker(); static final boolean DEFAULT_USE_PLAIN_TEXT = false; + static final boolean DEFAULT_IS_EXPERIMENTAL_HOST = false; static final boolean DEFAULT_AUTOCOMMIT = true; static final boolean DEFAULT_READONLY = false; static final boolean DEFAULT_RETRY_ABORTS_INTERNALLY = true; @@ -260,6 +262,8 @@ public String[] getValidValues() { static final boolean DEFAULT_AUTO_BATCH_DML = false; static final long DEFAULT_AUTO_BATCH_DML_UPDATE_COUNT = 1L; static final boolean DEFAULT_AUTO_BATCH_DML_UPDATE_COUNT_VERIFICATION = true; + private static final String EXPERIMENTAL_HOST_PROJECT_ID = "default"; + private static final String DEFAULT_EXPERIMENTAL_HOST_INSTANCE_ID = "default"; private static final String PLAIN_TEXT_PROTOCOL = "http:"; private static final String HOST_PROTOCOL = "https:"; @@ -268,6 +272,8 @@ public String[] getValidValues() { private static final String DEFAULT_EMULATOR_HOST = "http://localhost:9010"; /** Use plain text is only for local testing purposes. */ static final String USE_PLAIN_TEXT_PROPERTY_NAME = "usePlainText"; + /** Connect to a Experimental Host * */ + static final String IS_EXPERIMENTAL_HOST_PROPERTY_NAME = "isExperimentalHost"; /** Client certificate path to establish mTLS */ static final String CLIENT_CERTIFICATE_PROPERTY_NAME = "clientCertificate"; /** Client key path to establish mTLS */ @@ -444,6 +450,10 @@ static boolean isEnableTransactionalConnectionStateForPostgreSQL() { USE_PLAIN_TEXT_PROPERTY_NAME, "Use a plain text communication channel (i.e. non-TLS) for communicating with the server (true/false). Set this value to true for communication with the Cloud Spanner emulator.", DEFAULT_USE_PLAIN_TEXT), + ConnectionProperty.createBooleanProperty( + IS_EXPERIMENTAL_HOST_PROPERTY_NAME, + "Set this value to true for communication with a Experimental Host.", + DEFAULT_IS_EXPERIMENTAL_HOST), ConnectionProperty.createStringProperty( CLIENT_CERTIFICATE_PROPERTY_NAME, "Specifies the file path to the client certificate required for establishing an mTLS connection."), @@ -857,10 +867,10 @@ public static Builder newBuilder() { private ConnectionOptions(Builder builder) { Matcher matcher; - boolean isExternalHost = false; + boolean isExperimentalHostPattern = false; if (builder.isValidExternalHostUri(builder.uri)) { matcher = Builder.EXTERNAL_HOST_PATTERN.matcher(builder.uri); - isExternalHost = true; + isExperimentalHostPattern = true; } else { matcher = Builder.SPANNER_URI_PATTERN.matcher(builder.uri); } @@ -939,7 +949,8 @@ && getInitialConnectionPropertyValue(OAUTH_TOKEN) == null this.credentials = new GoogleCredentials( new AccessToken(getInitialConnectionPropertyValue(OAUTH_TOKEN), null)); - } else if (isExternalHost && defaultExternalHostCredentials != null) { + } else if ((isExperimentalHostPattern || isExperimentalHost()) + && defaultExternalHostCredentials != null) { this.credentials = defaultExternalHostCredentials; } else if (getInitialConnectionPropertyValue(CREDENTIALS_PROVIDER) != null) { try { @@ -981,16 +992,19 @@ && getInitialConnectionPropertyValue(OAUTH_TOKEN) == null this.sessionPoolOptions = sessionPoolOptionsBuilder.build(); } else if (builder.sessionPoolOptions != null) { this.sessionPoolOptions = builder.sessionPoolOptions; + } else if (isExperimentalHostPattern || isExperimentalHost()) { + this.sessionPoolOptions = + SessionPoolOptions.newBuilder().setExperimentalHost().setAutoDetectDialect(true).build(); } else { this.sessionPoolOptions = SessionPoolOptions.newBuilder().setAutoDetectDialect(true).build(); } - String projectId = "default"; + String projectId = EXPERIMENTAL_HOST_PROJECT_ID; String instanceId = matcher.group(Builder.INSTANCE_GROUP); - if (!isExternalHost) { + if (!isExperimentalHost() && !isExperimentalHostPattern) { projectId = matcher.group(Builder.PROJECT_GROUP); } else if (instanceId == null) { - instanceId = "default"; + instanceId = DEFAULT_EXPERIMENTAL_HOST_INSTANCE_ID; } if (Builder.DEFAULT_PROJECT_ID_PLACEHOLDER.equalsIgnoreCase(projectId)) { projectId = getDefaultProjectId(this.credentials); @@ -1311,6 +1325,10 @@ boolean isUsePlainText() { || getInitialConnectionPropertyValue(USE_PLAIN_TEXT); } + boolean isExperimentalHost() { + return getInitialConnectionPropertyValue(IS_EXPERIMENTAL_HOST); + } + String getClientCertificate() { return getInitialConnectionPropertyValue(CLIENT_CERTIFICATE); } diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java index f65dc533570..d9f2d5562b8 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java @@ -47,6 +47,7 @@ import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_ENABLE_END_TO_END_TRACING; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_ENABLE_EXTENDED_TRACING; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_ENDPOINT; +import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_IS_EXPERIMENTAL_HOST; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_KEEP_TRANSACTION_ALIVE; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_LENIENT; import static com.google.cloud.spanner.connection.ConnectionOptions.DEFAULT_MAX_PARTITIONED_PARALLELISM; @@ -76,6 +77,7 @@ import static com.google.cloud.spanner.connection.ConnectionOptions.ENABLE_EXTENDED_TRACING_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.ENCODED_CREDENTIALS_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.ENDPOINT_PROPERTY_NAME; +import static com.google.cloud.spanner.connection.ConnectionOptions.IS_EXPERIMENTAL_HOST_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.KEEP_TRANSACTION_ALIVE_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.LENIENT_PROPERTY_NAME; import static com.google.cloud.spanner.connection.ConnectionOptions.MAX_PARTITIONED_PARALLELISM_PROPERTY_NAME; @@ -197,7 +199,14 @@ public class ConnectionProperties { BOOLEANS, BooleanConverter.INSTANCE, Context.STARTUP); - + static final ConnectionProperty IS_EXPERIMENTAL_HOST = + create( + IS_EXPERIMENTAL_HOST_PROPERTY_NAME, + "Set this value to true for communication with a Experimental Host.", + DEFAULT_IS_EXPERIMENTAL_HOST, + BOOLEANS, + BooleanConverter.INSTANCE, + Context.STARTUP); static final ConnectionProperty CLIENT_CERTIFICATE = create( CLIENT_CERTIFICATE_PROPERTY_NAME, diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 72bbdf82eae..3d58e162d71 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -16,7 +16,6 @@ package com.google.cloud.spanner; -import static com.google.cloud.spanner.SpannerOptions.CLOUD_SPANNER_HOST_PATTERN; import static com.google.common.truth.Truth.assertThat; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -1167,10 +1166,17 @@ public void checkGlobalOpenTelemetryWhenNotInjected() { } @Test - public void testCloudSpannerHostPattern() { - assertTrue(CLOUD_SPANNER_HOST_PATTERN.matcher("https://spanner.googleapis.com").matches()); - assertTrue( - CLOUD_SPANNER_HOST_PATTERN.matcher("https://product-area.googleapis.com:443").matches()); - assertFalse(CLOUD_SPANNER_HOST_PATTERN.matcher("https://some-company.com:443").matches()); + public void testExperimentalHostOptions() { + SpannerOptions options = + SpannerOptions.newBuilder() + .setExperimentalHost("localhost:8080") + .setCredentials(NoCredentials.getInstance()) + .build(); + assertEquals("default", options.getProjectId()); + assertEquals("localhost:8080", options.getHost()); + assertEquals(0, options.getSessionPoolOptions().getMinSessions()); + assertTrue(options.getSessionPoolOptions().getUseMultiplexedSession()); + assertTrue(options.getSessionPoolOptions().getUseMultiplexedSessionForRW()); + assertTrue(options.getSessionPoolOptions().getUseMultiplexedSessionPartitionedOps()); } } diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java index 76771ffcc9b..36ca4c124f5 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/ConnectionOptionsTest.java @@ -1270,4 +1270,39 @@ public void testBuildWithValidURIWithPrefixSpanner() { assertThat(options.isAutocommit()).isEqualTo(false); assertThat(options.isReadOnly()).isEqualTo(true); } + + @Test + public void testExperimentalHost() { + ConnectionOptions.Builder builderWithoutExperimentalHostParam = ConnectionOptions.newBuilder(); + builderWithoutExperimentalHostParam.setUri( + "spanner://localhost:15000/instances/default/databases/singers-db;usePlainText=true"); + ConnectionOptions optionsWithoutExperimentalHostParam = + builderWithoutExperimentalHostParam.build(); + assertFalse(optionsWithoutExperimentalHostParam.isExperimentalHost()); + assertEquals(0, optionsWithoutExperimentalHostParam.getSessionPoolOptions().getMinSessions()); + assertTrue( + optionsWithoutExperimentalHostParam.getSessionPoolOptions().getUseMultiplexedSession()); + assertTrue( + optionsWithoutExperimentalHostParam + .getSessionPoolOptions() + .getUseMultiplexedSessionForRW()); + assertTrue( + optionsWithoutExperimentalHostParam + .getSessionPoolOptions() + .getUseMultiplexedSessionPartitionedOps()); + + ConnectionOptions.Builder builderWithExperimentalHostParam = ConnectionOptions.newBuilder(); + builderWithExperimentalHostParam.setUri( + "spanner://localhost:15000/projects/default/instances/default/databases/singers-db;usePlainText=true;isExperimentalHost=true"); + ConnectionOptions optionsWithExperimentalHostParam = builderWithExperimentalHostParam.build(); + assertTrue(optionsWithExperimentalHostParam.isExperimentalHost()); + assertEquals(0, optionsWithExperimentalHostParam.getSessionPoolOptions().getMinSessions()); + assertTrue(optionsWithExperimentalHostParam.getSessionPoolOptions().getUseMultiplexedSession()); + assertTrue( + optionsWithExperimentalHostParam.getSessionPoolOptions().getUseMultiplexedSessionForRW()); + assertTrue( + optionsWithExperimentalHostParam + .getSessionPoolOptions() + .getUseMultiplexedSessionPartitionedOps()); + } } From 7f9c135c9009c807095e03ffaaecb8cf575f2743 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Wed, 5 Mar 2025 08:48:49 +0000 Subject: [PATCH 2/7] pr link added to experimentalApi annotations --- .../main/java/com/google/cloud/spanner/SessionPoolOptions.java | 2 +- .../src/main/java/com/google/cloud/spanner/SpannerOptions.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index b45569c9325..0d8741421b5 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -826,7 +826,7 @@ public Builder setWarnAndCloseIfInactiveTransactions() { return this; } - @ExperimentalApi("") + @ExperimentalApi("https://github.com/googleapis/java-spanner/pull/3676") public Builder setExperimentalHost() { this.isExperimentalHost = true; return this; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index dcddbb69212..b018a2f47f8 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -1485,7 +1485,7 @@ public Builder setHost(String host) { return this; } - @ExperimentalApi("") + @ExperimentalApi("https://github.com/googleapis/java-spanner/pull/3676") public Builder setExperimentalHost(String host) { super.setHost(host); super.setProjectId(EXPERIMENTAL_HOST_PROJECT_ID); From 80c8faa463f31f89e95412761d6791682d5f3648 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Thu, 6 Mar 2025 05:23:42 +0000 Subject: [PATCH 3/7] max sessions set to 0 for exp host --- .../java/com/google/cloud/spanner/SessionPoolOptions.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 0d8741421b5..32d5776059b 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -35,7 +35,7 @@ public class SessionPoolOptions { private static final int DEFAULT_MAX_SESSIONS = 400; private static final int DEFAULT_MIN_SESSIONS = 100; private static final int DEFAULT_INC_STEP = 25; - private static final int EXPERIMENTAL_HOST_MIN_SESSIONS = 0; + private static final int EXPERIMENTAL_HOST_REGULAR_SESSIONS = 0; private static final ActionOnExhaustion DEFAULT_ACTION = ActionOnExhaustion.BLOCK; private final int minSessions; private final int maxSessions; @@ -93,9 +93,10 @@ private SessionPoolOptions(Builder builder) { // maxSessions value is less than the default for minSessions. this.minSessions = builder.isExperimentalHost - ? EXPERIMENTAL_HOST_MIN_SESSIONS + ? EXPERIMENTAL_HOST_REGULAR_SESSIONS : Math.min(builder.minSessions, builder.maxSessions); - this.maxSessions = builder.maxSessions; + this.maxSessions = + builder.isExperimentalHost ? EXPERIMENTAL_HOST_REGULAR_SESSIONS : builder.maxSessions; this.incStep = builder.incStep; this.maxIdleSessions = builder.maxIdleSessions; this.writeSessionsFraction = builder.writeSessionsFraction; From 3a59e8d5fa2701345c45109851527d968e8fd1f8 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 10 Mar 2025 13:05:27 +0000 Subject: [PATCH 4/7] comments addressed --- .../clirr-ignored-differences.xml | 4 ++-- .../cloud/spanner/SessionPoolOptions.java | 14 +++++--------- .../google/cloud/spanner/SpannerOptions.java | 18 +++++++++--------- .../spanner/connection/ConnectionOptions.java | 18 +++++++++--------- .../cloud/spanner/SpannerOptionsTest.java | 2 +- 5 files changed, 26 insertions(+), 30 deletions(-) diff --git a/google-cloud-spanner/clirr-ignored-differences.xml b/google-cloud-spanner/clirr-ignored-differences.xml index 5c103beca6f..b568708d4f8 100644 --- a/google-cloud-spanner/clirr-ignored-differences.xml +++ b/google-cloud-spanner/clirr-ignored-differences.xml @@ -822,11 +822,11 @@ java.lang.Object runTransaction(com.google.cloud.spanner.connection.Connection$TransactionCallable) - + 7012 com/google/cloud/spanner/SpannerOptions$SpannerEnvironment - com.google.auth.oauth2.GoogleCredentials getDefaultExternalHostCredentials() + com.google.auth.oauth2.GoogleCredentials getDefaultExperimentalHostCredentials() diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java index 32d5776059b..3080290f720 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SessionPoolOptions.java @@ -24,7 +24,6 @@ import com.google.cloud.spanner.SessionPool.Position; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import io.grpc.ExperimentalApi; import java.time.Duration; import java.util.Locale; import java.util.Objects; @@ -121,8 +120,7 @@ private SessionPoolOptions(Builder builder) { Boolean useMultiplexedSessionFromEnvVariable = getUseMultiplexedSessionFromEnvVariable(); this.useMultiplexedSession = builder.isExperimentalHost - ? true - : ((useMultiplexedSessionFromEnvVariable != null) + || ((useMultiplexedSessionFromEnvVariable != null) ? useMultiplexedSessionFromEnvVariable : builder.useMultiplexedSession); // useMultiplexedSessionForRW priority => Environment var > private setter > client default @@ -130,8 +128,7 @@ private SessionPoolOptions(Builder builder) { getUseMultiplexedSessionForRWFromEnvVariable(); this.useMultiplexedSessionForRW = builder.isExperimentalHost - ? true - : ((useMultiplexedSessionForRWFromEnvVariable != null) + || ((useMultiplexedSessionForRWFromEnvVariable != null) ? useMultiplexedSessionForRWFromEnvVariable : builder.useMultiplexedSessionForRW); // useMultiplexedSessionPartitionedOps priority => Environment var > private setter > client @@ -140,13 +137,12 @@ private SessionPoolOptions(Builder builder) { getUseMultiplexedSessionFromEnvVariablePartitionedOps(); this.useMultiplexedSessionForPartitionedOps = builder.isExperimentalHost - ? true - : ((useMultiplexedSessionFromEnvVariablePartitionedOps != null) + || ((useMultiplexedSessionFromEnvVariablePartitionedOps != null) ? useMultiplexedSessionFromEnvVariablePartitionedOps : builder.useMultiplexedSessionPartitionedOps); this.multiplexedSessionMaintenanceDuration = builder.multiplexedSessionMaintenanceDuration; this.skipVerifyingBeginTransactionForMuxRW = - builder.isExperimentalHost ? true : builder.skipVerifyingBeginTransactionForMuxRW; + builder.isExperimentalHost || builder.skipVerifyingBeginTransactionForMuxRW; } @Override @@ -827,7 +823,7 @@ public Builder setWarnAndCloseIfInactiveTransactions() { return this; } - @ExperimentalApi("https://github.com/googleapis/java-spanner/pull/3676") + @InternalApi public Builder setExperimentalHost() { this.isExperimentalHost = true; return this; diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java index b018a2f47f8..412bbbb151c 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java @@ -852,13 +852,13 @@ default String getMonitoringHost() { return null; } - default GoogleCredentials getDefaultExternalHostCredentials() { + default GoogleCredentials getDefaultExperimentalHostCredentials() { return null; } } - static final String DEFAULT_SPANNER_EXTERNAL_HOST_CREDENTIALS = - "SPANNER_EXTERNAL_HOST_AUTH_TOKEN"; + static final String DEFAULT_SPANNER_EXPERIMENTAL_HOST_CREDENTIALS = + "SPANNER_EXPERIMENTAL_HOST_AUTH_TOKEN"; /** * Default implementation of {@link SpannerEnvironment}. Reads all configuration from environment @@ -917,8 +917,8 @@ public String getMonitoringHost() { } @Override - public GoogleCredentials getDefaultExternalHostCredentials() { - return getOAuthTokenFromFile(System.getenv(DEFAULT_SPANNER_EXTERNAL_HOST_CREDENTIALS)); + public GoogleCredentials getDefaultExperimentalHostCredentials() { + return getOAuthTokenFromFile(System.getenv(DEFAULT_SPANNER_EXPERIMENTAL_HOST_CREDENTIALS)); } } @@ -1532,7 +1532,7 @@ public Builder setEmulatorHost(String emulatorHost) { /** * Configures mTLS authentication using the provided client certificate and key files. mTLS is - * only supported for external spanner hosts. + * only supported for experimental spanner hosts. * * @param clientCertificate Path to the client certificate file. * @param clientCertificateKey Path to the client private key file. @@ -1660,7 +1660,7 @@ public SpannerOptions build() { // As we are using plain text, we should never send any credentials. this.setCredentials(NoCredentials.getInstance()); } else if (isExperimentalHost && credentials == null) { - credentials = environment.getDefaultExternalHostCredentials(); + credentials = environment.getDefaultExperimentalHostCredentials(); } if (this.numChannels == null) { this.numChannels = @@ -1702,8 +1702,8 @@ public static void useDefaultEnvironment() { } @InternalApi - public static GoogleCredentials getDefaultExternalHostCredentialsFromSysEnv() { - return getOAuthTokenFromFile(System.getenv(DEFAULT_SPANNER_EXTERNAL_HOST_CREDENTIALS)); + public static GoogleCredentials getDefaultExperimentalCredentialsFromSysEnv() { + return getOAuthTokenFromFile(System.getenv(DEFAULT_SPANNER_EXPERIMENTAL_HOST_CREDENTIALS)); } private static @Nullable GoogleCredentials getOAuthTokenFromFile(@Nullable String file) { diff --git a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java index 71155759058..1353a32a6a7 100644 --- a/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java +++ b/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java @@ -452,7 +452,7 @@ static boolean isEnableTransactionalConnectionStateForPostgreSQL() { DEFAULT_USE_PLAIN_TEXT), ConnectionProperty.createBooleanProperty( IS_EXPERIMENTAL_HOST_PROPERTY_NAME, - "Set this value to true for communication with a Experimental Host.", + "Set this value to true for communication with an Experimental Host.", DEFAULT_IS_EXPERIMENTAL_HOST), ConnectionProperty.createStringProperty( CLIENT_CERTIFICATE_PROPERTY_NAME, @@ -674,7 +674,7 @@ private boolean isValidUri(String uri) { return SPANNER_URI_PATTERN.matcher(uri).matches(); } - private boolean isValidExternalHostUri(String uri) { + private boolean isValidExperimentalHostUri(String uri) { return EXTERNAL_HOST_PATTERN.matcher(uri).matches(); } @@ -735,7 +735,7 @@ private boolean isValidExternalHostUri(String uri) { * @return this builder */ public Builder setUri(String uri) { - if (!isValidExternalHostUri(uri)) { + if (!isValidExperimentalHostUri(uri)) { Preconditions.checkArgument( isValidUri(uri), "The specified URI is not a valid Cloud Spanner connection URI. Please specify a URI in the format \"cloudspanner:[//host[:port]]/projects/project-id[/instances/instance-id[/databases/database-name]][\\?property-name=property-value[;property-name=property-value]*]?\""); @@ -868,7 +868,7 @@ public static Builder newBuilder() { private ConnectionOptions(Builder builder) { Matcher matcher; boolean isExperimentalHostPattern = false; - if (builder.isValidExternalHostUri(builder.uri)) { + if (builder.isValidExperimentalHostUri(builder.uri)) { matcher = Builder.EXTERNAL_HOST_PATTERN.matcher(builder.uri); isExperimentalHostPattern = true; } else { @@ -933,8 +933,8 @@ private ConnectionOptions(Builder builder) { getInitialConnectionPropertyValue(AUTO_CONFIG_EMULATOR), usePlainText, System.getenv()); - GoogleCredentials defaultExternalHostCredentials = - SpannerOptions.getDefaultExternalHostCredentialsFromSysEnv(); + GoogleCredentials defaultExperimentalHostCredentials = + SpannerOptions.getDefaultExperimentalCredentialsFromSysEnv(); // Using credentials on a plain text connection is not allowed, so if the user has not specified // any credentials and is using a plain text connection, we should not try to get the // credentials from the environment, but default to NoCredentials. @@ -950,8 +950,8 @@ && getInitialConnectionPropertyValue(OAUTH_TOKEN) == null new GoogleCredentials( new AccessToken(getInitialConnectionPropertyValue(OAUTH_TOKEN), null)); } else if ((isExperimentalHostPattern || isExperimentalHost()) - && defaultExternalHostCredentials != null) { - this.credentials = defaultExternalHostCredentials; + && defaultExperimentalHostCredentials != null) { + this.credentials = defaultExperimentalHostCredentials; } else if (getInitialConnectionPropertyValue(CREDENTIALS_PROVIDER) != null) { try { this.credentials = getInitialConnectionPropertyValue(CREDENTIALS_PROVIDER).getCredentials(); @@ -1003,7 +1003,7 @@ && getInitialConnectionPropertyValue(OAUTH_TOKEN) == null String instanceId = matcher.group(Builder.INSTANCE_GROUP); if (!isExperimentalHost() && !isExperimentalHostPattern) { projectId = matcher.group(Builder.PROJECT_GROUP); - } else if (instanceId == null) { + } else if (instanceId == null && isExperimentalHost()) { instanceId = DEFAULT_EXPERIMENTAL_HOST_INSTANCE_ID; } if (Builder.DEFAULT_PROJECT_ID_PLACEHOLDER.equalsIgnoreCase(projectId)) { diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java index 3d58e162d71..285097d4af7 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java @@ -1173,8 +1173,8 @@ public void testExperimentalHostOptions() { .setCredentials(NoCredentials.getInstance()) .build(); assertEquals("default", options.getProjectId()); - assertEquals("localhost:8080", options.getHost()); assertEquals(0, options.getSessionPoolOptions().getMinSessions()); + assertEquals(0, options.getSessionPoolOptions().getMaxSessions()); assertTrue(options.getSessionPoolOptions().getUseMultiplexedSession()); assertTrue(options.getSessionPoolOptions().getUseMultiplexedSessionForRW()); assertTrue(options.getSessionPoolOptions().getUseMultiplexedSessionPartitionedOps()); From b37a43652e2d0aaa331a4576f574b5e9e8ecd195 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 10 Mar 2025 14:54:20 +0000 Subject: [PATCH 5/7] unit test added --- .../ExperimentalHostMockServerTest.java | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java new file mode 100644 index 00000000000..5f7123aaed2 --- /dev/null +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java @@ -0,0 +1,89 @@ +/* + * Copyright 2023 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 org.junit.Assert.assertEquals; + +import com.google.cloud.NoCredentials; +import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; +import com.google.protobuf.ListValue; +import com.google.protobuf.Value; +import com.google.spanner.v1.ResultSetMetadata; +import com.google.spanner.v1.StructType; +import com.google.spanner.v1.TypeCode; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for samples that use an in-mem mock server instead of running on real Cloud Spanner. */ +@RunWith(JUnit4.class) +public class ExperimentalHostMockServerTest extends AbstractMockServerTest { + + private static final String SQL_QUERY = "SELECT * FROM Singers"; + + private static final ResultSetMetadata SINGERS_METADATA = + ResultSetMetadata.newBuilder() + .setRowType( + StructType.newBuilder() + .addFields( + StructType.Field.newBuilder() + .setName("FirstName") + .setType( + com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.STRING))) + .addFields( + StructType.Field.newBuilder() + .setName("LastName") + .setType( + com.google.spanner.v1.Type.newBuilder().setCode(TypeCode.STRING))) + .build()) + .build(); + + private static final com.google.spanner.v1.ResultSet SINGERS_RESULT_SET = + com.google.spanner.v1.ResultSet.newBuilder() + .setMetadata(SINGERS_METADATA) + .addRows( + ListValue.newBuilder() + .addValues(Value.newBuilder().setStringValue("Jane")) + .addValues(Value.newBuilder().setStringValue("Doe")) + .build()) + .build(); + + @Test + public void testExperimentalHostPreventsBatchCreateSessions() { + mockSpanner.putStatementResult( + StatementResult.query(Statement.of(SQL_QUERY), SINGERS_RESULT_SET)); + + SpannerOptions options = + SpannerOptions.newBuilder() + .setProjectId("p") + .setCredentials(NoCredentials.getInstance()) + .setExperimentalHost(null) + .setChannelProvider(channelProvider) + .build(); + + try (Spanner spanner = options.getService()) { + DatabaseClient dbClient = spanner.getDatabaseClient(DatabaseId.of("p", "i", "d")); + + // Perform an operation to trigger session creation + ResultSet resultSet = dbClient.singleUse().executeQuery(Statement.of(SQL_QUERY)); + while (resultSet.next()) {} + + // In case BatchCreateSessions is called getSessions size is 100 + assertEquals(0, (mockSpanner.getSessions()).size()); + } + } +} From cc6d70549c9c2bfec542890745f97be252efffb4 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 10 Mar 2025 14:57:29 +0000 Subject: [PATCH 6/7] removed comment --- .../com/google/cloud/spanner/ExperimentalHostMockServerTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java index 5f7123aaed2..345a6dc771f 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java @@ -29,7 +29,6 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; -/** Tests for samples that use an in-mem mock server instead of running on real Cloud Spanner. */ @RunWith(JUnit4.class) public class ExperimentalHostMockServerTest extends AbstractMockServerTest { From 695013d8fd3b2dae77218e954016c9e774397c65 Mon Sep 17 00:00:00 2001 From: sagnghos Date: Mon, 10 Mar 2025 15:19:24 +0000 Subject: [PATCH 7/7] changed assert statement --- .../cloud/spanner/ExperimentalHostMockServerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java index 345a6dc771f..423c3337ab8 100644 --- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java +++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/ExperimentalHostMockServerTest.java @@ -16,12 +16,13 @@ package com.google.cloud.spanner; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import com.google.cloud.NoCredentials; import com.google.cloud.spanner.MockSpannerServiceImpl.StatementResult; import com.google.protobuf.ListValue; import com.google.protobuf.Value; +import com.google.spanner.v1.BatchCreateSessionsRequest; import com.google.spanner.v1.ResultSetMetadata; import com.google.spanner.v1.StructType; import com.google.spanner.v1.TypeCode; @@ -81,8 +82,7 @@ public void testExperimentalHostPreventsBatchCreateSessions() { ResultSet resultSet = dbClient.singleUse().executeQuery(Statement.of(SQL_QUERY)); while (resultSet.next()) {} - // In case BatchCreateSessions is called getSessions size is 100 - assertEquals(0, (mockSpanner.getSessions()).size()); + assertFalse(mockSpanner.getRequestTypes().contains(BatchCreateSessionsRequest.class)); } } }