Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dm: update dm-precheck.md #8409

Merged
merged 18 commits into from
Mar 21, 2022
Merged

dm: update dm-precheck.md #8409

merged 18 commits into from
Mar 21, 2022

Conversation

okJiang
Copy link
Member

@okJiang okJiang commented Feb 16, 2022

First-time contributors' checklist

What is changed, added or deleted? (Required)

  • update dm/dm-precheck.md

Which TiDB version(s) do your changes apply to? (Required)

Tips for choosing the affected version(s):

By default, CHOOSE MASTER ONLY so your changes will be applied to the next TiDB major or minor releases. If your PR involves a product feature behavior change or a compatibility change, CHOOSE THE AFFECTED RELEASE BRANCH(ES) AND MASTER.

For details, see tips for choosing the affected versions (in Chinese).

  • master (the latest development version)
  • v5.4 (TiDB 5.4 versions)
  • v5.3 (TiDB 5.3 versions)
  • v5.2 (TiDB 5.2 versions)
  • v5.1 (TiDB 5.1 versions)
  • v5.0 (TiDB 5.0 versions)
  • v4.0 (TiDB 4.0 versions)
  • v3.1 (TiDB 3.1 versions)
  • v3.0 (TiDB 3.0 versions)
  • v2.1 (TiDB 2.1 versions)

What is the related PR or file link(s)?

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Need modification after applied to another branch
  • Might cause conflicts after applied to another branch

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Feb 16, 2022

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • shichun-0415

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added missing-translation-status This PR does not have translation status info. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 16, 2022
@okJiang
Copy link
Member Author

okJiang commented Feb 16, 2022

/cc @sunzhaoyang @lance6716

dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved

- 分表存在自增主键,且自增主键 column 类型不为 bigint
- 分表存在自增主键,自增主键 column 类型为 bigint,但没有为其配置列值转换
+ 检查上游是否处于 [Online-DDL](/dm/feature-online-ddl.md) 过程中,即创建了 `ghost` 表,但还未执行 `rename` 的阶段。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

再补充下这个检查结果吧。如果上游出于 online ddl 中,将会怎样,用户应该如何做

dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
@TomShawn TomShawn added area/dm needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. translation/doing This PR’s assignee is translating this PR. and removed missing-translation-status This PR does not have translation status info. labels Feb 17, 2022
@okJiang okJiang removed the needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. label Feb 18, 2022
@TomShawn
Copy link
Contributor

@okJiang 这个文档改动是适用于 v5.4 以后的版本吗?

@okJiang
Copy link
Member Author

okJiang commented Feb 18, 2022

@okJiang 这个文档改动是适用于 v5.4 以后的版本吗?

是的,我之前标错了

@TomShawn TomShawn added for-future-release This PR only applies to master for now and might cherry-pick to a future release. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Feb 18, 2022
@okJiang
Copy link
Member Author

okJiang commented Feb 18, 2022

all comment fixed. ptal again @sunzhaoyang

dm/dm-precheck.md Outdated Show resolved Hide resolved
```yaml
mydumpers: # dump 处理单元的运行配置参数
global: # 配置名称
threads: 4 # dump 处理单元从上游数据库实例导出数据和 check-task 访问上游的线程数量,默认值为 4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
threads: 4 # dump 处理单元从上游数据库实例导出数据和 check-task 访问上游的线程数量,默认值为 4
threads: 4 # dump 处理单元从上游数据库实例导出数据和执行前置检查时访问上游的线程数量,默认值为 4,建议不要超过 64。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 64 是我拍的,可能不是合理的值

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

之前的 dumpThread 有设置过大而产生的 issue 和 oncall 么?我们应该在文档中标明这个参数的影响就好了,例如

该参数与上游的连接数有关,如果太大可能会加大上游的负载

Copy link
Contributor

@Ehco1996 Ehco1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved

对于全量加增量数据迁移模式,除了通用检查项外,前置检查还将包含全量数据迁移模式(`task-mode: full`)相关的检查项,以及增量数据迁移模式(`task-mode: incremental`)相关的检查项。

## 检查配置
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

标题不够明确,检查配置和前面检查项是什么关系?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个是 checker 本身的配置,用于配置 checker 自己的参数的,和前面的检查项没关系

dm/dm-precheck.md Outdated Show resolved Hide resolved

+ 数据库配置
* 通用检查项
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

跟“通用检查项”并列的是什么呢?如果没有,用 item list 不太合适。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为 CI 里要求使用统一的 code block style

dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
dm/dm-precheck.md Outdated Show resolved Hide resolved
Copy link
Contributor

@shichun-0415 shichun-0415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 21, 2022
@qiancai qiancai removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 21, 2022
@qiancai
Copy link
Collaborator

qiancai commented Mar 21, 2022

/remove-status LGT1
/status LGT2

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Mar 21, 2022
@qiancai
Copy link
Collaborator

qiancai commented Mar 21, 2022

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: 45c1846

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 21, 2022
@ti-chi-bot ti-chi-bot merged commit 16ec173 into pingcap:master Mar 21, 2022
@qiancai qiancai mentioned this pull request Mar 23, 2022
16 tasks
@qiancai qiancai added translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. and removed translation/doing This PR’s assignee is translating this PR. labels Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dm size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR. v6.0 This PR/issue applies to TiDB v6.0.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants