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

@mutianfmutianf 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 ownerAugust 14, 2026 21:51

@gemini-code-assistgemini-code-assistBot 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.

@daniel-sanchedaniel-sanche changed the title feat(bigtable): fail V3 mutate_rows entries the server never acknowle…fix(bigtable): data client should acknowledge all mutations in batchAug 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-sanchedaniel-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-kokoroyoshi-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:mainAug 17, 2026
47 checks passed
@release-pleaserelease-pleaseBot mentioned this pull request Aug 17, 2026
@mutianf
mutianf deleted the bigtable-v3-mutate-rows-response-count branch August 17, 2026 20:50
Sign up for freeto 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

@mutianf@daniel-sanche@yoshi-kokoro