Skip to content

Support MySQL binlog transaction compression - #57

Merged
coding-chimp merged 1 commit into
bb/go-mysql-v1.16from
bb/binlog-compression
Sep 30, 2026
Merged

coding-chimp merged 1 commit into
bb/go-mysql-v1.16from
bb/binlog-compression

Conversation

@coding-chimp

@coding-chimp coding-chimp commented Sep 22, 2026 •

Copy link
Copy Markdown

Summary

Add support for MySQL's binlog_transaction_compression=ON on top of the separately reviewed checkpoint-safety and go-mysql upgrade PRs.

Compressed transactions arrive as a TransactionPayloadEvent containing nested row and commit events. Previously, gh-ost ignored the wrapper and silently skipped its DML.

Changes

  • Process nested INSERT, UPDATE, DELETE, and XID events through the existing cancellation-safe row and transaction-completion handlers.
  • Preserve the outer payload's file coordinates and the existing cloned GTID completion boundaries.
  • Keep TransactionPayloadEvent last in the reader's event switch.
  • Add compression-specific parser/decoding, filtering, mixed-transaction, error, and marker-cancellation tests.
  • Extend the existing checkpoint integration matrix to compressed transactions, retaining file-position/GTID, partial-apply replay, failed-batch, and cancellation coverage.

Testing

  • Full and short GOTOOLCHAIN=auto go test ./... suites passed.
  • Targeted race tests passed.
  • golangci-lint v2.11.4: 0 issues.
  • MySQL 8.0.42 integration verifies that compressed payloads are actually written and consumed.

Rollback note

Do not resume with an older reader while the replay interval still contains compressed transactions. Turning compression off for future writes does not remove already-retained compressed events.

@coding-chimp coding-chimp self-assigned this Sep 22, 2026
@coding-chimp
coding-chimp added this pull request to stack #58 September 22, 2026 07:59
for _, nested := range event.Events {
switch nestedEvent := nested.Event.(type) {
case *replication.RowsEvent:
if err := gmr.handleRowsEvent(nested, nestedEvent, entriesChannel); err != nil {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Enqueue a transaction-completion marker after the nested XID.
  2. Buffer the entire transaction and apply it as one queue item.
  3. Track outstanding DML per transaction and advance the coordinate when the count reaches zero.
  4. Keep a pending-transaction watermark that makes Checkpoint() wait independently of applier.CurrentCoordinates.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction: prior to this change, the race would be limited to multiple rows inside a RowsEvent.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Should be fixed now, though the PR grew quite a bit. 😅

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought whether it would make it easier to break it down as 3 distinct PRs, maybe it would be easier to upstream?

Last week I also found out that we're behind on upstreaming binlog metrics and opened an issue for that (private).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I figure we can get these merged on our side this week and I'll work on upstreaming these changes once I'm back from vacation.

Base automatically changed from sync-upstream-2026-11-22 to master September 24, 2026 07:13
Process nested row and XID events from TransactionPayloadEvent through the existing cancellation-safe transaction marker pipeline. Retain outer payload file coordinates and keep payload handling last in the reader switch.

Add compressed parsing, filtering, decoding, and cancellation tests. Extend MySQL integration coverage to compressed transactions in file-position and GTID modes, including partial-apply replay and checkpoint cancellation.
@coding-chimp
coding-chimp removed this pull request from stack #58 September 28, 2026 15:54
@coding-chimp
coding-chimp changed the base branch from master to bb/go-mysql-v1.16 September 28, 2026 15:54
@coding-chimp
coding-chimp added this pull request to stack #62 September 28, 2026 15:54
@coding-chimp
coding-chimp merged commit e3122d4 into master Sep 30, 2026
16 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.

2 participants