fix(vttablet): scope 2PC redo rows by tablet database - #20764
fix(vttablet): scope 2PC redo rows by tablet database#20764cengdall-blaze wants to merge 1 commit into
Conversation
Add `db_name` to the 2PC redo sidecar tables and include it in the primary keys so multiple vttablets sharing a mysqld instance do not collide when preparing the same distributed transaction DTID. Record the tablet DB name when saving redo state/statements, and scope redo reads, updates, deletes, and unresolved counts by that DB name. Keep fallback visibility for rows with an empty db_name so pre-upgrade prepared redo records can still be recovered or cleaned up. Update tabletserver, end-to-end, vtexplain, and binlog test expectations for the new redo table shape. Fixes vitessio#19493 Signed-off-by: Chase Engdall <173094250+cengdall-blaze@users.noreply.github.com>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
There was a problem hiding this comment.
Pull request overview
Scopes 2PC redo metadata by tablet database to prevent DTID collisions on shared MySQL instances.
Changes:
- Adds
db_nameto redo schemas and primary keys. - Scopes redo operations while retaining legacy-row fallback.
- Updates tests, documentation, and expected query output.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
go/vt/vttablet/tabletserver/twopc.go |
Implements database-scoped redo operations. |
go/vt/vttablet/tabletserver/twopc_test.go |
Tests scoped query generation. |
go/vt/vttablet/tabletserver/tx_engine_test.go |
Updates failed-redo query expectations. |
go/vt/vttablet/tabletserver/tabletserver_test.go |
Updates supported redo reads. |
go/vt/vttablet/tabletserver/query_executor_test.go |
Updates executor query fixtures. |
go/vt/vttablet/tabletserver/dt_executor_test.go |
Updates redo lifecycle fixtures. |
go/vt/vttablet/endtoend/twopc/prepare_test.go |
Scopes redo locking expectations. |
go/vt/vtexplain/vtexplain_vttablet.go |
Updates simulated sidecar schemas. |
go/vt/vtexplain/testdata/twopc-output/updatesharded-output.txt |
Updates generated 2PC output. |
go/vt/vtexplain/testdata/twopc-output/options-output.txt |
Updates generated option output. |
go/vt/vtexplain/testdata/twopc-output/insertsharded-output.txt |
Updates generated insert output. |
go/vt/vtexplain/testdata/twopc-output/deletesharded-output.txt |
Updates generated delete output. |
go/vt/sidecardb/schema/twopc/redo_statement.sql |
Adds statement database scope. |
go/vt/sidecardb/schema/twopc/redo_state.sql |
Adds state database scope. |
go/test/endtoend/transaction/twopc/main_test.go |
Adjusts binlog column extraction. |
doc/design-docs/AtomicDistributedTransaction.md |
Documents the new schema. |
| join %s.redo_statement s on t.dtid = s.dtid and t.db_name = s.db_name | ||
| where t.db_name = %s or (t.db_name = '' and not exists (select 1 from %s.redo_state cur where cur.dtid = t.dtid and cur.db_name = %s)) |
There was a problem hiding this comment.
Hmm. That makes sense. The goal here was to try to avoid the need to migrate or drain legacy rows. A clean option is to have maintainers drain before upgrading, but that feels like a high impact for an issue that only impacts the vttestserver.
The issue only affects environments where multiple vttablets share the same mysqld and the same sidecar database, which my understanding is unique to the vttestserver / vtcombo.
There was a problem hiding this comment.
Well actually, thinking about this more, I believe the scenario described here is already unsafe in the legacy format.
If multiple vttablets share the same sidecar database, legacy redo rows do not contain enough ownership information to determine which tablet/application DB should replay them. This is probably already failing as a DTID primary-key collision, and even when it does not fail visibly, recovery of an unscoped legacy row is ambiguous. That's the current status-quo.
My understanding is that this shared-sidecar setup is not a normal production topology and is mainly relevant to vttestserver/vtcombo, which is the case this PR is trying to fix for newly written redo rows.
So I agree the fallback is not safe in the abstract for old shared-sidecar rows, but that was already true.
One possible mitigation would be to add a setting for shared-sidecar environments to skip legacy db_name = '' redo recovery, but I’m not sure that is worth adding given that it's an already-ambiguous state.
Description
Add
db_nameto the 2PC redo sidecar tables and include it in the primary keys so multiple vttablets sharing a mysqld instance do not collide when preparing the same distributed transaction DTID.Record the tablet DB name when saving redo state/statements, and scope redo reads, updates, deletes, and unresolved counts by that DB name. Keep fallback visibility for rows with an empty db_name so pre-upgrade prepared redo records can still be recovered or cleaned up.
Update tabletserver, end-to-end, vtexplain, and binlog test expectations for the new redo table shape.
Issue was confirmed fixed locally when running vttestserver and executing 2PCs.
Fixes #19493
Related Issue(s)
#19493
Checklist
Deployment Notes
2PC redo sidecar tables now include the tablet database name in their primary key. Existing prepared redo rows from before this change use an empty
db_nameand remain visible for recovery and cleanup during upgrade. New redo rows are scoped by tablet database name, preventing DTID collisions when multiple vttablets share the same mysqld instance.Legacy redo rows without
db_nameremain recoverable for the existing isolated-sidecar deployment model. In shared-sidecar environments, legacy rows were already ambiguous because they did not record the owning tablet DB; this change fixes newly written redo rows but cannot safely infer ownership for pre-existing shared-sidecar redo.AI Disclosure
AI Assistance: This PR was written primarily by Codex. I used Codex to help root cause the initial issue while using vttestserver, then used Codex to determine and implement an appropriate fix.