Support MySQL binlog transaction compression - #57
coding-chimp wants to merge 1 commit into
Conversation
368c028 to
0cb3e00
Compare
| for _, nested := range event.Events { | ||
| switch nestedEvent := nested.Event.(type) { | ||
| case *replication.RowsEvent: | ||
| if err := gmr.handleRowsEvent(nested, nestedEvent, entriesChannel); err != nil { |
There was a problem hiding this comment.
A local review agent helped me find this. Each nested DML here carries the outer payload position, so the first applied row falsely claims that the complete payload was applied, which is a likely issue for the applier / checkpointing in compressed + file mode.
handleRowsEvent does currentCoords := gmr.GetCurrentBinlogCoordinates() for every event.
I think this potential bug also affects GTID-streaming which is unrelated to this change, because all row events in a transaction carry the same GTID, so applying the first row can mark the entire transaction as applied while later rows remain queued in entriesChannel.
There's a tiny window when this can happen (either in file-compressed here or GTID modes before):
- DML event process start for transaction T
- Partial row event processed
- Checkpoint
- Gh-ost crash
- A resume from checkpoint would treat T as applied 🔥
For an uncompressed GTID transaction:
GTID event → reader advances to G
row 1 → queued with G
row 2 → queued with G
row 3 → queued with G
XID → reader marks G as completely read
A compressed GTID transaction has the same unsafe outcome:
top-level GTID event → reader.currentCoordinates = G
top-level TransactionPayload → unpack nested events
nested row 1 → queued with G
nested row 2 → queued with G
nested row 3 → queued with G
nested XID → reader.LastTrxCoords = G
Here's a test with DMLBatchSize=1 to reproduce these 4 cases: https://vault.shopify.io/snippify/snippets/e3ebc6d0f58823d0c661
Possible fixes:
- Enqueue a transaction-completion marker after the nested XID.
- Buffer the entire transaction and apply it as one queue item.
- Track outstanding DML per transaction and advance the coordinate when the count reaches zero.
- Keep a pending-transaction watermark that makes Checkpoint() wait independently of applier.CurrentCoordinates.
There was a problem hiding this comment.
Correction: prior to this change, the race would be limited to multiple rows inside a RowsEvent.
Summary
Add support for MySQL’s
binlog_transaction_compression=ON.Compressed transactions arrive as a
TransactionPayloadEventcontaining nested row and commit events. Previously, gh-ost only handled top-level events, silently skipping the enclosed DML changes and failing to advance its completed-transaction checkpoint.Changes
go-mysqlfrom v1.15.0 to v1.16.0, which preserves decoding settings inside compressed payloads and fixes nested-event checkpoint metadata.INSERT,UPDATE,DELETE, andXIDevents through gh-ost’s existing row and transaction-completion handling.Testing