Skip to content

fix(bigtable): data client should acknowledge all mutations in batch - #18124

Merged
daniel-sanche merged 2 commits into
googleapis:mainfrom
mutianf:bigtable-v3-mutate-rows-response-count
Aug 17, 2026
Merged

fix(bigtable): data client should acknowledge all mutations in batch#18124
daniel-sanche merged 2 commits into
googleapis:mainfrom
mutianf:bigtable-v3-mutate-rows-response-count

Conversation

@mutianf

@mutianf mutianf commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

fail V3 mutate_rows entries the server never acknowledged

…dged

Change-Id: I9a166818c5829b446553fc1427d1f1d1ec078d0d
@mutianf
mutianf requested a review from a team as a code owner August 14, 2026 21:51

@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 response completeness check to both the async and sync implementations of _mutate_rows.py, ensuring that any mutation entries not acknowledged by the server are explicitly failed with a ClientError rather than being silently treated as successful. While the added test coverage is thorough, the reviewer correctly identified a critical bug in both implementations: when mutations are retried across multiple attempts, obsolete acknowledgments and errors from previous attempts are not cleared, which prevents the completeness check from correctly identifying silently dropped entries in subsequent attempts. To resolve this, the state of retried entries should be cleared at the start of each attempt.

Comment thread packages/google-cloud-bigtable/google/cloud/bigtable/data/_async/_mutate_rows.py Outdated
@daniel-sanche daniel-sanche changed the title feat(bigtable): fail V3 mutate_rows entries the server never acknowle… fix(bigtable): data client should acknowledge all mutations in batch Aug 14, 2026
@daniel-sanche

Copy link
Copy Markdown
Contributor

Did your original approach not work? I thought that seemed like a good solution.

Gemini pointed out some issues with the new approach. It could be difficult to keep a global _acknowledged_indices map up-to-date, because each attempt is another chance to drop a mutation. It would be better if we could keep all the per-attempt state within _run_attempt

…ead of a count check

Change-Id: I4ce3e38536a6aa6270806868074eb51c7832653a
@daniel-sanche daniel-sanche added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 17, 2026
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Aug 17, 2026
@daniel-sanche
daniel-sanche merged commit e7f6a34 into googleapis:main Aug 17, 2026
47 checks passed
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.

3 participants