perf: optimize JdbcDataSource#getConnection() (#2371)

The `JdbcDataSource#getConnection()` method repeatedly executed a number of steps
that were not necessary for each new connection. This has now been optimized, so
they are only executed once, as long as the properties of the DataSource do not
change.

Author: olavloite

pom.xml

@@ -260,6 +260,7 @@
             <spanner.testenv.instance>
               projects/gcloud-devel/instances/spanner-testing-east1
             </spanner.testenv.instance>
+            <java.util.logging.config.file>logging.properties</java.util.logging.config.file>
           </systemPropertyVariables>
         </configuration>
       </plugin>
@@ -274,6 +275,7 @@
             <spanner.testenv.instance>
               projects/gcloud-devel/instances/spanner-testing-east1
             </spanner.testenv.instance>
+            <java.util.logging.config.file>logging.properties</java.util.logging.config.file>
           </systemPropertyVariables>
           <forkedProcessTimeoutInSeconds>2400</forkedProcessTimeoutInSeconds>
           <forkCount>4</forkCount>

src/main/java/com/google/cloud/spanner/jdbc/JdbcDataSource.java

@@ -16,11 +16,14 @@
 
 package com.google.cloud.spanner.jdbc;
 
+import static com.google.cloud.spanner.jdbc.JdbcDriver.appendPropertiesToUrl;
+import static com.google.cloud.spanner.jdbc.JdbcDriver.buildConnectionOptions;
+import static com.google.cloud.spanner.jdbc.JdbcDriver.maybeAddUserAgent;
+
 import com.google.cloud.spanner.connection.ConnectionOptions;
 import com.google.rpc.Code;
 import java.io.PrintWriter;
 import java.sql.Connection;
-import java.sql.DriverManager;
 import java.sql.SQLException;
 import java.sql.SQLFeatureNotSupportedException;
 import java.util.Properties;
@@ -35,6 +38,8 @@ public class JdbcDataSource extends AbstractJdbcWrapper implements DataSource {
   private Boolean readonly;
   private Boolean retryAbortsInternally;
 
+  private volatile ConnectionOptions cachedConnectionOptions;
+
   // Make sure the JDBC driver class is loaded.
   static {
     try {
@@ -76,12 +81,22 @@ public Connection getConnection() throws SQLException {
       throw JdbcSqlExceptionFactory.of(
           "There is no URL specified for this data source", Code.FAILED_PRECONDITION);
     }
-    if (!JdbcDriver.getRegisteredDriver().acceptsURL(getUrl())) {
-      throw JdbcSqlExceptionFactory.of(
-          "The URL " + getUrl() + " is not valid for the data source " + getClass().getName(),
-          Code.FAILED_PRECONDITION);
+    if (cachedConnectionOptions == null) {
+      synchronized (this) {
+        if (cachedConnectionOptions == null) {
+          if (!JdbcDriver.getRegisteredDriver().acceptsURL(getUrl())) {
+            throw JdbcSqlExceptionFactory.of(
+                "The URL " + getUrl() + " is not valid for the data source " + getClass().getName(),
+                Code.FAILED_PRECONDITION);
+          }
+          Properties properties = createProperties();
+          maybeAddUserAgent(properties);
+          String connectionUri = appendPropertiesToUrl(url.substring(5), properties);
+          cachedConnectionOptions = buildConnectionOptions(connectionUri, properties);
+        }
+      }
     }
-    return DriverManager.getConnection(getUrl(), createProperties());
+    return new JdbcConnection(getUrl(), cachedConnectionOptions);
   }
 
   @Override
@@ -114,6 +129,12 @@ public boolean isClosed() {
     return false;
   }
 
+  private void clearCachedConnectionOptions() {
+    synchronized (this) {
+      cachedConnectionOptions = null;
+    }
+  }
+
   /**
    * @return the JDBC URL to use for this {@link DataSource}.
    */
@@ -125,6 +146,7 @@ public String getUrl() {
    * @param url The JDBC URL to use for this {@link DataSource}.
    */
   public void setUrl(String url) {
+    clearCachedConnectionOptions();
     this.url = url;
   }
 
@@ -143,6 +165,7 @@ public String getCredentials() {
    *     connection URL will be used.
    */
   public void setCredentials(String credentials) {
+    clearCachedConnectionOptions();
     this.credentials = credentials;
   }
 
@@ -161,6 +184,7 @@ public Boolean getAutocommit() {
    *     the connection URL will be used.
    */
   public void setAutocommit(Boolean autocommit) {
+    clearCachedConnectionOptions();
     this.autocommit = autocommit;
   }
 
@@ -179,6 +203,7 @@ public Boolean getReadonly() {
    *     URL will be used.
    */
   public void setReadonly(Boolean readonly) {
+    clearCachedConnectionOptions();
     this.readonly = readonly;
   }
 
@@ -197,6 +222,7 @@ public Boolean getRetryAbortsInternally() {
    *     this property, the value in the connection URL will be used.
    */
   public void setRetryAbortsInternally(Boolean retryAbortsInternally) {
+    clearCachedConnectionOptions();
     this.retryAbortsInternally = retryAbortsInternally;
   }
 }

src/main/java/com/google/cloud/spanner/jdbc/JdbcDriver.java

@@ -37,6 +37,7 @@
 import java.sql.SQLWarning;
 import java.util.Map.Entry;
 import java.util.Properties;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -247,7 +248,7 @@ public Connection connect(String url, Properties info) throws SQLException {
     return null;
   }
 
-  private ConnectionOptions buildConnectionOptions(String connectionUrl, Properties info) {
+  static ConnectionOptions buildConnectionOptions(String connectionUrl, Properties info) {
     ConnectionOptions.Builder builder =
         ConnectionOptions.newBuilder().setTracingPrefix("JDBC").setUri(connectionUrl);
     if (info.containsKey(OPEN_TELEMETRY_PROPERTY_KEY)
@@ -272,40 +273,42 @@ static void maybeAddUserAgent(Properties properties) {
     }
   }
 
-  static boolean isHibernate() {
-    // Cache the result as the check is relatively expensive, and we also don't want to create
-    // multiple different Spanner instances just to get the correct user-agent in every case.
-    return Suppliers.memoize(
-            () -> {
-              try {
-                // First check if the Spanner Hibernate dialect is on the classpath. If it is, then
-                // we assume that Hibernate will (eventually) be used.
-                Class.forName(
-                    "com.google.cloud.spanner.hibernate.SpannerDialect",
-                    /* initialize= */ false,
-                    JdbcDriver.class.getClassLoader());
-                return true;
-              } catch (Throwable ignore) {
-              }
+  private static final Supplier<Boolean> isHibernate =
+      Suppliers.memoize(
+          () -> {
+            try {
+              // First check if the Spanner Hibernate dialect is on the classpath. If it is, then
+              // we assume that Hibernate will (eventually) be used.
+              Class.forName(
+                  "com.google.cloud.spanner.hibernate.SpannerDialect",
+                  /* initialize= */ false,
+                  JdbcDriver.class.getClassLoader());
+              return true;
+            } catch (Throwable ignore) {
+            }
 
-              // If we did not find the Spanner Hibernate dialect on the classpath, then do a
-              // check if the connection is still being created by Hibernate using the built-in
-              // Spanner dialect in Hibernate.
-              try {
-                StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
-                for (StackTraceElement element : callStack) {
-                  if (element.getClassName().contains(".hibernate.")) {
-                    return true;
-                  }
+            // If we did not find the Spanner Hibernate dialect on the classpath, then do a
+            // check if the connection is still being created by Hibernate using the built-in
+            // Spanner dialect in Hibernate.
+            try {
+              StackTraceElement[] callStack = Thread.currentThread().getStackTrace();
+              for (StackTraceElement element : callStack) {
+                if (element.getClassName().contains(".hibernate.")) {
+                  return true;
                 }
-              } catch (Throwable ignore) {
               }
-              return false;
-            })
-        .get();
+            } catch (Throwable ignore) {
+            }
+            return false;
+          });
+
+  static boolean isHibernate() {
+    // Cache the result as the check is relatively expensive, and we also don't want to create
+    // multiple different Spanner instances just to get the correct user-agent in every case.
+    return isHibernate.get();
   }
 
-  private String appendPropertiesToUrl(String url, Properties info) {
+  static String appendPropertiesToUrl(String url, Properties info) {
     StringBuilder res = new StringBuilder(url);
     for (Entry<Object, Object> entry : info.entrySet()) {
       if (entry.getValue() instanceof String && !"".equals(entry.getValue())) {

src/test/java/com/google/cloud/spanner/jdbc/JdbcDataSourceTest.java

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2026 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.jdbc;
+
+import static org.junit.Assert.assertEquals;
+
+import com.google.cloud.spanner.connection.AbstractMockServerTest;
+import java.sql.Connection;
+import java.sql.SQLException;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class JdbcDataSourceTest extends AbstractMockServerTest {
+
+  @Override
+  protected String getBaseUrl() {
+    return String.format(
+        "jdbc:cloudspanner://localhost:%d/projects/p/instances/i/databases/d?usePlainText=true",
+        getPort());
+  }
+
+  @Test
+  public void testGetConnectionFromNewDataSource() throws SQLException {
+    for (boolean autoCommit : new boolean[] {true, false}) {
+      JdbcDataSource dataSource = new JdbcDataSource();
+      dataSource.setUrl(getBaseUrl());
+      dataSource.setAutocommit(autoCommit);
+      try (Connection connection = dataSource.getConnection()) {
+        assertEquals(autoCommit, connection.getAutoCommit());
+      }
+    }
+  }
+
+  @Test
+  public void testGetConnectionFromCachedDataSource() throws SQLException {
+    JdbcDataSource dataSource = new JdbcDataSource();
+    dataSource.setUrl(getBaseUrl());
+    for (boolean autoCommit : new boolean[] {true, false}) {
+      // Changing a property on the DataSource should invalidate the internally cached
+      // ConnectionOptions.
+      dataSource.setAutocommit(autoCommit);
+      try (Connection connection = dataSource.getConnection()) {
+        assertEquals(autoCommit, connection.getAutoCommit());
+      }
+    }
+  }
+}

src/test/resources/logging.properties

@@ -0,0 +1,7 @@
+.level=INFO
+.handlers=java.util.logging.ConsoleHandler
+java.util.logging.ConsoleHandler.level=INFO
+java.util.logging.Logger.useParentHandlers=true
+
+# Set log level to WARN for SpannerImpl to prevent log spamming of the Spanner configuration.
+com.google.cloud.spanner.SpannerImpl.LEVEL=WARN