labelValues) {
this.options = options;
@@ -2108,6 +2138,7 @@ private SessionPool(
this.executor = executor;
this.sessionClient = sessionClient;
this.clock = clock;
+ this.initialReleasePosition = initialReleasePosition;
this.poolMaintainer = new PoolMaintainer();
this.initMetricsCollection(metricRegistry, labelValues);
this.waitOnMinSessionsLatch =
@@ -2233,7 +2264,7 @@ private void handleException(SpannerException e, PooledSession session) {
if (isSessionNotFound(e)) {
invalidateSession(session);
} else {
- releaseSession(session, Position.FIRST);
+ releaseSession(session, false);
}
}
@@ -2396,26 +2427,38 @@ private void maybeCreateSession() {
}
}
}
+
/** Releases a session back to the pool. This might cause one of the waiters to be unblocked. */
- private void releaseSession(PooledSession session, Position position) {
+ private void releaseSession(PooledSession session, boolean isNewSession) {
Preconditions.checkNotNull(session);
synchronized (lock) {
if (closureFuture != null) {
return;
}
if (waiters.size() == 0) {
- // No pending waiters
- switch (position) {
- case RANDOM:
- if (!sessions.isEmpty()) {
- int pos = random.nextInt(sessions.size() + 1);
- sessions.add(pos, session);
- break;
- }
- // fallthrough
- case FIRST:
- default:
- sessions.addFirst(session);
+ // There are no pending waiters.
+ // Add to a random position if the head of the session pool already contains many sessions
+ // with the same channel as this one.
+ if (session.releaseToPosition == Position.FIRST && isUnbalanced(session)) {
+ session.releaseToPosition = Position.RANDOM;
+ } else if (session.releaseToPosition == Position.RANDOM
+ && !isNewSession
+ && checkedOutSessions.size() <= 2) {
+ // Do not randomize if there are few other sessions checked out and this session has been
+ // used. This ensures that this session will be re-used for the next transaction, which is
+ // more efficient.
+ session.releaseToPosition = Position.FIRST;
+ }
+ if (session.releaseToPosition == Position.RANDOM && !sessions.isEmpty()) {
+ // A session should only be added at a random position the first time it is added to
+ // the pool or if the pool was deemed unbalanced. All following releases into the pool
+ // should normally happen at the front of the pool (unless the pool is again deemed to be
+ // unbalanced).
+ session.releaseToPosition = Position.FIRST;
+ int pos = random.nextInt(sessions.size() + 1);
+ sessions.add(pos, session);
+ } else {
+ sessions.addFirst(session);
}
} else {
waiters.poll().put(session);
@@ -2423,6 +2466,89 @@ private void releaseSession(PooledSession session, Position position) {
}
}
+ private boolean isUnbalanced(PooledSession session) {
+ int channel = session.getChannel();
+ int numChannels = sessionClient.getSpanner().getOptions().getNumChannels();
+ return isUnbalanced(channel, this.sessions, this.checkedOutSessions, numChannels);
+ }
+
+ /**
+ * Returns true if the given list of sessions is considered unbalanced when compared to the
+ * sessionChannel that is about to be added to the pool.
+ *
+ * The method returns true if all the following is true:
+ *
+ *
+ * - The list of sessions is not empty.
+ *
- The number of checked out sessions is > 2.
+ *
- The number of channels being used by the pool is > 1.
+ *
- And at least one of the following is true:
+ *
+ * - The first numChannels sessions in the list of sessions contains more than 2
+ * sessions that use the same channel as the one being added.
+ *
- The list of currently checked out sessions contains more than 2 times the the
+ * number of sessions with the same channel as the one being added than it should in
+ * order for it to be perfectly balanced. Perfectly balanced in this case means that
+ * the list should preferably contain size/numChannels sessions of each channel.
+ *
+ *
+ *
+ * @param channelOfSessionBeingAdded the channel number being used by the session that is about to
+ * be released into the pool
+ * @param sessions the list of all sessions in the pool
+ * @param checkedOutSessions the currently checked out sessions of the pool
+ * @param numChannels the number of channels in use
+ * @return true if the pool is considered unbalanced, and false otherwise
+ */
+ @VisibleForTesting
+ static boolean isUnbalanced(
+ int channelOfSessionBeingAdded,
+ List sessions,
+ Set checkedOutSessions,
+ int numChannels) {
+ // Do not re-balance the pool if the number of checked out sessions is low, as it is
+ // better to re-use sessions as much as possible in a low-QPS scenario.
+ if (sessions.isEmpty() || checkedOutSessions.size() <= 2) {
+ return false;
+ }
+ if (numChannels == 1) {
+ return false;
+ }
+
+ // Ideally, the first numChannels sessions in the pool should contain exactly one session for
+ // each channel.
+ // Check if the first numChannels sessions at the head of the pool already contain more than 2
+ // sessions that use the same channel as this one. If so, we re-balance.
+ // We also re-balance the pool in the specific case that the pool uses 2 channels and the first
+ // two sessions use those two channels.
+ int maxSessionsAtHeadOfPool = Math.min(numChannels, 3);
+ int count = 0;
+ for (int i = 0; i < Math.min(numChannels, sessions.size()); i++) {
+ PooledSession otherSession = sessions.get(i);
+ if (channelOfSessionBeingAdded == otherSession.getChannel()) {
+ count++;
+ if (count >= maxSessionsAtHeadOfPool) {
+ return true;
+ }
+ }
+ }
+ // Ideally, the use of a channel in the checked out sessions is exactly
+ // numCheckedOut / numChannels
+ // We check whether we are more than a factor two away from that perfect distribution.
+ // If we are, then we re-balance.
+ count = 0;
+ int checkedOutThreshold = Math.max(2, 2 * checkedOutSessions.size() / numChannels);
+ for (PooledSessionFuture otherSession : checkedOutSessions) {
+ if (otherSession.isDone() && channelOfSessionBeingAdded == otherSession.get().getChannel()) {
+ count++;
+ if (count > checkedOutThreshold) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private void handleCreateSessionsFailure(SpannerException e, int count) {
synchronized (lock) {
for (int i = 0; i < count; i++) {
@@ -2622,7 +2748,7 @@ public void onSessionReady(SessionImpl session) {
// Release the session to a random position in the pool to prevent the case that a batch
// of sessions that are affiliated with the same channel are all placed sequentially in
// the pool.
- releaseSession(pooledSession, Position.RANDOM);
+ releaseSession(pooledSession, true);
}
}
}
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 a5753eca8ca..47397100acd 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
@@ -36,6 +36,7 @@
import com.google.cloud.spanner.SpannerOptions;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import com.google.spanner.v1.ExecuteSqlRequest.QueryOptions;
import java.io.IOException;
@@ -205,8 +206,14 @@ public String[] getValidValues() {
public static final String CREDENTIALS_PROPERTY_NAME = "credentials";
/** Name of the 'encodedCredentials' connection property. */
public static final String ENCODED_CREDENTIALS_PROPERTY_NAME = "encodedCredentials";
+
+ public static final String ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY =
+ "ENABLE_ENCODED_CREDENTIALS";
/** Name of the 'credentialsProvider' connection property. */
public static final String CREDENTIALS_PROVIDER_PROPERTY_NAME = "credentialsProvider";
+
+ public static final String ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY =
+ "ENABLE_CREDENTIALS_PROVIDER";
/**
* OAuth token to use for authentication. Cannot be used in combination with a credentials file.
*/
@@ -219,6 +226,8 @@ public String[] getValidValues() {
public static final String NUM_CHANNELS_PROPERTY_NAME = "numChannels";
/** Name of the 'channelProvider' connection property. */
public static final String CHANNEL_PROVIDER_PROPERTY_NAME = "channelProvider";
+
+ public static final String ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY = "ENABLE_CHANNEL_PROVIDER";
/** Custom user agent string is only for other Google libraries. */
private static final String USER_AGENT_PROPERTY_NAME = "userAgent";
/** Query optimizer version to use for a connection. */
@@ -248,6 +257,19 @@ public String[] getValidValues() {
public static final String MAX_PARTITIONED_PARALLELISM_PROPERTY_NAME =
"maxPartitionedParallelism";
+ private static final String GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE =
+ "%s can only be used if the system property %s has been set to true. "
+ + "Start the application with the JVM command line option -D%s=true";
+
+ private static String generateGuardedConnectionPropertyError(
+ String systemPropertyName, String connectionPropertyName) {
+ return String.format(
+ GUARDED_CONNECTION_PROPERTY_ERROR_MESSAGE,
+ connectionPropertyName,
+ systemPropertyName,
+ systemPropertyName);
+ }
+
/** All valid connection properties. */
public static final Set VALID_PROPERTIES =
Collections.unmodifiableSet(
@@ -846,40 +868,64 @@ static boolean parseRetryAbortsInternally(String uri) {
@VisibleForTesting
static @Nullable String parseEncodedCredentials(String uri) {
- return parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
+ String encodedCredentials = parseUriProperty(uri, ENCODED_CREDENTIALS_PROPERTY_NAME);
+ checkGuardedProperty(
+ encodedCredentials,
+ ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
+ ENCODED_CREDENTIALS_PROPERTY_NAME);
+ return encodedCredentials;
}
@VisibleForTesting
static @Nullable CredentialsProvider parseCredentialsProvider(String uri) {
- String name = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
- if (name != null) {
+ String credentialsProviderName = parseUriProperty(uri, CREDENTIALS_PROVIDER_PROPERTY_NAME);
+ checkGuardedProperty(
+ credentialsProviderName,
+ ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
+ CREDENTIALS_PROVIDER_PROPERTY_NAME);
+ if (!Strings.isNullOrEmpty(credentialsProviderName)) {
try {
Class clazz =
- (Class) Class.forName(name);
+ (Class) Class.forName(credentialsProviderName);
Constructor constructor = clazz.getDeclaredConstructor();
return constructor.newInstance();
} catch (ClassNotFoundException classNotFoundException) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
- "Unknown or invalid CredentialsProvider class name: " + name,
+ "Unknown or invalid CredentialsProvider class name: " + credentialsProviderName,
classNotFoundException);
} catch (NoSuchMethodException noSuchMethodException) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
- "Credentials provider " + name + " does not have a public no-arg constructor.",
+ "Credentials provider "
+ + credentialsProviderName
+ + " does not have a public no-arg constructor.",
noSuchMethodException);
} catch (InvocationTargetException
| InstantiationException
| IllegalAccessException exception) {
throw SpannerExceptionFactory.newSpannerException(
ErrorCode.INVALID_ARGUMENT,
- "Failed to create an instance of " + name + ": " + exception.getMessage(),
+ "Failed to create an instance of "
+ + credentialsProviderName
+ + ": "
+ + exception.getMessage(),
exception);
}
}
return null;
}
+ private static void checkGuardedProperty(
+ String value, String systemPropertyName, String connectionPropertyName) {
+ if (!Strings.isNullOrEmpty(value)
+ && !Boolean.parseBoolean(System.getProperty(systemPropertyName))) {
+ throw SpannerExceptionFactory.newSpannerException(
+ ErrorCode.FAILED_PRECONDITION,
+ generateGuardedConnectionPropertyError(systemPropertyName, connectionPropertyName));
+ }
+ }
+
@VisibleForTesting
static @Nullable String parseOAuthToken(String uri) {
String value = parseUriProperty(uri, OAUTH_TOKEN_PROPERTY_NAME);
@@ -907,6 +953,8 @@ static String parseNumChannels(String uri) {
@VisibleForTesting
static String parseChannelProvider(String uri) {
String value = parseUriProperty(uri, CHANNEL_PROVIDER_PROPERTY_NAME);
+ checkGuardedProperty(
+ value, ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY, CHANNEL_PROVIDER_PROPERTY_NAME);
return value != null ? value : DEFAULT_CHANNEL_PROVIDER;
}
diff --git a/google-cloud-spanner/src/main/resources/META-INF/native-image/com.google.cloud.spanner/reflect-config.json b/google-cloud-spanner/src/main/resources/META-INF/native-image/com.google.cloud.spanner/reflect-config.json
new file mode 100644
index 00000000000..ddb6ab0a676
--- /dev/null
+++ b/google-cloud-spanner/src/main/resources/META-INF/native-image/com.google.cloud.spanner/reflect-config.json
@@ -0,0 +1,172 @@
+[ {
+ "name": "com.google.rpc.LocalizedMessage",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+},
+ {
+ "name": "com.google.rpc.LocalizedMessage$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.BadRequest",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.BadRequest$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.BadRequest$FieldViolation",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.Help",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.Help$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.Help$Link",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.QuotaFailure",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.QuotaFailure$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.QuotaFailure$Violation",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.RequestInfo",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.RequestInfo$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.ResourceInfo",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.ResourceInfo$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.RetryInfo",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.RetryInfo$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.ErrorInfo",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ },
+ {
+ "name": "com.google.rpc.ErrorInfo$Builder",
+ "queryAllDeclaredConstructors": true,
+ "queryAllPublicConstructors": true,
+ "queryAllDeclaredMethods": true,
+ "allPublicMethods": true,
+ "allDeclaredClasses": true,
+ "allPublicClasses": true
+ }
+]
\ No newline at end of file
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BaseSessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BaseSessionPoolTest.java
index 3a595358fea..7f8cf5cc1b0 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BaseSessionPoolTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/BaseSessionPoolTest.java
@@ -26,16 +26,21 @@
import com.google.api.core.ApiFutures;
import com.google.cloud.grpc.GrpcTransportOptions.ExecutorFactory;
import com.google.cloud.spanner.SessionPool.Clock;
+import com.google.cloud.spanner.spi.v1.SpannerRpc.Option;
import com.google.protobuf.Empty;
+import java.util.HashMap;
+import java.util.Map;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.ScheduledThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
import org.threeten.bp.Instant;
abstract class BaseSessionPoolTest {
ScheduledExecutorService mockExecutor;
int sessionIndex;
+ AtomicLong channelHint = new AtomicLong(0L);
final class TestExecutorFactory implements ExecutorFactory {
@@ -64,6 +69,9 @@ public void release(ScheduledExecutorService executor) {
@SuppressWarnings("unchecked")
SessionImpl mockSession() {
final SessionImpl session = mock(SessionImpl.class);
+ Map options = new HashMap<>();
+ options.put(Option.CHANNEL_HINT, channelHint.getAndIncrement());
+ when(session.getOptions()).thenReturn(options);
when(session.getName())
.thenReturn(
"projects/dummy/instances/dummy/database/dummy/sessions/session" + sessionIndex);
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java
index ab6a51c9260..2f7b14fdadf 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolMaintainerTest.java
@@ -26,6 +26,7 @@
import com.google.cloud.spanner.SessionClient.SessionConsumer;
import com.google.cloud.spanner.SessionPool.PooledSession;
import com.google.cloud.spanner.SessionPool.PooledSessionFuture;
+import com.google.cloud.spanner.SessionPool.Position;
import com.google.cloud.spanner.SessionPool.SessionConsumerImpl;
import java.util.ArrayList;
import java.util.HashMap;
@@ -58,6 +59,7 @@ public void setUp() {
initMocks(this);
when(client.getOptions()).thenReturn(spannerOptions);
when(client.getSessionClient(db)).thenReturn(sessionClient);
+ when(sessionClient.getSpanner()).thenReturn(client);
when(spannerOptions.getNumChannels()).thenReturn(4);
when(spannerOptions.getDatabaseRole()).thenReturn("role");
setupMockSessionCreation();
@@ -111,9 +113,11 @@ private SessionImpl setupMockSession(final SessionImpl session) {
}
private SessionPool createPool() throws Exception {
+ // Allow sessions to be added to the head of the pool in all cases in this test, as it is
+ // otherwise impossible to know which session exactly is getting pinged at what point in time.
SessionPool pool =
SessionPool.createPool(
- options, new TestExecutorFactory(), client.getSessionClient(db), clock);
+ options, new TestExecutorFactory(), client.getSessionClient(db), clock, Position.FIRST);
pool.idleSessionRemovedListener =
input -> {
idledSessions.add(input);
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolStressTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolStressTest.java
index d1aab02d32f..c9ba9b360af 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolStressTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolStressTest.java
@@ -25,6 +25,7 @@
import com.google.api.core.ApiFutures;
import com.google.cloud.spanner.SessionClient.SessionConsumer;
import com.google.cloud.spanner.SessionPool.PooledSessionFuture;
+import com.google.cloud.spanner.SessionPool.Position;
import com.google.cloud.spanner.SessionPool.SessionConsumerImpl;
import com.google.cloud.spanner.SessionPoolOptions.ActionOnInactiveTransaction;
import com.google.cloud.spanner.SessionPoolOptions.InactiveTransactionRemovalOptions;
@@ -39,6 +40,7 @@
import java.util.Map;
import java.util.Random;
import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -65,10 +67,10 @@ public class SessionPoolStressTest extends BaseSessionPoolTest {
SessionPool pool;
SessionPoolOptions options;
ExecutorService createExecutor = Executors.newSingleThreadExecutor();
- Object lock = new Object();
+ final Object lock = new Object();
Random random = new Random();
FakeClock clock = new FakeClock();
- Map sessions = new HashMap<>();
+ final Map sessions = new ConcurrentHashMap<>();
// Exception keeps track of where the session was closed at.
Map closedSessions = new HashMap<>();
Set expiredSessions = new HashSet<>();
@@ -92,6 +94,7 @@ private void setupSpanner(DatabaseId db) {
when(spannerOptions.getNumChannels()).thenReturn(4);
when(spannerOptions.getDatabaseRole()).thenReturn("role");
SessionClient sessionClient = mock(SessionClient.class);
+ when(sessionClient.getSpanner()).thenReturn(mockSpanner);
when(mockSpanner.getSessionClient(db)).thenReturn(sessionClient);
when(mockSpanner.getOptions()).thenReturn(spannerOptions);
doAnswer(
@@ -226,22 +229,26 @@ public void stressTest() throws Exception {
}
pool =
SessionPool.createPool(
- builder.build(), new TestExecutorFactory(), mockSpanner.getSessionClient(db), clock);
+ builder.build(),
+ new TestExecutorFactory(),
+ mockSpanner.getSessionClient(db),
+ clock,
+ Position.RANDOM);
pool.idleSessionRemovedListener =
pooled -> {
String name = pooled.getName();
- synchronized (lock) {
- sessions.remove(name);
- return null;
- }
+ // We do not take the test lock here, as we already hold the session pool lock. Taking the
+ // test lock as well here can cause a deadlock.
+ sessions.remove(name);
+ return null;
};
pool.longRunningSessionRemovedListener =
pooled -> {
String name = pooled.getName();
- synchronized (lock) {
- sessions.remove(name);
- return null;
- }
+ // We do not take the test lock here, as we already hold the session pool lock. Taking the
+ // test lock as well here can cause a deadlock.
+ sessions.remove(name);
+ return null;
};
for (int i = 0; i < concurrentThreads; i++) {
new Thread(
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java
index b20d5dd652b..8949ba6afa8 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolTest.java
@@ -52,6 +52,7 @@
import com.google.cloud.spanner.SessionPool.Clock;
import com.google.cloud.spanner.SessionPool.PooledSession;
import com.google.cloud.spanner.SessionPool.PooledSessionFuture;
+import com.google.cloud.spanner.SessionPool.Position;
import com.google.cloud.spanner.SessionPool.SessionConsumerImpl;
import com.google.cloud.spanner.SpannerImpl.ClosedException;
import com.google.cloud.spanner.TransactionRunner.TransactionCallable;
@@ -126,7 +127,7 @@ private SessionPool createPool() {
private SessionPool createPool(Clock clock) {
return SessionPool.createPool(
- options, new TestExecutorFactory(), client.getSessionClient(db), clock);
+ options, new TestExecutorFactory(), client.getSessionClient(db), clock, Position.RANDOM);
}
private SessionPool createPool(
@@ -137,6 +138,7 @@ private SessionPool createPool(
new TestExecutorFactory(),
client.getSessionClient(db),
clock,
+ Position.RANDOM,
metricRegistry,
labelValues);
}
@@ -146,6 +148,7 @@ public void setUp() {
initMocks(this);
when(client.getOptions()).thenReturn(spannerOptions);
when(client.getSessionClient(db)).thenReturn(sessionClient);
+ when(sessionClient.getSpanner()).thenReturn(client);
when(spannerOptions.getNumChannels()).thenReturn(4);
when(spannerOptions.getDatabaseRole()).thenReturn("role");
options =
@@ -204,13 +207,27 @@ public void sessionCreation() {
@Test
public void poolLifo() {
setupMockSessionCreation();
+ options =
+ options
+ .toBuilder()
+ .setMinSessions(2)
+ .setWaitForMinSessions(Duration.ofSeconds(10L))
+ .build();
pool = createPool();
+ pool.maybeWaitOnMinSessions();
Session session1 = pool.getSession().get();
Session session2 = pool.getSession().get();
assertThat(session1).isNotEqualTo(session2);
session2.close();
session1.close();
+
+ // Check the session out and back in once more to finalize their positions.
+ session1 = pool.getSession().get();
+ session2 = pool.getSession().get();
+ session2.close();
+ session1.close();
+
Session session3 = pool.getSession().get();
Session session4 = pool.getSession().get();
assertThat(session3).isEqualTo(session1);
@@ -1187,6 +1204,7 @@ public void testSessionNotFoundReadWriteTransaction() {
SpannerImpl spanner = mock(SpannerImpl.class);
SessionClient sessionClient = mock(SessionClient.class);
when(spanner.getSessionClient(db)).thenReturn(sessionClient);
+ when(sessionClient.getSpanner()).thenReturn(spanner);
doAnswer(
invocation -> {
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java
new file mode 100644
index 00000000000..5a9365eaed9
--- /dev/null
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/SessionPoolUnbalancedTest.java
@@ -0,0 +1,241 @@
+/*
+ * 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 com.google.cloud.spanner.SessionPool.isUnbalanced;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.cloud.spanner.SessionPool.PooledSession;
+import com.google.cloud.spanner.SessionPool.PooledSessionFuture;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class SessionPoolUnbalancedTest {
+
+ static PooledSession mockedSession(int channel) {
+ PooledSession session = mock(PooledSession.class);
+ when(session.getChannel()).thenReturn(channel);
+ return session;
+ }
+
+ static List mockedSessions(int... channels) {
+ return Arrays.stream(channels)
+ .mapToObj(SessionPoolUnbalancedTest::mockedSession)
+ .collect(Collectors.toList());
+ }
+
+ static PooledSessionFuture mockedCheckedOutSession(int channel) {
+ PooledSession session = mockedSession(channel);
+ PooledSessionFuture future = mock(PooledSessionFuture.class);
+ when(future.get()).thenReturn(session);
+ when(future.isDone()).thenReturn(true);
+ return future;
+ }
+
+ static Set mockedCheckedOutSessions(int... channels) {
+ return Arrays.stream(channels)
+ .mapToObj(SessionPoolUnbalancedTest::mockedCheckedOutSession)
+ .collect(Collectors.toSet());
+ }
+
+ @Test
+ public void testIsUnbalancedBasics() {
+ // An empty session pool is never unbalanced.
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1), 4));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1), 4));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1, 1), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1, 1), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(), mockedCheckedOutSessions(1, 1, 1, 1, 1), 4));
+
+ // A session pool that has 2 or fewer sessions checked out is never unbalanced.
+ // This prevents low-QPS scenarios from re-balancing the pool.
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1), mockedCheckedOutSessions(), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1), mockedCheckedOutSessions(), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1), mockedCheckedOutSessions(), 4));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1), mockedCheckedOutSessions(1), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1), mockedCheckedOutSessions(1), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1), mockedCheckedOutSessions(1), 4));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1), mockedCheckedOutSessions(1, 1), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1), mockedCheckedOutSessions(1, 1), 2));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1), mockedCheckedOutSessions(1, 1), 4));
+
+ // A session pool that uses only 1 channel is never unbalanced.
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1), mockedCheckedOutSessions(), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1), mockedCheckedOutSessions(), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1), mockedCheckedOutSessions(), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1, 1), mockedCheckedOutSessions(), 1));
+ assertFalse(isUnbalanced(1, mockedSessions(1, 1, 1), mockedCheckedOutSessions(1, 1, 1), 1));
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 1, 1, 1), mockedCheckedOutSessions(1, 1, 1, 1), 1));
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 1, 1, 1, 1), mockedCheckedOutSessions(1, 1, 1, 1, 1), 1));
+ assertFalse(
+ isUnbalanced(
+ 1, mockedSessions(1, 1, 1, 1, 1, 1), mockedCheckedOutSessions(1, 1, 1, 1, 1, 1), 1));
+ }
+
+ @Test
+ public void testIsUnbalanced_returnsFalseForBalancedPool() {
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertFalse(
+ isUnbalanced(2, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertFalse(
+ isUnbalanced(3, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertFalse(
+ isUnbalanced(4, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(4, 3, 2, 1), 4));
+ assertFalse(
+ isUnbalanced(2, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(4, 3, 2, 1), 4));
+ assertFalse(
+ isUnbalanced(3, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(4, 3, 2, 1), 4));
+ assertFalse(
+ isUnbalanced(4, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(4, 3, 2, 1), 4));
+
+ assertFalse(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 2, 3, 4, 1, 2, 3, 4),
+ mockedCheckedOutSessions(1, 2, 3, 4, 1, 2, 3, 4),
+ 4));
+
+ // We only check the first numChannels sessions that are in the pool, so the fact that the end
+ // of the pool is unbalanced is not a reason to re-balance.
+ assertFalse(
+ isUnbalanced(
+ 1, mockedSessions(1, 2, 3, 4, 1, 1, 1, 1), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 2, 1, 1, 1, 1), mockedCheckedOutSessions(1, 2), 2));
+ assertFalse(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 2, 3, 4, 1, 2, 3, 4, 1, 1, 1, 1),
+ mockedCheckedOutSessions(1, 2, 3, 4),
+ 8));
+ assertFalse(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 1, 2, 2, 3, 3, 4, 4, 1, 1, 1, 1),
+ mockedCheckedOutSessions(1, 2, 3, 4),
+ 8));
+
+ // The list of checked out sessions is allowed to contain up to twice the number of sessions
+ // with a given channel than it should for a perfect distribution (perfect means
+ // num_sessions_with_a_channel == num_channels).
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 1, 2, 3), 4));
+ assertFalse(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 2, 3, 4),
+ mockedCheckedOutSessions(1, 1, 1, 1, 2, 3, 4, 5, 6, 7, 8, 2, 3, 4, 5, 6),
+ 8));
+ // We're only checking the list of checked out sessions against the channel that is being added
+ // to the pool.
+ assertFalse(
+ isUnbalanced(1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(2, 2, 2, 2), 4));
+
+ // We do not consider a pool unbalanced if the list of checked out sessions only contains 2 of
+ // the same channel, even if that would still be 'more than twice the ideal number'. This
+ // prevents that a small number of checked out sessions that happen to use the same channel
+ // causes the pool to be considered unbalanced.
+ assertFalse(
+ isUnbalanced(
+ 1, mockedSessions(1, 2, 3, 4, 5, 6, 7, 8), mockedCheckedOutSessions(1, 1, 2), 8));
+
+ // A larger number of checked out sessions means that we can also have a 'large' number of the
+ // same channels in that list, as long as it does not exceed twice the number that it should be
+ // for an ideal distribution.
+ assertFalse(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 2, 3, 4, 5, 6, 7, 8),
+ mockedCheckedOutSessions(1, 1, 1, 1, 1, 2, 3, 4, 5, 6, 7, 8, 2, 4, 5, 5, 3, 4, 8, 8),
+ 8));
+ }
+
+ @Test
+ public void testIsUnbalanced_returnsTrueForUnbalancedPool() {
+ // The pool is considered unbalanced if the first numChannel sessions contain 3 or more of the
+ // same sessions as the one that is being added. Also; if the pool uses only 2 channels, then it
+ // is also considered unbalanced if the two first sessions in the pool already use the same
+ // channel as the one being added.
+ assertTrue(isUnbalanced(1, mockedSessions(1, 1), mockedCheckedOutSessions(1, 2, 1, 2), 2));
+ assertTrue(isUnbalanced(2, mockedSessions(2, 2), mockedCheckedOutSessions(1, 2, 1, 2), 2));
+
+ assertTrue(
+ isUnbalanced(1, mockedSessions(1, 1, 1, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertTrue(
+ isUnbalanced(2, mockedSessions(2, 2, 2, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertTrue(
+ isUnbalanced(3, mockedSessions(1, 3, 3, 3), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+ assertTrue(
+ isUnbalanced(4, mockedSessions(1, 4, 4, 4), mockedCheckedOutSessions(1, 2, 3, 4), 4));
+
+ assertTrue(
+ isUnbalanced(
+ 1, mockedSessions(1, 2, 3, 4, 5, 6, 1, 1), mockedCheckedOutSessions(1, 2, 3, 4), 8));
+ assertTrue(
+ isUnbalanced(
+ 2, mockedSessions(1, 3, 4, 5, 6, 2, 2, 2), mockedCheckedOutSessions(1, 2, 3, 4), 8));
+ assertTrue(
+ isUnbalanced(
+ 3, mockedSessions(1, 2, 3, 3, 4, 5, 3, 6), mockedCheckedOutSessions(1, 2, 3, 4), 8));
+ assertTrue(
+ isUnbalanced(
+ 4, mockedSessions(1, 2, 3, 4, 5, 4, 5, 4), mockedCheckedOutSessions(1, 2, 3, 4), 8));
+
+ // The pool is also considered unbalanced if the list of checked out sessions contain more than
+ // 2 times as many sessions of the one being returned as it should.
+ assertTrue(
+ isUnbalanced(1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 1, 2, 1), 4));
+ assertTrue(
+ isUnbalanced(2, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(2, 2, 2, 4), 4));
+ assertTrue(
+ isUnbalanced(3, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 3, 3, 3), 4));
+ assertTrue(
+ isUnbalanced(4, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(4, 2, 4, 4), 4));
+ assertTrue(
+ isUnbalanced(
+ 1, mockedSessions(1, 2, 3, 4), mockedCheckedOutSessions(1, 1, 2, 1, 1, 2, 3, 1), 4));
+
+ assertTrue(
+ isUnbalanced(
+ 1, mockedSessions(1, 2, 3, 4, 5, 6, 7, 8), mockedCheckedOutSessions(1, 1, 1, 3), 8));
+ assertTrue(
+ isUnbalanced(
+ 1,
+ mockedSessions(1, 2, 3, 4, 5, 6, 7, 8),
+ mockedCheckedOutSessions(1, 1, 1, 2, 3, 4, 5, 6, 7, 8, 1, 1),
+ 8));
+ }
+}
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 50b376f5ac5..7690ea3934c 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
@@ -36,18 +36,19 @@
import com.google.common.io.BaseEncoding;
import com.google.common.io.Files;
import java.io.File;
-import java.io.IOException;
-import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collections;
+import java.util.Objects;
import org.junit.Test;
+import org.junit.function.ThrowingRunnable;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
public class ConnectionOptionsTest {
private static final String FILE_TEST_PATH =
- ConnectionOptionsTest.class.getResource("test-key.json").getFile();
+ Objects.requireNonNull(ConnectionOptionsTest.class.getResource("test-key.json")).getFile();
private static final String DEFAULT_HOST = "https://spanner.googleapis.com";
private static final String TEST_PROJECT = "test-project-123";
private static final String TEST_INSTANCE = "test-instance-123";
@@ -727,34 +728,61 @@ public void testInvalidCredentials() {
}
@Test
- public void testNonBase64EncodedCredentials() {
- String uri =
- "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=not-a-base64-string/";
- SpannerException e =
- assertThrows(
- SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
- assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
- assertThat(e.getMessage())
- .contains("The encoded credentials could not be decoded as a base64 string.");
- }
-
- @Test
- public void testInvalidEncodedCredentials() throws UnsupportedEncodingException {
- String uri =
- String.format(
- "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=%s",
- BaseEncoding.base64Url().encode("not-a-credentials-JSON-string".getBytes("UTF-8")));
- SpannerException e =
- assertThrows(
- SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
- assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
- assertThat(e.getMessage())
- .contains(
- "The encoded credentials do not contain a valid Google Cloud credentials JSON string.");
- }
-
- @Test
- public void testValidEncodedCredentials() throws Exception {
+ public void testNonBase64EncodedCredentials() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
+ () -> {
+ String uri =
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=not-a-base64-string/";
+ SpannerException e =
+ assertThrows(
+ SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+ assertThat(e.getMessage())
+ .contains("The encoded credentials could not be decoded as a base64 string.");
+ });
+ }
+
+ @Test
+ public void testInvalidEncodedCredentials() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
+ () -> {
+ String uri =
+ String.format(
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=%s",
+ BaseEncoding.base64Url()
+ .encode("not-a-credentials-JSON-string".getBytes(StandardCharsets.UTF_8)));
+ SpannerException e =
+ assertThrows(
+ SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertEquals(ErrorCode.INVALID_ARGUMENT, e.getErrorCode());
+ assertThat(e.getMessage())
+ .contains(
+ "The encoded credentials do not contain a valid Google Cloud credentials JSON string.");
+ });
+ }
+
+ @Test
+ public void testValidEncodedCredentials() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
+ () -> {
+ String encoded =
+ BaseEncoding.base64Url().encode(Files.asByteSource(new File(FILE_TEST_PATH)).read());
+ String uri =
+ String.format(
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=%s",
+ encoded);
+
+ ConnectionOptions options = ConnectionOptions.newBuilder().setUri(uri).build();
+ assertEquals(
+ new CredentialsService().createCredentials(FILE_TEST_PATH), options.getCredentials());
+ });
+ }
+
+ @Test
+ public void testValidEncodedCredentials_WithoutEnablingProperty() throws Throwable {
String encoded =
BaseEncoding.base64Url().encode(Files.asByteSource(new File(FILE_TEST_PATH)).read());
String uri =
@@ -762,81 +790,146 @@ public void testValidEncodedCredentials() throws Exception {
"cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?encodedCredentials=%s",
encoded);
- ConnectionOptions options = ConnectionOptions.newBuilder().setUri(uri).build();
- assertEquals(
- new CredentialsService().createCredentials(FILE_TEST_PATH), options.getCredentials());
+ SpannerException exception =
+ assertThrows(
+ SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
}
@Test
- public void testSetCredentialsAndEncodedCredentials() throws Exception {
- String encoded =
- BaseEncoding.base64Url().encode(Files.asByteSource(new File(FILE_TEST_PATH)).read());
- String uri =
- String.format(
- "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentials=%s;encodedCredentials=%s",
- FILE_TEST_PATH, encoded);
+ public void testSetCredentialsAndEncodedCredentials() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_ENCODED_CREDENTIALS_SYSTEM_PROPERTY,
+ () -> {
+ String encoded =
+ BaseEncoding.base64Url().encode(Files.asByteSource(new File(FILE_TEST_PATH)).read());
+ String uri =
+ String.format(
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentials=%s;encodedCredentials=%s",
+ FILE_TEST_PATH, encoded);
- IllegalArgumentException e =
- assertThrows(
- IllegalArgumentException.class,
- () -> ConnectionOptions.newBuilder().setUri(uri).build());
- assertTrue(
- e.getMessage(),
- e.getMessage()
- .contains(
- "Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token"));
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertTrue(
+ e.getMessage(),
+ e.getMessage()
+ .contains(
+ "Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token"));
+ });
}
public static class TestCredentialsProvider implements CredentialsProvider {
@Override
- public Credentials getCredentials() throws IOException {
+ public Credentials getCredentials() {
return NoCredentials.getInstance();
}
}
+ static void runWithSystemPropertyEnabled(String systemPropertyName, ThrowingRunnable runnable)
+ throws Throwable {
+ String originalValue = System.getProperty(systemPropertyName);
+ System.setProperty(systemPropertyName, "true");
+ try {
+ runnable.run();
+ } finally {
+ if (originalValue == null) {
+ System.clearProperty(systemPropertyName);
+ } else {
+ System.setProperty(systemPropertyName, originalValue);
+ }
+ }
+ }
+
+ @Test
+ public void testValidCredentialsProvider() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
+ () -> {
+ String uri =
+ String.format(
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentialsProvider=%s",
+ TestCredentialsProvider.class.getName());
+
+ ConnectionOptions options = ConnectionOptions.newBuilder().setUri(uri).build();
+ assertEquals(NoCredentials.getInstance(), options.getCredentials());
+ });
+ }
+
@Test
- public void testValidCredentialsProvider() {
+ public void testValidCredentialsProvider_WithoutEnablingSystemProperty() {
String uri =
String.format(
"cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentialsProvider=%s",
TestCredentialsProvider.class.getName());
+ SpannerException exception =
+ assertThrows(
+ SpannerException.class, () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
+ assertEquals(
+ "FAILED_PRECONDITION: credentialsProvider can only be used if the system property ENABLE_CREDENTIALS_PROVIDER has been set to true. "
+ + "Start the application with the JVM command line option -DENABLE_CREDENTIALS_PROVIDER=true",
+ exception.getMessage());
+ }
- ConnectionOptions options = ConnectionOptions.newBuilder().setUri(uri).build();
- assertEquals(NoCredentials.getInstance(), options.getCredentials());
+ @Test
+ public void testSetCredentialsAndCredentialsProvider() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
+ () -> {
+ String uri =
+ String.format(
+ "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentials=%s;credentialsProvider=%s",
+ FILE_TEST_PATH, NoCredentialsProvider.class.getName());
+
+ IllegalArgumentException e =
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> ConnectionOptions.newBuilder().setUri(uri).build());
+ assertTrue(
+ e.getMessage(),
+ e.getMessage()
+ .contains(
+ "Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token"));
+ });
}
@Test
- public void testSetCredentialsAndCredentialsProvider() {
- String uri =
- String.format(
- "cloudspanner:/projects/test-project/instances/test-instance/databases/test-database?credentials=%s;credentialsProvider=%s",
- FILE_TEST_PATH, NoCredentialsProvider.class.getName());
+ public void testExternalChannelProvider() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_CHANNEL_PROVIDER_SYSTEM_PROPERTY,
+ () -> {
+ String baseUri =
+ "cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database";
- IllegalArgumentException e =
- assertThrows(
- IllegalArgumentException.class,
- () -> ConnectionOptions.newBuilder().setUri(uri).build());
- assertTrue(
- e.getMessage(),
- e.getMessage()
- .contains(
- "Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token"));
+ ConnectionOptions options =
+ ConnectionOptions.newBuilder()
+ .setUri(
+ baseUri
+ + "?channelProvider=com.google.cloud.spanner.connection.TestChannelProvider")
+ .setCredentials(NoCredentials.getInstance())
+ .build();
+
+ TransportChannelProvider provider = options.getChannelProvider();
+ assertTrue(provider instanceof InstantiatingGrpcChannelProvider);
+ });
}
@Test
- public void testExternalChannelProvider() {
+ public void testExternalChannelProvider_WithoutEnablingProperty() throws Throwable {
String baseUri =
"cloudspanner:/projects/test-project-123/instances/test-instance/databases/test-database";
-
- ConnectionOptions options =
- ConnectionOptions.newBuilder()
- .setUri(
- baseUri
- + "?channelProvider=com.google.cloud.spanner.connection.TestChannelProvider")
- .setCredentials(NoCredentials.getInstance())
- .build();
-
- TransportChannelProvider provider = options.getChannelProvider();
- assertTrue(provider instanceof InstantiatingGrpcChannelProvider);
+ SpannerException exception =
+ assertThrows(
+ SpannerException.class,
+ () ->
+ ConnectionOptions.newBuilder()
+ .setUri(
+ baseUri
+ + "?channelProvider=com.google.cloud.spanner.connection.TestChannelProvider")
+ .setCredentials(NoCredentials.getInstance())
+ .build());
+ assertEquals(ErrorCode.FAILED_PRECONDITION, exception.getErrorCode());
}
}
diff --git a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/CredentialsProviderTest.java b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/CredentialsProviderTest.java
index da7751f3889..cdab8535029 100644
--- a/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/CredentialsProviderTest.java
+++ b/google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/CredentialsProviderTest.java
@@ -16,13 +16,13 @@
package com.google.cloud.spanner.connection;
+import static com.google.cloud.spanner.connection.ConnectionOptionsTest.runWithSystemPropertyEnabled;
import static org.junit.Assert.assertEquals;
import com.google.api.gax.core.CredentialsProvider;
import com.google.auth.Credentials;
import com.google.auth.oauth2.OAuth2Credentials;
import io.grpc.ManagedChannelBuilder;
-import java.io.IOException;
import java.io.ObjectStreamException;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.BeforeClass;
@@ -64,63 +64,81 @@ public int hashCode() {
static final class TestCredentialsProvider implements CredentialsProvider {
@Override
- public Credentials getCredentials() throws IOException {
+ public Credentials getCredentials() {
return new TestCredentials(COUNTER.incrementAndGet());
}
}
@Test
- public void testCredentialsProvider() {
- ConnectionOptions options =
- ConnectionOptions.newBuilder()
- .setUri(
- String.format(
- "cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
- getPort(), TestCredentialsProvider.class.getName()))
- .setConfigurator(
- spannerOptions ->
- spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext))
- .build();
+ public void testCredentialsProvider() throws Throwable {
+ runWithSystemPropertyEnabled(
+ ConnectionOptions.ENABLE_CREDENTIALS_PROVIDER_SYSTEM_PROPERTY,
+ () -> {
+ ConnectionOptions options =
+ ConnectionOptions.newBuilder()
+ .setUri(
+ String.format(
+ "cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
+ getPort(), TestCredentialsProvider.class.getName()))
+ .setConfigurator(
+ spannerOptions ->
+ spannerOptions.setChannelConfigurator(
+ ManagedChannelBuilder::usePlaintext))
+ .build();
- try (Connection connection = options.getConnection()) {
- assertEquals(
- TestCredentials.class,
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials().getClass());
- TestCredentials credentials =
- (TestCredentials)
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
- assertEquals(1, credentials.id);
- }
- // The second connection should get the same credentials from the provider.
- try (Connection connection = options.getConnection()) {
- assertEquals(
- TestCredentials.class,
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials().getClass());
- TestCredentials credentials =
- (TestCredentials)
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
- assertEquals(1, credentials.id);
- }
+ try (Connection connection = options.getConnection()) {
+ assertEquals(
+ TestCredentials.class,
+ ((ConnectionImpl) connection)
+ .getSpanner()
+ .getOptions()
+ .getCredentials()
+ .getClass());
+ TestCredentials credentials =
+ (TestCredentials)
+ ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
+ assertEquals(1, credentials.id);
+ }
+ // The second connection should get the same credentials from the provider.
+ try (Connection connection = options.getConnection()) {
+ assertEquals(
+ TestCredentials.class,
+ ((ConnectionImpl) connection)
+ .getSpanner()
+ .getOptions()
+ .getCredentials()
+ .getClass());
+ TestCredentials credentials =
+ (TestCredentials)
+ ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
+ assertEquals(1, credentials.id);
+ }
- // Creating new ConnectionOptions should refresh the credentials.
- options =
- ConnectionOptions.newBuilder()
- .setUri(
- String.format(
- "cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
- getPort(), TestCredentialsProvider.class.getName()))
- .setConfigurator(
- spannerOptions ->
- spannerOptions.setChannelConfigurator(ManagedChannelBuilder::usePlaintext))
- .build();
- try (Connection connection = options.getConnection()) {
- assertEquals(
- TestCredentials.class,
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials().getClass());
- TestCredentials credentials =
- (TestCredentials)
- ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
- assertEquals(2, credentials.id);
- }
+ // Creating new ConnectionOptions should refresh the credentials.
+ options =
+ ConnectionOptions.newBuilder()
+ .setUri(
+ String.format(
+ "cloudspanner://localhost:%d/projects/proj/instances/inst/databases/db?credentialsProvider=%s",
+ getPort(), TestCredentialsProvider.class.getName()))
+ .setConfigurator(
+ spannerOptions ->
+ spannerOptions.setChannelConfigurator(
+ ManagedChannelBuilder::usePlaintext))
+ .build();
+ try (Connection connection = options.getConnection()) {
+ assertEquals(
+ TestCredentials.class,
+ ((ConnectionImpl) connection)
+ .getSpanner()
+ .getOptions()
+ .getCredentials()
+ .getClass());
+ TestCredentials credentials =
+ (TestCredentials)
+ ((ConnectionImpl) connection).getSpanner().getOptions().getCredentials();
+ assertEquals(2, credentials.id);
+ }
+ });
}
}
diff --git a/grpc-google-cloud-spanner-admin-database-v1/pom.xml b/grpc-google-cloud-spanner-admin-database-v1/pom.xml
index efbc3323034..c72c46cf01b 100644
--- a/grpc-google-cloud-spanner-admin-database-v1/pom.xml
+++ b/grpc-google-cloud-spanner-admin-database-v1/pom.xml
@@ -4,13 +4,13 @@
4.0.0
com.google.api.grpc
grpc-google-cloud-spanner-admin-database-v1
- 6.46.0
+ 6.47.0
grpc-google-cloud-spanner-admin-database-v1
GRPC library for grpc-google-cloud-spanner-admin-database-v1
com.google.cloud
google-cloud-spanner-parent
- 6.46.0
+ 6.47.0
diff --git a/grpc-google-cloud-spanner-admin-instance-v1/pom.xml b/grpc-google-cloud-spanner-admin-instance-v1/pom.xml
index 07a676637d4..f79c2e6b1f9 100644
--- a/grpc-google-cloud-spanner-admin-instance-v1/pom.xml
+++ b/grpc-google-cloud-spanner-admin-instance-v1/pom.xml
@@ -4,13 +4,13 @@
4.0.0
com.google.api.grpc
grpc-google-cloud-spanner-admin-instance-v1
- 6.46.0
+ 6.47.0
grpc-google-cloud-spanner-admin-instance-v1
GRPC library for grpc-google-cloud-spanner-admin-instance-v1
com.google.cloud
google-cloud-spanner-parent
- 6.46.0
+ 6.47.0
diff --git a/grpc-google-cloud-spanner-v1/pom.xml b/grpc-google-cloud-spanner-v1/pom.xml
index c87f44d3c78..f1b13b9ab8f 100644
--- a/grpc-google-cloud-spanner-v1/pom.xml
+++ b/grpc-google-cloud-spanner-v1/pom.xml
@@ -4,13 +4,13 @@
4.0.0
com.google.api.grpc
grpc-google-cloud-spanner-v1
- 6.46.0
+ 6.47.0
grpc-google-cloud-spanner-v1
GRPC library for grpc-google-cloud-spanner-v1
com.google.cloud
google-cloud-spanner-parent
- 6.46.0
+ 6.47.0
diff --git a/pom.xml b/pom.xml
index 0274cdc4543..017960333a8 100644
--- a/pom.xml
+++ b/pom.xml
@@ -4,7 +4,7 @@
com.google.cloud
google-cloud-spanner-parent
pom
- 6.46.0
+ 6.47.0
Google Cloud Spanner Parent
https://github.com/googleapis/java-spanner
@@ -54,7 +54,7 @@
UTF-8
github
google-cloud-spanner-parent
- 3.14.0
+ 3.15.0