From 31a57f4ef1a8c75cebbe6b625ebdbdf730aafc1f Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 1 Jul 2026 18:50:46 +0000 Subject: [PATCH 1/2] fix(bigtable): honour session_load override when server-returned session_load is 0 ClientConfigurationManager.normalizeConfig() overlays the sys-prop override into the builder and then guards a "clear session_configuration when disabled" branch on session_load. The guard was reading cfg (the original server response), not the merged builder, so any client that supplied a nonzero session_load via the bigtable.internal.client-config-override sys-prop still had its entire session_configuration wiped whenever the server returned session_load=0. In practice this meant such clients ran with no sessions pre-started, DynamicPicker logged LOADBALANCINGSTRATEGY_NOT_SET, and every RPC fell through to the old stack. Fix reads from the builder so the override is respected. Adds a regression test covering server session_load=0 + override session_load>0. --- .../util/ClientConfigurationManager.java | 6 ++- .../util/ClientConfigurationManagerTest.java | 37 +++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java index 8f273dc34263..c7733a3da69b 100644 --- a/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java +++ b/java-bigtable/google-cloud-bigtable/src/main/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManager.java @@ -395,8 +395,10 @@ private ClientConfiguration normalizeConfig(ClientConfiguration cfg) { // Inject overrides overrideConfig.ifPresent(builder::mergeFrom); - // When sessions are disabled make sure to clear out the config - if (cfg.getSessionConfiguration().getSessionLoad() == 0) { + // When sessions are disabled make sure to clear out the config. Read from the builder, not + // cfg, so that a nonzero session_load supplied via the override sys-prop is honoured even when + // the server-returned config has session_load=0. + if (builder.getSessionConfiguration().getSessionLoad() == 0) { builder.clearSessionConfiguration(); return builder.build(); } diff --git a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java index 421e442831b8..f5eb07a4bcd5 100644 --- a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java +++ b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java @@ -196,6 +196,43 @@ void testDisabledSessions() throws ExecutionException, InterruptedException { .isEqualToDefaultInstance(); } + @Test + void testDisabledSessionsOverriddenBySysProp() + throws ExecutionException, InterruptedException, IOException { + // Server returns session_load=0 (sessions disabled from the server's perspective)… + ClientConfiguration.Builder disabledBuilder = manager.getDefaultConfig().toBuilder(); + disabledBuilder.getSessionConfigurationBuilder().setSessionLoad(0); + service.config.set(disabledBuilder.build()); + + // …but a sys-prop override forces session_load>0 to opt this client into sessions. + manager.close(); + String clientConfigOverrides = + TextFormat.printer() + .printToString( + ClientConfiguration.newBuilder() + .setSessionConfiguration( + SessionClientConfiguration.newBuilder().setSessionLoad(0.75f)) + .build()); + Properties sysProps = new Properties(); + sysProps.setProperty(ClientConfigurationManager.OVERRIDE_SYS_PROP_KEY, clientConfigOverrides); + manager = + new ClientConfigurationManager( + sysProps, FEATURE_FLAGS, CLIENT_INFO, channelProvider, noopDebugTracer, mockExecutor); + + ClientConfiguration resolvedConfig = manager.start().get(); + + // The override must be honoured: session_load and the default SessionPoolConfiguration must + // survive normalization instead of being cleared as if sessions were disabled. + assertThat(resolvedConfig.getSessionConfiguration().getSessionLoad()).isEqualTo(0.75f); + assertThat(resolvedConfig.getSessionConfiguration().getSessionPoolConfiguration()) + .isEqualTo( + manager + .getDefaultConfig() + .getSessionConfiguration() + .getSessionPoolConfiguration()); + assertThat(manager.areSessionsRequired()).isTrue(); + } + @Deprecated @Test void testMigrateSessionPool() throws ExecutionException, InterruptedException { From a8673993ad338e19bd773924a0bdc01a25d8a455 Mon Sep 17 00:00:00 2001 From: Igor Bernstein Date: Wed, 1 Jul 2026 15:20:45 -0400 Subject: [PATCH 2/2] fmt --- .../v2/internal/util/ClientConfigurationManagerTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java index f5eb07a4bcd5..d9ca7aaa9dc6 100644 --- a/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java +++ b/java-bigtable/google-cloud-bigtable/src/test/java/com/google/cloud/bigtable/data/v2/internal/util/ClientConfigurationManagerTest.java @@ -226,10 +226,7 @@ void testDisabledSessionsOverriddenBySysProp() assertThat(resolvedConfig.getSessionConfiguration().getSessionLoad()).isEqualTo(0.75f); assertThat(resolvedConfig.getSessionConfiguration().getSessionPoolConfiguration()) .isEqualTo( - manager - .getDefaultConfig() - .getSessionConfiguration() - .getSessionPoolConfiguration()); + manager.getDefaultConfig().getSessionConfiguration().getSessionPoolConfiguration()); assertThat(manager.areSessionsRequired()).isTrue(); }