Skip to content

feat(bigtable): fall back to native when the accelerator fails health checks - #18690

Open
mutianf wants to merge 5 commits into
bigtable-accelfrom
accel9
Open

mutianf wants to merge 5 commits into
bigtable-accelfrom
accel9

Conversation

@mutianf

@mutianf mutianf commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕


Stack created with GitHub Stacks CLI • Give Feedback 💬

@mutianf
mutianf added this pull request to stack #18691 October 13, 2026 20:07
@mutianf
mutianf requested a review from a team as a code owner October 13, 2026 20:07
@mutianf
mutianf requested a review from sushanb October 13, 2026 20:07

@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 background health monitor for the accelerator daemon in the Google Cloud Bigtable Python client. It adds a hand-rolled gRPC health check codec to avoid external dependencies, updates the AcceleratorBreaker to support a reversible 'degraded' state alongside the permanent 'tripped' state, and implements a background polling loop (_AcceleratorHealthMonitor) in both async and sync clients to dynamically route traffic away from the daemon if it becomes unresponsive. Comprehensive unit tests are also added to verify the codec, state machine, and polling behavior. There are no review comments, so we have no feedback to provide.

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/client.py Outdated
Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/client.py Outdated

@sushanb sushanb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM. All three of my prior inline suggestions landed verbatim (metrics.close placement, dead except branch removed, Event-pair breaker keeping bypass() lock-free). The health-monitor rework (ThreadPool→daemon Thread, weakref.finalize for request_stop, bounded-wait close) is a solid tightening — nothing new to flag.

Base automatically changed from accel8 to bigtable-accel October 23, 2026 16:33
… checks

fix(bigtable): lock all reads and writes in AcceleratorBreaker

Change-Id: I9d1daffbe086788481d6802cec035a3d4d284729
…, clear stale auth_secret on teardown

Change-Id: I02c7ecb2d9ac5aa082bec0dc3325938b621bd210
Change-Id: I6665fec71ca24efe8ec47f95a71de6dda59abe5e
…), and breaker lock

Change-Id: I21cf617dbea7aa85dc7b0d093e15c1ea22c7f95c
…degraded on exit

Change-Id: I99ce4c0ffb7b12a3f1746801ddcadc4294f61b17

This branch has not been deployed

No deployments
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.

2 participants