Add experimental move command #467
Conversation
morgo
force-pushed
the
mtocker-move-command
branch
from
5b83241 to
8e5abce
Compare
| func (r *Runner) Close() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Is there any clean-up or logging we need to do here?
There was a problem hiding this comment.
Likely yes in future. Currently because most code is in Run() I am able to do cleanups in defers.
| if err := r.copier.Run(ctx); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Do we log status / progress per table?
There was a problem hiding this comment.
Not yet. The copier does have a logger in its struct, but (comparing to table migration) the 30s log prints are external. This has not been implemented for move yet.
|
|
||
| // getSourceTables connects to r.source and fetches the list of tables | ||
| // to populate r.sourceTables. | ||
| func (r *Runner) getSourceTables(ctx context.Context) error { |
There was a problem hiding this comment.
I suppose a future enhancement could be to provide an explicit list of tables to operate on from the source. This idea is inspired by the Vitess MoveTables --tables option, which I've seen used in practice to omit some tables from being moved
There was a problem hiding this comment.
Yes, for future improvement. Could we use two regexes? (include and ignore list).
| r.logger.Infof("All tables copied successfully.") | ||
|
|
There was a problem hiding this comment.
If any of the table moves were to fail mid-flight, would the entire command / process stop? Or can there be a partial success state where some tables succeed and others fail?
There was a problem hiding this comment.
There is no checkpointing, and it doesn't currently support that there could be a schema change mid-flight either (I will add this one to the list of restrictions).
| "github.com/go-sql-driver/mysql" | ||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Are there any failure scenarios that are worth testing here?
There was a problem hiding this comment.
Yes (in future). This is experimental to start with, so I can start work planning how to integrate this with our orchestrator (tern) in parallel.
2 participants
A Pull Request should be associated with an Issue.
Fixes #463
What's missing: