Skip to content

chore(spanner): introduce commitRpcTimeout to connection properties #13801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sagnghos wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

chore(spanner): introduce commitRpcTimeout to connection properties #13801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions java-spanner/google-cloud-spanner/src/main/java/com/google/cloud/spanner/SpannerOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ static GcpChannelPoolOptions mergeWithDefaultChannelPoolOptions(
private final DatabaseAdminStubSettings databaseAdminStubSettings;
private final Duration partitionedDmlTimeout;
private final Duration grpcKeepAliveTime;
private final Duration commitRpcTimeout;
private final Duration grpcKeepAliveTimeout;
private final boolean grpcGcpExtensionEnabled;
private final GcpManagedChannelOptions grpcGcpOptions;
Expand Down Expand Up @@ -925,6 +926,15 @@ protected SpannerOptions(Builder builder) {
sessionLabels = builder.sessionLabels;
try {
String resolvedUniversalDomain = getResolvedUniverseDomain();
if (builder.commitRpcTimeout != null) {
com.google.api.gax.retrying.RetrySettings currentCommitRetrySettings = builder.spannerStubSettingsBuilder.commitSettings().getRetrySettings();
builder.spannerStubSettingsBuilder.commitSettings().setRetrySettings(
currentCommitRetrySettings.toBuilder()
.setInitialRpcTimeoutDuration(builder.commitRpcTimeout)
.setMaxRpcTimeoutDuration(builder.commitRpcTimeout)
.setTotalTimeoutDuration(builder.commitRpcTimeout)
.build());
}
spannerStubSettings =
builder.spannerStubSettingsBuilder.setUniverseDomain(resolvedUniversalDomain).build();
instanceAdminStubSettings =
Expand All @@ -942,6 +952,7 @@ protected SpannerOptions(Builder builder) {
}
partitionedDmlTimeout = builder.partitionedDmlTimeout;
grpcKeepAliveTime = builder.grpcKeepAliveTime;
commitRpcTimeout = builder.commitRpcTimeout;
grpcKeepAliveTimeout = builder.grpcKeepAliveTimeout;
grpcGcpExtensionEnabled = builder.grpcGcpExtensionEnabled;
grpcGcpOptions = builder.grpcGcpOptions;
Expand Down Expand Up @@ -1322,6 +1333,7 @@ private static Builder prepareBuilder(Builder builder) {
private boolean enableGrpcGcpOtelMetrics =
SpannerOptions.environment.isEnableGrpcGcpOtelMetrics();
private Duration grpcKeepAliveTime = Duration.ofSeconds(120);
private Duration commitRpcTimeout = Duration.ofMinutes(60);
private Duration grpcKeepAliveTimeout = Duration.ofSeconds(20);
private CallCredentialsProvider callCredentialsProvider;
private CloseableExecutorProvider asyncExecutorProvider;
Expand Down Expand Up @@ -1437,6 +1449,7 @@ protected Builder() {
this.defaultQueryOptions = options.defaultQueryOptions;
this.callCredentialsProvider = options.callCredentialsProvider;
this.grpcKeepAliveTime = options.grpcKeepAliveTime;
this.commitRpcTimeout = options.commitRpcTimeout;
this.grpcKeepAliveTimeout = options.grpcKeepAliveTimeout;
this.asyncExecutorProvider = options.asyncExecutorProvider;
this.compressorName = options.compressorName;
Expand Down Expand Up @@ -1704,6 +1717,11 @@ public Builder setPartitionedDmlTimeoutDuration(Duration timeout) {
* Sets the keep-alive time for gRPC connections. The default is 120 seconds. Note that the
* client-side keepalive time is clamped to a minimum of 10 seconds by gRPC.
*/
public Builder setCommitRpcTimeout(Duration commitRpcTimeout) {
this.commitRpcTimeout = commitRpcTimeout;
return this;
}
Comment on lines +1720 to +1723

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To prevent invalid configurations early, consider adding a check to ensure that commitRpcTimeout is not negative when it is non-null.

    public Builder setCommitRpcTimeout(Duration commitRpcTimeout) {
      if (commitRpcTimeout != null) {
        Preconditions.checkArgument(
            !commitRpcTimeout.isNegative(), "commitRpcTimeout cannot be negative");
      }
      this.commitRpcTimeout = commitRpcTimeout;
      return this;
    }


public Builder setGrpcKeepAliveTime(Duration grpcKeepAliveTime) {
Preconditions.checkNotNull(grpcKeepAliveTime, "grpcKeepAliveTime cannot be null");
Preconditions.checkArgument(
Expand Down Expand Up @@ -2527,6 +2545,10 @@ public Duration getPartitionedDmlTimeoutDuration() {
return partitionedDmlTimeout;
}

public Duration getCommitRpcTimeout() {
return commitRpcTimeout;
}

public Duration getGrpcKeepAliveTime() {
return grpcKeepAliveTime;
}
Expand Down
9 changes: 9 additions & 0 deletions ...le-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import static com.google.cloud.spanner.connection.ConnectionProperties.ENDPOINT;
import static com.google.cloud.spanner.connection.ConnectionProperties.GRPC_INTERCEPTOR_PROVIDER;
import static com.google.cloud.spanner.connection.ConnectionProperties.GRPC_KEEPALIVE_TIME;
import static com.google.cloud.spanner.connection.ConnectionProperties.COMMIT_RPC_TIMEOUT;
import static com.google.cloud.spanner.connection.ConnectionProperties.GRPC_KEEPALIVE_TIMEOUT;
import static com.google.cloud.spanner.connection.ConnectionProperties.IS_EXPERIMENTAL_HOST;
import static com.google.cloud.spanner.connection.ConnectionProperties.LENIENT;
Expand Down Expand Up @@ -284,6 +285,9 @@ public class ConnectionOptions {
/** Name of the 'grpcKeepAliveTime' connection property. */
public static final String GRPC_KEEPALIVE_TIME_PROPERTY_NAME = "grpcKeepAliveTime";

/** Name of the 'commitRpcTimeout' connection property. */
public static final String COMMIT_RPC_TIMEOUT_PROPERTY_NAME = "commitRpcTimeout";

/** Name of the 'grpcKeepAliveTimeout' connection property. */
public static final String GRPC_KEEPALIVE_TIMEOUT_PROPERTY_NAME = "grpcKeepAliveTimeout";

Expand Down Expand Up @@ -1092,6 +1096,11 @@ public Integer getNumChannels() {
}

/** The gRPC keepalive time for this connection. */
/** The commit RPC timeout for this connection. */
public Duration getCommitRpcTimeout() {
return getInitialConnectionPropertyValue(COMMIT_RPC_TIMEOUT);
}

public Duration getGrpcKeepAliveTime() {
return getInitialConnectionPropertyValue(GRPC_KEEPALIVE_TIME);
}
Comment on lines 1098 to 1106

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Javadoc comment for getGrpcKeepAliveTime() was accidentally moved above getCommitRpcTimeout(), resulting in duplicated Javadoc blocks for getCommitRpcTimeout() and leaving getGrpcKeepAliveTime() without any documentation. Please correct the placement of these Javadoc comments.

Suggested change
/** The gRPC keepalive time for this connection. */
/** The commit RPC timeout for this connection. */
public Duration getCommitRpcTimeout() {
return getInitialConnectionPropertyValue(COMMIT_RPC_TIMEOUT);
}
public Duration getGrpcKeepAliveTime() {
return getInitialConnectionPropertyValue(GRPC_KEEPALIVE_TIME);
}
/** The commit RPC timeout for this connection. */
public Duration getCommitRpcTimeout() {
return getInitialConnectionPropertyValue(COMMIT_RPC_TIMEOUT);
}
/** The gRPC keepalive time for this connection. */
public Duration getGrpcKeepAliveTime() {
return getInitialConnectionPropertyValue(GRPC_KEEPALIVE_TIME);
}

Expand Down
8 changes: 8 additions & 0 deletions ...cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@
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.GRPC_KEEPALIVE_TIMEOUT_PROPERTY_NAME;
import static com.google.cloud.spanner.connection.ConnectionOptions.COMMIT_RPC_TIMEOUT_PROPERTY_NAME;
import static com.google.cloud.spanner.connection.ConnectionOptions.GRPC_KEEPALIVE_TIME_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;
Expand Down Expand Up @@ -255,6 +256,13 @@ public class ConnectionProperties {
BOOLEANS,
BooleanConverter.INSTANCE,
Context.STARTUP);
static final ConnectionProperty<Duration> COMMIT_RPC_TIMEOUT =
create(
COMMIT_RPC_TIMEOUT_PROPERTY_NAME,
"The default RPC timeout applied strictly to commit operations (e.g. '30s', '10000ms').",
null,
DurationConverter.INSTANCE,
Context.STARTUP);
static final ConnectionProperty<Duration> GRPC_KEEPALIVE_TIME =
create(
GRPC_KEEPALIVE_TIME_PROPERTY_NAME,
Expand Down
7 changes: 7 additions & 0 deletions ...r/google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ static class SpannerPoolKey {
private final String host;
private final String projectId;
private final Duration grpcKeepAliveTime;
final Duration commitRpcTimeout;

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The field commitRpcTimeout in SpannerPoolKey is declared with package-private visibility, whereas all other fields in this class are declared as private final. For consistency and proper encapsulation, please make it private final.

Suggested change
final Duration commitRpcTimeout;
private final Duration commitRpcTimeout;

private final Duration grpcKeepAliveTimeout;
private final CredentialsKey credentialsKey;
private final SessionPoolOptions sessionPoolOptions;
Expand Down Expand Up @@ -226,6 +227,7 @@ private SpannerPoolKey(ConnectionOptions options) throws IOException {
this.universeDomain = options.getUniverseDomain();
this.grpcInterceptorProvider = options.getGrpcInterceptorProviderName();
this.grpcKeepAliveTime = options.getGrpcKeepAliveTime();
this.commitRpcTimeout = options.getCommitRpcTimeout();
this.grpcKeepAliveTimeout = options.getGrpcKeepAliveTimeout();
}

Expand Down Expand Up @@ -266,6 +268,7 @@ public boolean equals(Object o) {
&& Objects.equals(this.universeDomain, other.universeDomain)
&& Objects.equals(this.grpcInterceptorProvider, other.grpcInterceptorProvider)
&& Objects.equals(this.grpcKeepAliveTime, other.grpcKeepAliveTime)
&& Objects.equals(this.commitRpcTimeout, other.commitRpcTimeout)
&& Objects.equals(this.grpcKeepAliveTimeout, other.grpcKeepAliveTimeout);
}

Expand Down Expand Up @@ -301,6 +304,7 @@ public int hashCode() {
this.universeDomain,
this.grpcInterceptorProvider,
this.grpcKeepAliveTime,
commitRpcTimeout,

@gemini-code-assist gemini-code-assist Bot Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency with the other fields referenced in this method, please use this.commitRpcTimeout instead of commitRpcTimeout.

Suggested change
commitRpcTimeout,
this.commitRpcTimeout,

this.grpcKeepAliveTimeout);
}
}
Expand Down Expand Up @@ -522,6 +526,9 @@ Spanner createSpanner(SpannerPoolKey key, ConnectionOptions options) {
if (key.grpcKeepAliveTime != null) {
builder.setGrpcKeepAliveTime(key.grpcKeepAliveTime);
}
if (key.commitRpcTimeout != null) {
builder.setCommitRpcTimeout(key.commitRpcTimeout);
}
if (key.grpcKeepAliveTimeout != null) {
builder.setGrpcKeepAliveTimeout(key.grpcKeepAliveTimeout);
}
Expand Down
Loading