fix(bigtable): report swallowed batch flush errors and unacknowledged entries - #18122
Conversation
Sorry, something went wrong.
… mutate_rows entries Change-Id: I61a1444656d46f5b8e62aea0d16670dc9115659f
There was a problem hiding this comment.
Code Review
This pull request improves error handling in Google Cloud Bigtable by surfacing exceptions raised during asynchronous flushes in the batcher and ensuring that mutations without response entries in successfully closed streams are treated as incomplete rather than silently dropped. The review feedback points out a potential CancelledError when calling future.exception() on a cancelled future in _batch_completed_callback, suggesting a check for future.cancelled() to avoid unhandled exceptions.
Sorry, something went wrong.
Change-Id: Ib6727718ec0e39dd7aad1298532b5e2e64439070
daniel-sanche
left a comment
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
The unit tests seem to be running for more than an hour. Please can you check if it is related to the changes in this PR?
https://github.com/googleapis/google-cloud-python/actions/runs/31826948380/attempts/1?pr=18122
Sorry, something went wrong.
|
Adding do not merge to investigate the delay with the unit tests. In the last PR that was merged to packages/google-cloud-bigtable, tests only took 7 minutes https://github.com/googleapis/google-cloud-python/actions/runs/31780116767 
|
Sorry, something went wrong.
parthea
left a comment
There was a problem hiding this comment.
Something is wrong with tests. The duration of unit tests increased from ~ 7 minutes to > 1 hour
Sorry, something went wrong.
|
Hmm I wonder if the fork could be causing the issue? I'm looking into this |
Sorry, something went wrong.
|
The errors seems to be: raise exceptions.from_grpc_error(exc) from exc E google.api_core.exceptions.ResourceExhausted: 429 Quota exceeded for quota metric 'Instance and cluster metadata write requests' and limit 'Instance and cluster metadata write requests per day' of service 'bigtableadmin.googleapis.com' for consumer 'project_number:'. [reason: "RATE_LIMIT_EXCEEDED" |
Sorry, something went wrong.
|
I think my fix on the async client broke the test: The async MutateRows operation retries by tracking remaining_indices — the set of entries still needing a response. At the start of each attempt it resets remaining_indices = [] and only re-adds an entry when the server explicitly returns a non-OK status for it. This means that if the stream closes cleanly (gRPC OK) but simply omits the response entry for a mutation, that entry is neither re-queued nor recorded as an error — it's silently treated as successful. Since the server sends one response entry per request entry and the client cannot know the true outcome of an omitted one, assuming success risks reporting a write as applied when it may not have been (silent data loss). The correct behavior is to treat an omitted entry as an unknown outcome: retry it if idempotent, or surface it as a failure otherwise. The catch is that the existing V3 unit tests bake in the old assumption — their mock responses return an entry only for the mutations they care about and let the rest be "omitted = success" (e.g. _mock_response([DeadlineExceeded]) returns a single entry for a 3-entry request). So fixing the omitted-entry handling changes established behavior and breaks |
Sorry, something went wrong.
… batcher fix Change-Id: If379b98672c53286ab3dcb3d788006fbeb027b9c
🤖 I have created a release *beep* *boop* --- ## [2.42.0](google-cloud-bigtable-v2.41.0...google-cloud-bigtable-v2.42.0) (2026-08-19) ### Features * **bigtable:** client side metrics handlers ([#16760](#16760)) ([43b786c](43b786c)) * **bigtable:** initialize internal data client in classic client ([#18080](#18080)) ([92008e2](92008e2)) ### Bug Fixes * **bigtable:** data client should acknowledge all mutations in batch ([#18124](#18124)) ([e7f6a34](e7f6a34)) * **bigtable:** report swallowed batch flush errors and unacknowledged entries ([#18122](#18122)) ([2fe5bdc](2fe5bdc)) * **bigtable:** standardize client side metrics ([#17899](#17899)) ([a69e2cb](a69e2cb)) * **bigtable:** surface batcher flush errors and disable timer ([#18145](#18145)) ([fac536e](fac536e)) * bump grpcio to 1.59.0; require Python 3.10+ ([#17351](#17351)) ([a53487a](a53487a)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Add warning to upgrade to MutationsBatcher docstrings, related to #18122
In #18122, the tests would stall without any logs after `Running tests across 1 package(s) using 1 parallel worker(s)...` This is because the unit test job would wait until tests were complete before printing outputs, which isn't helpful when a job freezes This PR attempts to improve the usability of unit tests by: - showing live logs when PARALLEL_WORKERS is 1 - this is always the case for unit tests, but mypy runs in parallel - parallel jobs still buffer their writes, to avoid overlapping logs - fail-fast. If one nox session or package fails, the CI worker exits with an error immediately - use emojis to designate pass/fail (✅/❌), to make it logs to scan - added `--no-tags`, to avoid unneeded download and printing --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
report swallowed batch flush errors and unacknowledged mutate_rows entries.
Double check if there's any active requests left to avoid incomplete mutations.