[SS-97] Small refactor to prep for MySQL parallel snapshot changes#37692
Conversation
26beebc to
1e8f86d
Compare
Behavior-preserving refactor of the MySQL snapshot reader: pull the SET lock_wait_timeout / LOCK TABLES / read @@gtid_executed sequence out of render() into lock_tables_and_read_gtid_set(), returning the raw GTID set string (definite TableDropped error on a vanished table). The parallel-snapshot work builds on this so its leader path reuses the same helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1e8f86d to
bf3441f
Compare
patrickwwbutler
left a comment
patrickwwbutler
left a comment
There was a problem hiding this comment.
Overall looks good! minor suggestion on the error enum but i don't think it's terribly important
| Ok(total) | ||
| } | ||
|
|
||
| enum SnapshotSetupError { |
There was a problem hiding this comment.
IMO it's reasonable to just reuse the ReplicationError enum that already exists with the same two variants, as this eum is effectively duplicated code, but it's not terribly important. If you want to maintain a separate name for clarity of purpose/origin of the error, you could use something like type SnapshotSetupError = ReplicationError to just alias it
There was a problem hiding this comment.
The one hiccup with this is that the ReplicationError wraps the underlying errors with Rc so it makes the code a good bit uglier to re-use that type. I'm leaning towards leaving it as is with the targeted type & conversions.
2 participants
Motivation
This is a subcomponent of the larger MySQL parallel snapshotting feature: #37642.
We're working on parallelizing mysql snapshotting. Trying to shrink the overall diff by a bit in the upcoming PR by extracting re-usable bits into methods.
Description
Pull the SET lock_wait_timeout / LOCK TABLES / read @@gtid_executed sequence out of render() into lock_tables_and_read_gtid_set(), returning the raw GTID set string (definite TableDropped error on a vanished table).