added flag to support sharded migrations via cli #913
Conversation
bharadwaj-aditya
requested review from
manitgupta and
Deep1998
and removed request for
a team
removed extra files added spanner migration tool to gitignore
bharadwaj-aditya
force-pushed
the
sharding-cli
branch
from
6d3908b
to
3be49a6
Compare
pull-request-size
bot
added
size/XXL
size/XL
size/L
size/M
and removed
size/L
size/XXL
size/XL
labels
bharadwaj-aditya
force-pushed
the
sharding-cli
branch
from
f46161a
to
a08594f
Compare
moving rules to internal corrected tests
bharadwaj-aditya
force-pushed
the
sharding-cli
branch
from
a08594f
to
de93959
Compare
| @@ -59,18 +59,32 @@ func (sads *SchemaFromSourceImpl) schemaFromDatabase(migrationProjectId string, | |||
| var infoSchema common.InfoSchema | |||
| var err error | |||
| isSharded := false | |||
| fmt.Printf("fetching schema from database %+v\n", sourceProfile.Config.ShardConfigurationBulk) | |||
There was a problem hiding this comment.
This might print sensitive details to the log? Please check print statements added (in other parts of PR as well)
| @@ -48,6 +48,8 @@ such as `avro` etc. | |||
| Please note that streaming migration is only supported for MySQL, Oracle and PostgreSQL databases currently. | |||
| Here is an example of a [streamingCfg JSON](./config-json.md#streamingcfg-for-non-sharded-minimal-downtime-migrations) and [how to use it in the CLI](./schema-and-data.md#examples). | |||
|
|
|||
| * **`shardingConfig`**: Specifies the sharding config file. | |||
There was a problem hiding this comment.
Let's add an example sharding config here.
| shardIdConfig, ok2 := params["shardIdConfig"] | ||
| if ok2 { | ||
| switch shardIdConfig { | ||
| case constants.SHARD_ID_PREFIX: | ||
| config.ShardIdConfig = shardIdConfig | ||
| case constants.SHARD_ID_SUFFIX: | ||
| config.ShardIdConfig = shardIdConfig | ||
| case constants.SHARD_ID_NONE: | ||
| config.ShardIdConfig = shardIdConfig | ||
| default: | ||
| return SourceProfile{}, fmt.Errorf("invalid shard id config passed") | ||
| } | ||
| } else { | ||
| //Default to setting shard id config to none | ||
| config.ShardIdConfig = constants.SHARD_ID_NONE | ||
| } |
There was a problem hiding this comment.
n.NewSourceProfileConfig seems to be a misnomer now since it is supposed to return a SourceProfileConfig but the SourceProfileConfig is now actually a combination of shardIdConfig and ShardConfiguration* (bulk, dms or dataflow). The shardIdConfig only gets added in later..
We should ideally rename NewSourceProfileConfig to something like parseShardConfigurationFile or shift this logic inside it so that it still returns the completed SourceProfileConfig implementation.
|
I am actually a bit confused on what this PR is trying to do. I think I initially misunderstood this as adding the ability to modify primary key, but it seems like it just allows user to do sharded migrations via CLI by passing in the cfg? If so, this is already supported today via the |
It started with that idea, and pulled back to break into 2 PR's as the larger PR was getting too big owing to a bunch of refactoring. But i agree, this PR doesnt seem super useful without the overall flow, so I'll probably close this and just raise the larger PR directly. |
Added support to perform sharded migrations from SMT CLI.
This defaults to not adding the migration_shard_id in the primary key.
Options to add in the primary key will be taken on as follow up work.