Skip to content

feat: make grpc-gcp default enabled #4239

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: make grpc-gcp default enabled #4239

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79c0c7e
feat: make grpc-gcp default enabled
rahul2393 Nov 20, 2025
957b147
fix test
rahul2393 Nov 20, 2025
a628194
add logs
rahul2393 Nov 21, 2025
02820b6
pin grpc-gcp version
rahul2393 Nov 21, 2025
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
23 changes: 8 additions & 15 deletions 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 @@ -151,7 +151,6 @@ public class SpannerOptions extends ServiceOptions<Spanner, SpannerOptions> {
private final InstanceAdminStubSettings instanceAdminStubSettings;
private final DatabaseAdminStubSettings databaseAdminStubSettings;
private final Duration partitionedDmlTimeout;
private final boolean grpcGcpExtensionEnabled;
private final GcpManagedChannelOptions grpcGcpOptions;
private final boolean autoThrottleAdministrativeRequests;
private final RetrySettings retryAdministrativeRequestsSettings;
Expand Down Expand Up @@ -798,7 +797,6 @@ protected SpannerOptions(Builder builder) {
throw SpannerExceptionFactory.newSpannerException(e);
}
partitionedDmlTimeout = builder.partitionedDmlTimeout;
grpcGcpExtensionEnabled = builder.grpcGcpExtensionEnabled;
grpcGcpOptions = builder.grpcGcpOptions;
autoThrottleAdministrativeRequests = builder.autoThrottleAdministrativeRequests;
retryAdministrativeRequestsSettings = builder.retryAdministrativeRequestsSettings;
Expand Down Expand Up @@ -1025,7 +1023,6 @@ public static class Builder
private DatabaseAdminStubSettings.Builder databaseAdminStubSettingsBuilder =
DatabaseAdminStubSettings.newBuilder();
private Duration partitionedDmlTimeout = Duration.ofHours(2L);
private boolean grpcGcpExtensionEnabled = false;
private GcpManagedChannelOptions grpcGcpOptions;
private RetrySettings retryAdministrativeRequestsSettings =
DEFAULT_ADMIN_REQUESTS_LIMIT_EXCEEDED_RETRY_SETTINGS;
Expand Down Expand Up @@ -1097,7 +1094,6 @@ protected Builder() {
this.instanceAdminStubSettingsBuilder = options.instanceAdminStubSettings.toBuilder();
this.databaseAdminStubSettingsBuilder = options.databaseAdminStubSettings.toBuilder();
this.partitionedDmlTimeout = options.partitionedDmlTimeout;
this.grpcGcpExtensionEnabled = options.grpcGcpExtensionEnabled;
this.grpcGcpOptions = options.grpcGcpOptions;
this.autoThrottleAdministrativeRequests = options.autoThrottleAdministrativeRequests;
this.retryAdministrativeRequestsSettings = options.retryAdministrativeRequestsSettings;
Expand Down Expand Up @@ -1268,8 +1264,8 @@ public Builder setRetrySettings(RetrySettings retrySettings) {
* builder
* .getSpannerStubSettingsBuilder()
* .applyToAllUnaryMethods(
* new ApiFunction&lt;UnaryCallSettings.Builder&lt;?, ?&gt;, Void&gt;() {
* public Void apply(Builder&lt;?, ?&gt; input) {
* new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
* public Void apply(Builder<?, ?> input) {
* input.setRetrySettings(retrySettings);
* return null;
* }
Expand All @@ -1296,8 +1292,8 @@ public SpannerStubSettings.Builder getSpannerStubSettingsBuilder() {
* builder
* .getInstanceAdminStubSettingsBuilder()
* .applyToAllUnaryMethods(
* new ApiFunction&lt;UnaryCallSettings.Builder&lt;?, ?&gt;, Void&gt;() {
* public Void apply(Builder&lt;?, ?&gt; input) {
* new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
* public Void apply(Builder<?, ?> input) {
* input.setRetrySettings(retrySettings);
* return null;
* }
Expand All @@ -1324,8 +1320,8 @@ public InstanceAdminStubSettings.Builder getInstanceAdminStubSettingsBuilder() {
* builder
* .getDatabaseAdminStubSettingsBuilder()
* .applyToAllUnaryMethods(
* new ApiFunction&lt;UnaryCallSettings.Builder&lt;?, ?&gt;, Void&gt;() {
* public Void apply(Builder&lt;?, ?&gt; input) {
* new ApiFunction<UnaryCallSettings.Builder<?, ?>, Void>() {
* public Void apply(Builder<?, ?> input) {
* input.setRetrySettings(retrySettings);
* return null;
* }
Expand Down Expand Up @@ -1573,14 +1569,12 @@ public Builder enableGrpcGcpExtension() {
* Multiplexed sessions are not supported for gRPC-GCP.
*/
public Builder enableGrpcGcpExtension(GcpManagedChannelOptions options) {
this.grpcGcpExtensionEnabled = true;
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

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

medium

This line is correctly removed. However, with gRPC-GCP now enabled by default, the method name enableGrpcGcpExtension is misleading as it only configures options. Consider deprecating this method and introducing a new one with a more descriptive name like setGrpcGcpOptions to improve API clarity.

this.grpcGcpOptions = options;
return this;
}

/** Disables gRPC-GCP extension. */
public Builder disableGrpcGcpExtension() {
this.grpcGcpExtensionEnabled = false;
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

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

medium

While removing this line is correct as part of making gRPC-GCP always enabled, the disableGrpcGcpExtension() method now has no effect. This can be misleading for users. The method should be deprecated to clearly communicate that it's a no-op and the extension cannot be disabled.

It would be best to modify the method on the right side to be:

    /**
     * Disables gRPC-GCP extension.
     *
     * @deprecated gRPC-GCP is now enabled by default and can no longer be disabled. This method is a no-op.
     */
    @java.lang.Deprecated
    public Builder disableGrpcGcpExtension() {
      return this;
    }

return this;
}

Expand Down Expand Up @@ -1793,8 +1787,7 @@ public SpannerOptions build() {
credentials = environment.getDefaultExperimentalHostCredentials();
}
if (this.numChannels == null) {
this.numChannels =
this.grpcGcpExtensionEnabled ? GRPC_GCP_ENABLED_DEFAULT_CHANNELS : DEFAULT_CHANNELS;
this.numChannels = GRPC_GCP_ENABLED_DEFAULT_CHANNELS;
}

synchronized (lock) {
Expand Down Expand Up @@ -1989,7 +1982,7 @@ public Duration getPartitionedDmlTimeoutDuration() {
}

public boolean isGrpcGcpExtensionEnabled() {
return grpcGcpExtensionEnabled;
return true;
}

public GcpManagedChannelOptions getGrpcGcpOptions() {
Expand Down
28 changes: 8 additions & 20 deletions google-cloud-spanner/src/main/java/com/google/cloud/spanner/spi/v1/GapicSpannerRpc.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ public class GapicSpannerRpc implements SpannerRpc {
private final boolean leaderAwareRoutingEnabled;
private final boolean endToEndTracingEnabled;
private final int numChannels;
private final boolean isGrpcGcpExtensionEnabled;

private final GrpcCallContext baseGrpcCallContext;

Expand Down Expand Up @@ -335,7 +334,6 @@ public GapicSpannerRpc(final SpannerOptions options) {
this.leaderAwareRoutingEnabled = options.isLeaderAwareRoutingEnabled();
this.endToEndTracingEnabled = options.isEndToEndTracingEnabled();
this.numChannels = options.getNumChannels();
this.isGrpcGcpExtensionEnabled = options.isGrpcGcpExtensionEnabled();
this.baseGrpcCallContext = createBaseCallContext();

if (initializeStubs) {
Expand Down Expand Up @@ -592,10 +590,6 @@ private static GcpManagedChannelOptions grpcGcpOptionsWithMetrics(SpannerOptions
private static void maybeEnableGrpcGcpExtension(
InstantiatingGrpcChannelProvider.Builder defaultChannelProviderBuilder,
final SpannerOptions options) {
if (!options.isGrpcGcpExtensionEnabled()) {
return;
}

final String jsonApiConfig = parseGrpcGcpApiConfig();
final GcpManagedChannelOptions grpcGcpOptions = grpcGcpOptionsWithMetrics(options);

Expand Down Expand Up @@ -2037,20 +2031,14 @@ <ReqT, RespT> GrpcCallContext newCallContext(
GrpcCallContext context = this.baseGrpcCallContext;
Long affinity = options == null ? null : Option.CHANNEL_HINT.getLong(options);
if (affinity != null) {
if (this.isGrpcGcpExtensionEnabled) {
// Set channel affinity in gRPC-GCP.
// Compute bounded channel hint to prevent gRPC-GCP affinity map from getting unbounded.
int boundedChannelHint = affinity.intValue() % this.numChannels;
context =
context.withCallOptions(
context
.getCallOptions()
.withOption(
GcpManagedChannel.AFFINITY_KEY, String.valueOf(boundedChannelHint)));
} else {
// Set channel affinity in GAX.
context = context.withChannelAffinity(affinity.intValue());
}
// Set channel affinity in gRPC-GCP.
// Compute bounded channel hint to prevent gRPC-GCP affinity map from getting unbounded.
int boundedChannelHint = affinity.intValue() % this.numChannels;
context =
context.withCallOptions(
context
.getCallOptions()
.withOption(GcpManagedChannel.AFFINITY_KEY, String.valueOf(boundedChannelHint)));
}
if (options != null) {
// TODO(@odeke-em): Infer the affinity if it doesn't match up with in the request-id.
Expand Down
8 changes: 5 additions & 3 deletions google-cloud-spanner/src/test/java/com/google/cloud/spanner/SpannerOptionsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,9 +1100,10 @@ public void testDefaultNumChannelsWithGrpcGcpExtensionDisabled() {
SpannerOptions.newBuilder()
.setProjectId("test-project")
.setCredentials(NoCredentials.getInstance())
.disableGrpcGcpExtension()
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

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

medium

The name of this test, testDefaultNumChannelsWithGrpcGcpExtensionDisabled, is now misleading because the gRPC-GCP extension can no longer be disabled. Since this test now verifies the behavior when the no-op disableGrpcGcpExtension() method is called, consider renaming it to something like testDefaultNumChannelsWhenDisableGrpcGcpExtensionIsCalled for clarity.

.build();

assertEquals(SpannerOptions.DEFAULT_CHANNELS, options.getNumChannels());
assertEquals(SpannerOptions.GRPC_GCP_ENABLED_DEFAULT_CHANNELS, options.getNumChannels());
}

@Test
Expand Down Expand Up @@ -1135,9 +1136,10 @@ public void testNumChannelsWithGrpcGcpExtensionEnabled() {

@Test
public void checkCreatedInstanceWhenGrpcGcpExtensionDisabled() {
SpannerOptions options = SpannerOptions.newBuilder().setProjectId("test-project").build();
SpannerOptions options =
SpannerOptions.newBuilder().setProjectId("test-project").disableGrpcGcpExtension().build();
Comment on lines +1139 to +1140
Copy link

@gemini-code-assist gemini-code-assist bot Nov 20, 2025

Choose a reason for hiding this comment

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

medium

The name of this test, checkCreatedInstanceWhenGrpcGcpExtensionDisabled, is now misleading as the gRPC-GCP extension is always enabled. To improve clarity, consider renaming the test to reflect that it's verifying behavior when the no-op disableGrpcGcpExtension() is called, for example: checkCreatedInstanceWhenDisableGrpcGcpExtensionIsCalled.

SpannerOptions options1 = options.toBuilder().build();
assertEquals(false, options.isGrpcGcpExtensionEnabled());
assertEquals(true, options.isGrpcGcpExtensionEnabled());
assertEquals(options.isGrpcGcpExtensionEnabled(), options1.isGrpcGcpExtensionEnabled());

Spanner spanner1 = options.getService();
Expand Down
Loading