[SS-97] MySQL Parallel Snapshot#37642
Conversation
4408381 to
1816886
Compare
1816886 to
c0311fc
Compare
b538f70 to
32b6fd3
Compare
### Motivation Setting up tests and benchmarks for #37642 ahead of time where possible. ### Description - 8-worker benchmark for a single 1M row table (which should get faster if we parallelize) - 8-worker benchmark for many tables to prevent regressions - Varied test-drive tests to help ensure no regressions in correctness ### Verification The first benchmark was used to show the speedup, the second caught a regression in the original version of the PR, and the testdrive tests should help with any obvious/simple correctness issues. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
d3285d8 to
32a4d3e
Compare
737d603 to
0352e90
Compare
I think we do want to add this too -- making this change now. |
de9b1ec to
aa33d1f
Compare
|
Given that we found a bunch of bugs, should we maybe have a way to fall back to non-parallel snapshotting with an LD flag, just in case? |
|
Yeah, I'm open to a kill switch. I can easily fall back to the old partitioning logic with a flag. It might be a little harder to fall back to the old way of handling locking and opening up read transactions. I'll draft the easier version, but if anyone feels strongly about avoiding or having a kill switch for the new structure I can scope that out. |
A BIT column maps to the UInt64 scalar type but carries a MySqlColumnMeta::Bit marker. The PK-range sampler rendered its split boundary from the scalar type alone, taking the integer path CAST(id AS CHAR). For a BIT column that returns the value's raw bytes, not a decimal literal, and the text was spliced unquoted into the worker range predicates. A crafted BIT value could then inject SQL so every worker read the whole table, duplicating rows. Exclude PK columns that carry a column-meta marker from range splitting, so they fall back to a single-worker whole-table read. Also validate that integer-path boundary literals are decimal before using them, guarding against any other type whose CAST-to-text is not a plain integer.
verify_schemas reads information_schema, which is not part of the repeatable-read snapshot, so the per-worker check after the lock was released compared each output against a live schema that need not match the snapshot the worker actually read. It also ran after PK sampling, so a primary-key column renamed or dropped upstream failed the sampling probe with a transient error that re-ran before verification every restart, wedging the whole source in a loop. Verify on the leader before sampling and locking. Outputs whose schema drifted are dropped and excluded from sampling, so a renamed or dropped column surfaces as a per-output definite error while the rest of the source keeps ingesting, with no retry loop. Each worker still re-verifies in its read transaction, but any incompatibility found there now means the schema moved after the leader's check, so it returns a transient error and the snapshot retries from a consistent state. The PK-boundary collation check moves off the leader's lock path the same way. Instead of the leader validating monotonicity under the lock and downgrading drifted tables to whole-table reads, each worker validates its ranged tables in its read transaction and fails transiently on drift, so the retry re-samples under the new collation. The leader's critical section shrinks to lock, read GTID, broadcast. DDL after the worker checks is caught by the reads failing with ER_TABLE_DEF_CHANGED.
Drop a redundant modulo in worker_pk_range that read like an off-by-precedence bug, remove an unnecessary loop label, fix a typo and a contradictory comment about the COUNT(*) fallback, and collapse a double blank line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the mysql_source_snapshot_parallelism dyncfg, enabled by default, as a kill switch for parallel PK-range snapshotting. When disabled the leader skips boundary sampling, so every table takes the existing single-worker whole-table fallback and the read-phase upstream connection footprint drops back to one connection per responsible worker. The leader-based lock/broadcast protocol is unaffected. Registering the dyncfg automatically exposes it as a system var, so it is settable via ALTER SYSTEM SET and manageable via LaunchDarkly. Lists the flag in the test-infra parameter tables so CI can vary it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cb2a9b0 to
383ecb5
Compare
|
Pulled testdrive changes out here: #37819 to shrink the diff a little bit. |
martykulma
left a comment
There was a problem hiding this comment.
Looks great @peterdukelarsen! The main concern I have is around all the extra work we now do while holding the lock for all tables. There's some low hanging fruit there in connection setup that we should address.
While not a blocker, I think we should give some serious considering to moving away from COUNT(*) for row count. As it stands, the implementation will perform 2 full index scans (one for the count, one for the offset queries) and then a full table scan (snapshot).
count is used in 2 places:
- statistics (and PG already uses an estimate for tables with > 1m rows)
- partitioning
My understanding is that a stale count affects the effectiveness of the partitioning and the accuracy of the statistics reporting, but it won't affect correctness.
| // by the leader to publish the snapshot size gauge. | ||
| let mut snapshot_counts: BTreeMap<MySqlTableName, u64> = BTreeMap::new(); | ||
| let mut lock_conn = if is_snapshot_leader { | ||
| match lock_and_prepare_snapshot( |
There was a problem hiding this comment.
We should try and minimize the time we're holding the lock. Right now, it looks like we pay connection costs for each worker under the lock. We can be more aggressive here and open the connections ahead of time. If there's no work, we'll just close them.
There was a problem hiding this comment.
That seems reasonable to me! Only downside to call out is we'll now have a higher peak of open connections, ~2x worker count (workers holding open a conn while waiting for the lock signal overlapping with the connections the leader uses to parallelize partition selection). I don't think that's a blocker.
Forgot to mention that I'm good with adding that in a separate PR to avoid any additional complexity in this one. |
593dea3 to
d29b062
Compare
Each worker connects to MySQL and validates replication settings before awaiting the leader's broadcast, concurrently with the leader's PK sampling and LOCK TABLES, shortening the window in which the leader holds the read lock. The leader opens its own read connection the same way, before it takes the locks. Workers that end up with no work, or whose leader failed, drop their connection on exit, the same way the completion path drops the read connection. Setup now briefly holds up to 2 * worker_count + 1 upstream connections per source, since idle worker connections overlap the leader's sampling pool. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d29b062 to
d05b1ae
Compare
martykulma
left a comment
There was a problem hiding this comment.
nice - lgtm!
Non-blocking, but small (and probably worth it). Those connections are going to sit idle for the duration of the partitioning. User may have overridden the default, so explicitly setting the wait timeout once those connections are established for defense.
SET @@session.wait_timeout = 28800 (this is the default )
Seeing if this unblocks merging. I've addressed all of Dennis' review comments and he's approved the tests I split out to the next PR.
Worker snapshot connections sit idle while the leader samples PK bounds and takes the table locks. On servers where the global wait_timeout is lowered below that setup time, the server reaps the idle connections and the snapshot restarts into the same wall. Explicitly restore the MySQL default wait_timeout of 28800 seconds on each snapshot connection once it is established. Network middlebox idle timeouts are already covered by the existing TCP keepalive configuration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5be2673 to
eca9fd1
Compare
Motivation
Taking over implementation of #35569.
For customers with large tables in MySQL the initial snapshot can be painfully slow. We can improve performance by distributing the initial snapshot across workers.
Description
Previously we would:
The problem with this is when one table is huge we are as slow as the single worker copying the whole table.
Now the MySQL snapshot logic at a high level will be:
The leader will parallelize the partition computation using multithreading. The partition computation is much faster than actually reading the full snapshot, but it is slow and scales with the table size. The leader will step through the table along the primary key sort order using offsets of
1 / # workersto compute partitions of approximately the same size to use for partitioning the table.Verificiation
Benchmark Performance
Some basic benchmarks: https://buildkite.com/materialize/nightly/builds/17371/list. First two show it hasn't gotten slower for many tables while the last demonstrates the case of a single large table being sped up. This shows a 2.7x speedup on a large table without a slowdown on many tables (although it's possible there could be a slowdown of ~10% on perfectly even distribution of tables due to the extra work of partitioning).
Known trade-offs/risk areas