Skip to content

chore(bigtable): remove legacy session/channel config migration in ClientConfigurationManager #14891

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

Merged
igorbernstein2 merged 1 commit into from
Nov 2, 2026
Merged

chore(bigtable): remove legacy session/channel config migration in ClientConfigurationManager #14891

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
24 changes: 0 additions & 24 deletions ...main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,33 +386,9 @@ public void onClose(Status status, Metadata trailers) {
return future;
}

@SuppressWarnings("deprecation")
private ClientConfiguration normalizeConfig(ClientConfiguration cfg) {
ClientConfiguration.Builder builder = cfg.toBuilder();

// TODO: remove this once server is updated
// Patch SessionPoolConfiguration (migrating LoadBalancingOptions)
if (!cfg.getSessionConfiguration().hasSessionPoolConfiguration()) {
builder
.getSessionConfigurationBuilder()
.clearLoadBalancingOptions()
.setSessionPoolConfiguration(
defaultConfig.getSessionConfiguration().getSessionPoolConfiguration().toBuilder()
.setLoadBalancingOptions(cfg.getSessionConfiguration().getLoadBalancingOptions())
.build());
} else {
// ignore LoadBalancing options when we migrated to SessionPoolConfig
builder.getSessionConfigurationBuilder().clearLoadBalancingOptions();
}
// TODO: remove this once server is updated
// Patch ChannelPoolConfiguration
if (!cfg.getSessionConfiguration().hasChannelConfiguration()) {
builder
.getSessionConfigurationBuilder()
.setChannelConfiguration(
defaultConfig.getSessionConfiguration().getChannelConfiguration());
}

// Patch debug tag level
switch (cfg.getTelemetryConfiguration().getDebugTagLevel()) {
case UNRECOGNIZED:
Expand Down
50 changes: 0 additions & 50 deletions .../java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.google.bigtable.v2.GetClientConfigurationRequest;
import com.google.bigtable.v2.LoadBalancingOptions;
import com.google.bigtable.v2.LoadBalancingOptions.LeastInFlight;
import com.google.bigtable.v2.LoadBalancingOptions.PeakEwma;
import com.google.bigtable.v2.SessionClientConfiguration;
import com.google.bigtable.v2.SessionClientConfiguration.ChannelPoolConfiguration.DirectAccessWithFallback;
import com.google.bigtable.v2.SessionClientConfiguration.SessionPoolConfiguration;
Expand Down Expand Up @@ -296,55 +295,6 @@ void testDisabledSessionsOverriddenBySysProp()
assertThat(manager.areSessionsRequired()).isTrue();
}

@Deprecated
@Test
void testMigrateSessionPool() throws ExecutionException, InterruptedException {
ClientConfiguration defaultConfig = manager.getDefaultConfig();

// Override LoadBalancingOptions
LoadBalancingOptions loadBalancingOptions =
LoadBalancingOptions.newBuilder()
.setPeakEwma(PeakEwma.newBuilder().setRandomSubsetSize(0))
.build();

ClientConfiguration.Builder legacyBuilder = defaultConfig.toBuilder();

// Enable sessions
legacyBuilder
.getSessionConfigurationBuilder()
.setSessionDiversionConfiguration(
SessionDiversionConfiguration.newBuilder()
.setGlobalDiversion(
SessionScopeDiversionConfiguration.newBuilder().setSessionLoad(0.1f)));

// Now patch default config to be a legacy config
// During migration, LoadBalancingOptions are a toplevel config, so clear out the
// SessionPoolConfiguration and set the toplevel option
legacyBuilder
.getSessionConfigurationBuilder()
.setLoadBalancingOptions(loadBalancingOptions)
.clearSessionPoolConfiguration()
.clearChannelConfiguration();
ClientConfiguration legacyCfg = legacyBuilder.build();

service.config.set(legacyCfg);
// Fetch initial config
ClientConfiguration resolvedConfig = manager.start().get();

// Make sure that SessionPoolConfiguration got patched with SessionPool & ChannelPool configs
// And ensure that the toplevel LoadBalancingOptions got migrated
ClientConfiguration.Builder expectedConfig = legacyCfg.toBuilder();
expectedConfig
.getSessionConfigurationBuilder()
.clearLoadBalancingOptions()
.setSessionPoolConfiguration(
defaultConfig.getSessionConfiguration().getSessionPoolConfiguration().toBuilder()
.setLoadBalancingOptions(loadBalancingOptions))
.setChannelConfiguration(defaultConfig.getSessionConfiguration().getChannelConfiguration());

assertThat(resolvedConfig).isEqualTo(expectedConfig.build());
}

@Test
void testSysPropOverride() throws Exception {
manager.close();
Expand Down
27 changes: 15 additions & 12 deletions ...ud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/stub/SessionDeadlineTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
import com.google.bigtable.v2.GetClientConfigurationRequest;
import com.google.bigtable.v2.OpenSessionResponse;
import com.google.bigtable.v2.PeerInfo;
import com.google.bigtable.v2.SessionClientConfiguration;
import com.google.bigtable.v2.SessionDiversionConfiguration;
import com.google.bigtable.v2.SessionRequest;
import com.google.bigtable.v2.SessionResponse;
import com.google.bigtable.v2.SessionScopeDiversionConfiguration;
import com.google.cloud.bigtable.data.v2.BigtableDataSettings;
import com.google.cloud.bigtable.data.v2.internal.util.ClientConfigurationManager;
import com.google.cloud.bigtable.data.v2.models.Query;
import com.google.cloud.bigtable.data.v2.models.Row;
import com.google.cloud.bigtable.data.v2.models.RowMutation;
Expand Down Expand Up @@ -254,17 +254,20 @@ public void shutdown() {
public void getClientConfiguration(
GetClientConfigurationRequest request,
StreamObserver<ClientConfiguration> responseObserver) {
responseObserver.onNext(
ClientConfiguration.newBuilder()
.setSessionConfiguration(
SessionClientConfiguration.newBuilder()
.setSessionDiversionConfiguration(
SessionDiversionConfiguration.newBuilder()
.setGlobalDiversion(
SessionScopeDiversionConfiguration.newBuilder()
.setSessionLoad(1)))
.build())
.build());
ClientConfiguration.Builder builder;
try {
builder = ClientConfigurationManager.loadDefault().toBuilder();
} catch (IOException e) {
responseObserver.onError(e);
return;
}
Comment on lines +257 to +263

@gemini-code-assist gemini-code-assist Bot Nov 2, 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

Loading and parsing the default configuration on every gRPC request in the mock service is inefficient because ClientConfigurationManager.loadDefault() performs classpath resource I/O and protobuf parsing. To optimize this, consider loading the default configuration once and caching it in a private static final field within the class. Additionally, propagating a raw IOException directly to responseObserver.onError is less idiomatic in gRPC; wrapping it in a StatusRuntimeException (e.g., Status.INTERNAL.withCause(e).asException()) is preferred.

builder
.getSessionConfigurationBuilder()
.setSessionDiversionConfiguration(
SessionDiversionConfiguration.newBuilder()
.setGlobalDiversion(
SessionScopeDiversionConfiguration.newBuilder().setSessionLoad(1)));
responseObserver.onNext(builder.build());
responseObserver.onCompleted();
}

Expand Down
Loading