From 44a29d6035d612fa0a09e1cbb0f62691e9d9b900 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Wed, 27 Apr 2022 17:01:24 +0800 Subject: [PATCH 1/3] check empty config --- dm/dm/config/task.go | 33 +++++++++++-------- dm/tests/dmctl_basic/check_list/start_task.sh | 28 ++++++++++++++++ .../dmctl_basic/conf/empty-unit-task.yaml | 27 +++++++++++++++ dm/tests/dmctl_basic/run.sh | 3 ++ 4 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 dm/tests/dmctl_basic/conf/empty-unit-task.yaml diff --git a/dm/dm/config/task.go b/dm/dm/config/task.go index 178b44dd627..f15ca09fb11 100644 --- a/dm/dm/config/task.go +++ b/dm/dm/config/task.go @@ -689,10 +689,11 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Mydumpers[inst.MydumperConfigName] if !ok { return terror.ErrConfigMydumperCfgNotFound.Generate(i, inst.MydumperConfigName) + } else if rule != nil { + globalConfigReferCount[configRefPrefixes[mydumperIdx]+inst.MydumperConfigName]++ + inst.Mydumper = new(MydumperConfig) + *inst.Mydumper = *rule // ref mydumper config } - globalConfigReferCount[configRefPrefixes[mydumperIdx]+inst.MydumperConfigName]++ - inst.Mydumper = new(MydumperConfig) - *inst.Mydumper = *rule // ref mydumper config } if inst.Mydumper == nil { if len(c.Mydumpers) != 0 { @@ -717,10 +718,11 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Loaders[inst.LoaderConfigName] if !ok { return terror.ErrConfigLoaderCfgNotFound.Generate(i, inst.LoaderConfigName) + } else if rule != nil { + globalConfigReferCount[configRefPrefixes[loaderIdx]+inst.LoaderConfigName]++ + inst.Loader = new(LoaderConfig) + *inst.Loader = *rule // ref loader config } - globalConfigReferCount[configRefPrefixes[loaderIdx]+inst.LoaderConfigName]++ - inst.Loader = new(LoaderConfig) - *inst.Loader = *rule // ref loader config } if inst.Loader == nil { if len(c.Loaders) != 0 { @@ -737,10 +739,11 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Syncers[inst.SyncerConfigName] if !ok { return terror.ErrConfigSyncerCfgNotFound.Generate(i, inst.SyncerConfigName) + } else if rule != nil { + globalConfigReferCount[configRefPrefixes[syncerIdx]+inst.SyncerConfigName]++ + inst.Syncer = new(SyncerConfig) + *inst.Syncer = *rule // ref syncer config } - globalConfigReferCount[configRefPrefixes[syncerIdx]+inst.SyncerConfigName]++ - inst.Syncer = new(SyncerConfig) - *inst.Syncer = *rule // ref syncer config } if inst.Syncer == nil { if len(c.Syncers) != 0 { @@ -816,10 +819,7 @@ func (c *TaskConfig) adjust() error { unusedConfigs = append(unusedConfigs, mydumper) } } - for loader, cfg := range c.Loaders { - if err1 := cfg.adjust(); err1 != nil { - return err1 - } + for loader := range c.Loaders { if globalConfigReferCount[configRefPrefixes[loaderIdx]+loader] == 0 { unusedConfigs = append(unusedConfigs, loader) } @@ -844,6 +844,13 @@ func (c *TaskConfig) adjust() error { sort.Strings(unusedConfigs) return terror.ErrConfigGlobalConfigsUnused.Generate(unusedConfigs) } + + for _, cfg := range c.Loaders { + if err1 := cfg.adjust(); err1 != nil { + return err1 + } + } + // we postpone default time_zone init in each unit so we won't change the config value in task/sub_task config if c.Timezone != "" { if _, err := utils.ParseTimeZone(c.Timezone); err != nil { diff --git a/dm/tests/dmctl_basic/check_list/start_task.sh b/dm/tests/dmctl_basic/check_list/start_task.sh index 0cb6758f292..1aec2a33874 100644 --- a/dm/tests/dmctl_basic/check_list/start_task.sh +++ b/dm/tests/dmctl_basic/check_list/start_task.sh @@ -26,3 +26,31 @@ function start_task_not_pass_with_message() { "start-task $task_conf" \ "$2" 1 } + +function start_task_empty_config() { + start_task_empty_dump_config $1 + start_task_empty_load_config $1 + start_task_empty_sync_config $1 +} + +function start_task_empty_dump_config() { + sed "/threads/d" $1 >/tmp/empty-dump.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "start-task /tmp/empty-dump.yaml" \ + "The configurations as following .* are set in global configuration but instances don't use them" 1 +} + +function start_task_empty_load_config() { + sed "/pool-size/d" $1 >/tmp/empty-load.yaml + cat /tmp/empty-load.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "start-task /tmp/empty-load.yaml" \ + "The configurations as following .* are set in global configuration but instances don't use them" 1 +} + +function start_task_empty_sync_config() { + sed "/worker-count/d" $1 >/tmp/empty-sync.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "start-task /tmp/empty-sync.yaml" \ + "The configurations as following .* are set in global configuration but instances don't use them" 1 +} diff --git a/dm/tests/dmctl_basic/conf/empty-unit-task.yaml b/dm/tests/dmctl_basic/conf/empty-unit-task.yaml new file mode 100644 index 00000000000..d5128b625c4 --- /dev/null +++ b/dm/tests/dmctl_basic/conf/empty-unit-task.yaml @@ -0,0 +1,27 @@ +--- +name: test +task-mode: all + +target-database: + host: "127.0.0.1" + port: 4000 + user: "root" + password: "" + +mysql-instances: + - source-id: "mysql-replica-01" + mydumper-config-name: "global" + loader-config-name: "global" + syncer-config-name: "global" + +mydumpers: + global: + threads: 4 + +loaders: + global: + pool-size: 16 + +syncers: + global: + worker-count: 32 diff --git a/dm/tests/dmctl_basic/run.sh b/dm/tests/dmctl_basic/run.sh index 828b6ace68e..0132f59c717 100755 --- a/dm/tests/dmctl_basic/run.sh +++ b/dm/tests/dmctl_basic/run.sh @@ -297,6 +297,9 @@ function run() { "stop-task $cur/conf/only_warning.yaml" \ "\"result\": true" 2 + echo "start task with empty unit config" + start_task_empty_config $cur/conf/empty-unit-task.yaml + cp $cur/conf/dm-task.yaml $WORK_DIR/dm-task-error-database-config.yaml sed -i "s/password: \"\"/password: \"wrond password\"/g" $WORK_DIR/dm-task-error-database-config.yaml check_task_error_database_config $WORK_DIR/dm-task-error-database-config.yaml From 4b49026daecf120d39777263b85b680a4bbb32d8 Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Fri, 29 Apr 2022 11:24:55 +0800 Subject: [PATCH 2/3] address comment --- dm/dm/config/task.go | 33 +++++++++-------- dm/tests/dmctl_basic/check_list/check_task.sh | 28 +++++++++++++++ dm/tests/dmctl_basic/check_list/start_task.sh | 36 +++++++------------ .../dmctl_basic/conf/empty-unit-task.yaml | 10 +++++- dm/tests/dmctl_basic/run.sh | 2 ++ 5 files changed, 71 insertions(+), 38 deletions(-) diff --git a/dm/dm/config/task.go b/dm/dm/config/task.go index f15ca09fb11..d0fd7f739e7 100644 --- a/dm/dm/config/task.go +++ b/dm/dm/config/task.go @@ -689,8 +689,9 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Mydumpers[inst.MydumperConfigName] if !ok { return terror.ErrConfigMydumperCfgNotFound.Generate(i, inst.MydumperConfigName) - } else if rule != nil { - globalConfigReferCount[configRefPrefixes[mydumperIdx]+inst.MydumperConfigName]++ + } + globalConfigReferCount[configRefPrefixes[mydumperIdx]+inst.MydumperConfigName]++ + if rule != nil { inst.Mydumper = new(MydumperConfig) *inst.Mydumper = *rule // ref mydumper config } @@ -718,8 +719,9 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Loaders[inst.LoaderConfigName] if !ok { return terror.ErrConfigLoaderCfgNotFound.Generate(i, inst.LoaderConfigName) - } else if rule != nil { - globalConfigReferCount[configRefPrefixes[loaderIdx]+inst.LoaderConfigName]++ + } + globalConfigReferCount[configRefPrefixes[loaderIdx]+inst.LoaderConfigName]++ + if rule != nil { inst.Loader = new(LoaderConfig) *inst.Loader = *rule // ref loader config } @@ -739,8 +741,9 @@ func (c *TaskConfig) adjust() error { rule, ok := c.Syncers[inst.SyncerConfigName] if !ok { return terror.ErrConfigSyncerCfgNotFound.Generate(i, inst.SyncerConfigName) - } else if rule != nil { - globalConfigReferCount[configRefPrefixes[syncerIdx]+inst.SyncerConfigName]++ + } + globalConfigReferCount[configRefPrefixes[syncerIdx]+inst.SyncerConfigName]++ + if rule != nil { inst.Syncer = new(SyncerConfig) *inst.Syncer = *rule // ref syncer config } @@ -763,7 +766,9 @@ func (c *TaskConfig) adjust() error { return terror.ErrContinuousValidatorCfgNotFound.Generate(i, inst.ContinuousValidatorConfigName) } globalConfigReferCount[configRefPrefixes[validatorIdx]+inst.ContinuousValidatorConfigName]++ - inst.ContinuousValidator = *rule + if rule != nil { + inst.ContinuousValidator = *rule + } } // for backward compatible, set global config `ansi-quotes: true` if any syncer is true @@ -819,7 +824,13 @@ func (c *TaskConfig) adjust() error { unusedConfigs = append(unusedConfigs, mydumper) } } - for loader := range c.Loaders { + + for loader, cfg := range c.Loaders { + if cfg != nil { + if err1 := cfg.adjust(); err1 != nil { + return err1 + } + } if globalConfigReferCount[configRefPrefixes[loaderIdx]+loader] == 0 { unusedConfigs = append(unusedConfigs, loader) } @@ -845,12 +856,6 @@ func (c *TaskConfig) adjust() error { return terror.ErrConfigGlobalConfigsUnused.Generate(unusedConfigs) } - for _, cfg := range c.Loaders { - if err1 := cfg.adjust(); err1 != nil { - return err1 - } - } - // we postpone default time_zone init in each unit so we won't change the config value in task/sub_task config if c.Timezone != "" { if _, err := utils.ParseTimeZone(c.Timezone); err != nil { diff --git a/dm/tests/dmctl_basic/check_list/check_task.sh b/dm/tests/dmctl_basic/check_list/check_task.sh index 95899bca558..d9a37867d8f 100644 --- a/dm/tests/dmctl_basic/check_list/check_task.sh +++ b/dm/tests/dmctl_basic/check_list/check_task.sh @@ -87,3 +87,31 @@ function check_task_only_warning() { "check-task $task_conf" \ "\"state\": \"warn\"" 1 } + +function check_task_empty_dump_config() { + sed "/threads/d" $1 >/tmp/empty-dump.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "check-task /tmp/empty-dump.yaml" \ + "pre-check is passed" 1 +} + +function check_task_empty_load_config() { + sed "/pool-size/d" $1 >/tmp/empty-load.yaml + cat /tmp/empty-load.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "check-task /tmp/empty-load.yaml" \ + "pre-check is passed" 1 +} + +function check_task_empty_sync_config() { + sed "/worker-count/d" $1 >/tmp/empty-sync.yaml + run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ + "check-task /tmp/empty-sync.yaml" \ + "pre-check is passed" 1 +} + +function check_task_empty_config() { + check_task_empty_dump_config $1 + check_task_empty_load_config $1 + check_task_empty_sync_config $1 +} diff --git a/dm/tests/dmctl_basic/check_list/start_task.sh b/dm/tests/dmctl_basic/check_list/start_task.sh index 1aec2a33874..7d98ae84b65 100644 --- a/dm/tests/dmctl_basic/check_list/start_task.sh +++ b/dm/tests/dmctl_basic/check_list/start_task.sh @@ -28,29 +28,19 @@ function start_task_not_pass_with_message() { } function start_task_empty_config() { - start_task_empty_dump_config $1 - start_task_empty_load_config $1 - start_task_empty_sync_config $1 -} - -function start_task_empty_dump_config() { - sed "/threads/d" $1 >/tmp/empty-dump.yaml + cp $1 /tmp/empty-cfg.yaml + sed -i "/threads/d" /tmp/empty-cfg.yaml + sed -i "/pool-size/d" /tmp/empty-cfg.yaml + sed -i "/worker-count/d" /tmp/empty-cfg.yaml run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ - "start-task /tmp/empty-dump.yaml" \ - "The configurations as following .* are set in global configuration but instances don't use them" 1 -} - -function start_task_empty_load_config() { - sed "/pool-size/d" $1 >/tmp/empty-load.yaml - cat /tmp/empty-load.yaml + "start-task /tmp/empty-cfg.yaml" \ + "\"result\": true" 2 run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ - "start-task /tmp/empty-load.yaml" \ - "The configurations as following .* are set in global configuration but instances don't use them" 1 -} - -function start_task_empty_sync_config() { - sed "/worker-count/d" $1 >/tmp/empty-sync.yaml + "config task empty-unit-task" \ + "threads: 4" 1 \ + "pool-size: 16" 1 \ + "worker-count: 16" 1 run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ - "start-task /tmp/empty-sync.yaml" \ - "The configurations as following .* are set in global configuration but instances don't use them" 1 -} + "stop-task $1" \ + "\"result\": true" 2 +} \ No newline at end of file diff --git a/dm/tests/dmctl_basic/conf/empty-unit-task.yaml b/dm/tests/dmctl_basic/conf/empty-unit-task.yaml index d5128b625c4..2eb41893d3d 100644 --- a/dm/tests/dmctl_basic/conf/empty-unit-task.yaml +++ b/dm/tests/dmctl_basic/conf/empty-unit-task.yaml @@ -1,5 +1,5 @@ --- -name: test +name: empty-unit-task task-mode: all target-database: @@ -13,6 +13,7 @@ mysql-instances: mydumper-config-name: "global" loader-config-name: "global" syncer-config-name: "global" + block-allow-list: "instance" mydumpers: global: @@ -25,3 +26,10 @@ loaders: syncers: global: worker-count: 32 + +block-allow-list: + instance: + do-dbs: ["dmctl"] + do-tables: + - db-name: "dmctl" + tbl-name: "~^t_[\\d]+" \ No newline at end of file diff --git a/dm/tests/dmctl_basic/run.sh b/dm/tests/dmctl_basic/run.sh index 0132f59c717..e803b7984a8 100755 --- a/dm/tests/dmctl_basic/run.sh +++ b/dm/tests/dmctl_basic/run.sh @@ -297,6 +297,8 @@ function run() { "stop-task $cur/conf/only_warning.yaml" \ "\"result\": true" 2 + echo "check task with empty unit config" + check_task_empty_config $cur/conf/empty-unit-task.yaml echo "start task with empty unit config" start_task_empty_config $cur/conf/empty-unit-task.yaml From 7cd1aa8a510e630af99d76db519d88550edee5cc Mon Sep 17 00:00:00 2001 From: gmhdbjd Date: Fri, 29 Apr 2022 15:07:01 +0800 Subject: [PATCH 3/3] fix lint --- dm/tests/dmctl_basic/check_list/start_task.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dm/tests/dmctl_basic/check_list/start_task.sh b/dm/tests/dmctl_basic/check_list/start_task.sh index 7d98ae84b65..d00e6678197 100644 --- a/dm/tests/dmctl_basic/check_list/start_task.sh +++ b/dm/tests/dmctl_basic/check_list/start_task.sh @@ -43,4 +43,4 @@ function start_task_empty_config() { run_dm_ctl $WORK_DIR "127.0.0.1:$MASTER_PORT" \ "stop-task $1" \ "\"result\": true" 2 -} \ No newline at end of file +}