Skip to content

feat(bigtable): attach client config UUID header to GetClientConfigur… - #13972

Closed
mutianf wants to merge 1 commit into
googleapis:mainfrom
mutianf:uuid
Closed

feat(bigtable): attach client config UUID header to GetClientConfigur…#13972
mutianf wants to merge 1 commit into
googleapis:mainfrom
mutianf:uuid

Conversation

@mutianf

@mutianf mutianf commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

…ationRequest

Generate a UUID when the session client is initialized and attach it via the bigtable-client-config-uuid header on every GetClientConfigurationRequest (the initial fetch and all refresh polls) so the server can correlate requests from the same client instance.

…ationRequest

Generate a UUID when the session client is initialized and attach it via
the bigtable-client-config-uuid header on every GetClientConfigurationRequest
(the initial fetch and all refresh polls) so the server can correlate
requests from the same client instance.
@mutianf
mutianf requested review from a team as code owners July 31, 2026 14:25

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a unique client UUID header (bigtable-client-config-uuid) to ClientConfigurationManager that is attached to all GetClientConfigurationRequest calls (both initial and refresh requests) to allow the server to correlate requests from the same client. It also adds a unit test to verify this behavior. The reviewer suggested adding a timeout to the get() call on the future in the test to prevent potential indefinite hangs.

assertThat(ClientConfigurationManager.CLIENT_UUID_KEY.name()).startsWith("bigtable-");

// Fetch the initial config and capture the header sent with it.
manager.start().get();

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

Using an unbounded get() on a Future in tests can cause the test suite to hang indefinitely if the future never completes (e.g., due to a deadlock or a bug in the production code). It is highly recommended to specify a timeout, such as get(10, java.util.concurrent.TimeUnit.SECONDS), to ensure the test fails fast and provides a clear error message.

Suggested change
manager.start().get();
manager.start().get(10, java.util.concurrent.TimeUnit.SECONDS);

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces a unique client session identifier (UUID) in ClientConfigurationManager that is attached as a gRPC metadata header (bigtable-client-config-uuid) to all initial and refresh configuration requests. This allows the server to correlate requests originating from the same client instance. Corresponding unit tests have been added to verify that the UUID is correctly generated and sent with both initial and refresh requests. There are no review comments, and we have no feedback to provide.

@mutianf

mutianf commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Will wrap it in a proto and send it for all requests.

@mutianf mutianf closed this Jul 31, 2026
@mutianf
mutianf deleted the uuid branch July 31, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant