Skip to content

Port merge DML optimization from PR #1378 to current master - #3

Closed
dnovitski wants to merge 6 commits into
masterfrom
pr-1378
Closed

Port merge DML optimization from PR #1378 to current master#3
dnovitski wants to merge 6 commits into
masterfrom
pr-1378

Conversation

@dnovitski

@dnovitski dnovitski commented May 22, 2026

Copy link
Copy Markdown
Owner

Summary

Ports the merge-DML batching optimization from upstream PR github#1378 to current master, adapting it to the refactored builder-pattern API.

What this does

When --is-merge-dml-event is enabled and the unique key is memory-comparable (all numeric columns):

  1. Deduplicates DML events by unique key (latest event wins per key)
  2. Cancels INSERT+DELETE pairs for same key (net no-op)
  3. Batches remaining INSERTs/UPDATEs as multi-row REPLACE INTO ... VALUES (row1), (row2), ...
  4. Batches remaining DELETEs as DELETE ... WHERE (pk) IN (val1, val2, ...)
  5. Skips events for rows beyond current migration range (already copied by row-copy)

Performance

Benchmarked on the original PR github#1378 branch:

  • 10% faster general workload (32K rows, 4K events)
  • 39% faster hot-spot dedup (100 keys, 4K updates)
  • 2.25× sustained throughput (12 → 27 events/s)

Key correctness fix over github#1378

Uses BuildColumnsPreparedValues(MappedSharedColumns) in generateBatchedReplaceQuery to generate proper per-column conversion tokens (convert_tz(?, ...), ELT(?, ...), etc.) instead of plain ? placeholders. This prevents data corruption for timezone, enum, and JSON columns.

Files changed

  • go/sql/types.go: Add CompareValueFunc, FormatValueFunc on Column; IsMemoryComparable, FormatValues on UniqueKey
  • go/sql/builder.go: Export BuildColumnsPreparedValues
  • go/base/context.go: Add TotalDMLEventsIgnored, IsMergeDMLEvents
  • go/logic/inspect.go: Compute IsMemoryComparable and set compare/format funcs for numeric columns
  • go/logic/applier.go: Merge-DML logic (dedup map, batched REPLACE/DELETE, range filtering)
  • go/logic/applier_test.go: Unit tests for batch query generation and range filtering
  • go/logic/migrator.go: Branch to merged apply path, add Ignored counter to status
  • go/cmd/gh-ost/main.go: --is-merge-dml-event flag
  • doc/command-line-flags.md: Documentation

Relationship to upstream

This is a clean port of the optimization from PR github#1378 (by @kolbe), rebased onto current master with my correctness fix applied. The original branch was 125 commits behind master and used the old standalone function API.

shaohoukun and others added 6 commits February 5, 2024 18:21
1. Support for ignoring binlog events that exceed chunk boundary values. 2. Support for binlog merge
processing.
When there is only one column in a unique index, merging of DML binlog events is permitted.
…tionRangeMaxValues is nil

Add handling for the case when MigrationIterationRangeMaxValues is nil
…reads

The merge DML optimization (PR github#1378) used REPLACE...SELECT FROM source
to batch-apply events, which reads live table state instead of the binlog
row images. This breaks correctness when events arrive during throttle,
after kill/restart, or before cutover drains completely.

Changes:
- Replace source-read REPLACE with parameterized REPLACE...VALUES using
  binlog after-images (sharedColumnArgs from BuildDML*Query)
- Use BuildColumnsPreparedValues for correct timezone/enum/JSON handling
- Fix nil MigrationIterationRangeMaxValues check (was returning true)
- Remove dead generateReplaceQuery (unsafe source-read version)
- Use mergedEntry struct to avoid unsafe string key reconstruction

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dnovitski dnovitski changed the title fix: use binlog row images in merge DML path instead of source table reads Port merge DML optimization from PR #1378 to current master May 22, 2026
@dnovitski

Copy link
Copy Markdown
Owner Author

Superseded by new PR from rebased branch (pr-1378-rebased on current master).

@dnovitski dnovitski closed this May 22, 2026
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.

1 participant