feat(bigtable): Option to disable direct access fallback - #14193
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to disable DirectPath fallback in ClientConfigurationManager using the environment variable CBT_DISABLE_DIRECT_ACCESS_FALLBACK or system property bigtable.internal.disable-direct-access-fallback. When enabled, configurations with DirectAccessWithFallback are normalized to DirectAccessOnly. The review feedback suggests renaming the internal field disableDirectPathFallback and its associated references and test method to disableDirectAccessFallback to ensure naming consistency with the configuration keys and protobuf fields.
|
|
||
| private final ClientConfiguration defaultConfig; | ||
| private final Optional<ClientConfiguration> overrideConfig; | ||
| private final boolean disableDirectPathFallback; |
Contributor
There was a problem hiding this comment.
To maintain consistency with the environment variable CBT_DISABLE_DIRECT_ACCESS_FALLBACK, the system property bigtable.internal.disable-direct-access-fallback, and the protobuf fields (DirectAccessOnly / DirectAccessWithFallback), please rename this field from disableDirectPathFallback to disableDirectAccessFallback.
Suggested change
| private final boolean disableDirectPathFallback; | |
| private final boolean disableDirectAccessFallback; |
Comment on lines
+186
to
+190
| this.disableDirectPathFallback = | ||
| Optional.ofNullable(System.getenv(DISABLE_DIRECT_ACCESS_FALLBACK_ENV_VAR)) | ||
| .orElseGet( | ||
| () -> sysProps.getProperty(DISABLE_DIRECT_ACCESS_FALLBACK_SYS_PROP_KEY, "false")) | ||
| .equalsIgnoreCase("true"); |
Contributor
There was a problem hiding this comment.
Rename the field assignment to disableDirectAccessFallback to match the renamed field.
Suggested change
| this.disableDirectPathFallback = | |
| Optional.ofNullable(System.getenv(DISABLE_DIRECT_ACCESS_FALLBACK_ENV_VAR)) | |
| .orElseGet( | |
| () -> sysProps.getProperty(DISABLE_DIRECT_ACCESS_FALLBACK_SYS_PROP_KEY, "false")) | |
| .equalsIgnoreCase("true"); | |
| this.disableDirectAccessFallback = | |
| Optional.ofNullable(System.getenv(DISABLE_DIRECT_ACCESS_FALLBACK_ENV_VAR)) | |
| .orElseGet( | |
| () -> sysProps.getProperty(DISABLE_DIRECT_ACCESS_FALLBACK_SYS_PROP_KEY, "false")) | |
| .equalsIgnoreCase("true"); |
Comment on lines
+420
to
+424
| if (disableDirectPathFallback | ||
| && builder | ||
| .getSessionConfiguration() | ||
| .getChannelConfiguration() | ||
| .hasDirectAccessWithFallback()) { |
Contributor
There was a problem hiding this comment.
Update the reference to use disableDirectAccessFallback instead of disableDirectPathFallback.
Suggested change
| if (disableDirectPathFallback | |
| && builder | |
| .getSessionConfiguration() | |
| .getChannelConfiguration() | |
| .hasDirectAccessWithFallback()) { | |
| if (disableDirectAccessFallback | |
| && builder | |
| .getSessionConfiguration() | |
| .getChannelConfiguration() | |
| .hasDirectAccessWithFallback()) { |
| } | ||
|
|
||
| @Test | ||
| void disableDirectPathFallbackTest() throws Exception { |
Contributor
There was a problem hiding this comment.
Rename the test method to disableDirectAccessFallbackTest to align with the renamed field and configuration keys.
Suggested change
| void disableDirectPathFallbackTest() throws Exception { | |
| void disableDirectAccessFallbackTest() throws Exception { |
mutianf
reviewed
Aug 27, 2026
| private static final Logger logger = Logger.getLogger(ClientConfigurationManager.class.getName()); | ||
|
|
||
| public static final String OVERRIDE_SYS_PROP_KEY = "bigtable.internal.client-config-override"; | ||
| public static final String DISABLE_DIRECT_ACCESS_FALLBACK_ENV_VAR = |
Contributor
There was a problem hiding this comment.
Do we need both env var and system property?
Contributor
Author
There was a problem hiding this comment.
I think it's okay to have both and convenient for users.
mutianf
approved these changes
Aug 27, 2026
mutianf
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
nimf
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
This was referenced Sep 4, 2026
lqiu96
pushed a commit
that referenced
this pull request
Sep 4, 2026
🤖 I have created a release *beep* *boop* --- <details><summary>1.91.0</summary> ## [1.91.0](v1.90.0...v1.91.0) (2026-09-04) ### ⚠ BREAKING CHANGES * **datalabeling:** remove java-datalabeling library ([#14189](#14189)) * **datacatalog:** remove java-datacatalog library ([#14178](#14178)) ### Features * **apptopology:** onboard a new library ([#14204](#14204)) ([f871a07](f871a07)) * **bigtable:** Option to disable direct access fallback ([#14193](#14193)) ([5b6a974](5b6a974)) * **datacatalog:** remove java-datacatalog library ([#14178](#14178)) ([2910f63](2910f63)) * **datalabeling:** remove java-datalabeling library ([#14189](#14189)) ([334d35a](334d35a)), refs [#14176](#14176) * **gax:** add getSingleHeader to HttpHeadersUtils ([#14137](#14137)) ([f114422](f114422)) * **gax:** add ResumableUploadClient.startUpload() and supporting types ([#14138](#14138)) ([d7e2be2](d7e2be2)) * **gax:** add ResumableUploadResponseParser ([#14135](#14135)) ([e1a0b17](e1a0b17)) * **gax:** allow non-JSON HttpContent and absolute request URLs in HttpRequestRunnable ([#14134](#14134)) ([ac4a49a](ac4a49a)) * **gax:** implement queryStatusCallable for resumable uploads ([#14155](#14155)) ([67719c0](67719c0)) * **gax:** implement startUploadCallable for resumable uploads ([#14139](#14139)) ([4e65b6c](4e65b6c)) * **gax:** implement uploadChunkCallable for resumable uploads ([#14140](#14140)) ([1c643de](1c643de)) * **google/shopping/merchant/loyaltycustomers/v1:** onboard new library ([#14257](#14257)) ([590a51c](590a51c)) * **grpc-gcp:** drain scaled-down channels ([#14216](#14216)) ([bbbd18c](bbbd18c)) * **grpc-gcp:** move scale-up to background worker ([#14206](#14206)) ([41f0a2e](41f0a2e)) * **grpc-gcp:** penalize retryable channel errors ([#14219](#14219)) ([829872e](829872e)) * **grpc-gcp:** prime scaled channels before publish ([#14232](#14232)) ([dd75645](dd75645)) * **spanner:** add client-level CallContextConfigurator to SpannerOptions ([#14256](#14256)) ([eef74d2](eef74d2)) * **spanner:** prime scaled-up dynamic channel pool channels with SELECT 1 ([#14254](#14254)) ([4d26645](4d26645)) ### Bug Fixes * **auth:** refine JSpecify nullability annotations for external account credentials ([#14164](#14164)) ([385e1f2](385e1f2)) * **bigquery-jdbc:** resolve `statementType` via `getJob` fallback to avoid post-execution dry run for DDL ([#14265](#14265)) ([d38d9f0](d38d9f0)) * **bigtable:** truncate client generated timestamps on the emulator c… ([#14234](#14234)) ([94e680f](94e680f)) * **gax:** propagate structured LRO error details to ApiException ([#14022](#14022)) ([865a15b](865a15b)) * **grpc-gcp:** correct channel lifecycle bookkeeping ([#14196](#14196)) ([3df32ba](3df32ba)) * **grpc-gcp:** prevent hot-channel skew ([#14198](#14198)) ([07a7505](07a7505)) * **java-sql:** temp fix for lint and skip generate ([#14005](#14005)) ([b6f73db](b6f73db)) * **spanner:** prioritize leader replica for read-write transactions in location-aware routing ([#14195](#14195)) ([6383f81](6383f81)) * **spanner:** route ExecuteBatchDml through transaction affinity endpoint ([#14192](#14192)) ([5e9e3cd](5e9e3cd)) * **storage:** resolve GraalVM Native Image test failure for java-storage ([#14226](#14226)) ([93e0920](93e0920)) ### Performance Improvements * **bigquery-jdbc:** eliminate sync getJob RPC call ([#14197](#14197)) ([ccf12b0](ccf12b0)) * **ci:** replace mvn help:evaluate with native bash and sed extraction ([#14218](#14218)) ([744a522](744a522)) ### Dependencies * Upgrade grpc-java to 1.83.0 ([#13967](#13967)) ([0cdc695](0cdc695)) </details> --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.