Skip to content

dm-best-practices: define N in the small-dataset DM-worker deployment guidance - #23747

Open
yahonda wants to merge 1 commit into
pingcap:release-8.5from
yahonda:fix-dm-best-practices-undefined-n
Open

yahonda wants to merge 1 commit into
pingcap:release-8.5from
yahonda:fix-dm-best-practices-undefined-n

Conversation

@yahonda

@yahonda yahonda commented Sep 7, 2026

Copy link
Copy Markdown
Member

What is changed, added or deleted? (Required)

The deployment-plan table in dm/dm-best-practices.md recommends deploying "1~N DM-worker nodes according to the number of upstream data sources" without ever defining what N refers to. This PR rewords it to explicitly tie N to the number of upstream data sources: "Deploy 1 to N DM-worker nodes, where N is the number of upstream data sources."

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

  • master (the latest development version)
  • v8.5 (TiDB 8.5 versions)
  • v8.4 (TiDB 8.4 versions)
  • v8.3 (TiDB 8.3 versions)
  • v8.2 (TiDB 8.2 versions)
  • v8.1 (TiDB 8.1 versions)
  • v7.5 (TiDB 7.5 versions)
  • v7.1 (TiDB 7.1 versions)
  • v6.5 (TiDB 6.5 versions)

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

AI agent involvement

  • The changes in this PR were primarily made by an AI agent on behalf of the PR author.

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

Summary by CodeRabbit

  • Documentation
    • Updated deployment guidance to clarify that DM workers should use between one and N nodes, where N matches the number of upstream data sources.
    • Replaced approximate range notation with explicit “1 to N” wording for clearer deployment planning.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 130d47d0-5402-49eb-9b3c-e7d78542e198

📥 Commits

Reviewing files that changed from the base of the PR and between 901e176 and 85e6d0a.

📒 Files selected for processing (1)
  • dm/dm-best-practices.md

Included review availability: Your plan provides up to 4 included reviews per hour; 1 remains after this review.


📝 Walkthrough

Walkthrough

The deployment-plan table now states that DM-worker deployment uses 1 to N nodes, where N is the number of upstream data sources.

Changes

DM deployment guidance

Layer / File(s) Summary
Clarify DM-worker node count
dm/dm-best-practices.md
The deployment table defines N as the number of upstream data sources and uses “1 to N” DM-worker nodes.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Change: Other · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to b1bca

The documentation clarification matches the stated deployment objective and introduces no identified production or operational risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: defining N in the DM-worker deployment guidance.
Description check ✅ Passed The description explains the documentation change, identifies the affected TiDB version, includes the related issue, records AI involvement, and addresses the template checklist sections.
Linked Issues check ✅ Passed Issue #23744 requires the English source to define N in the deployment-plan table. The PR changes dm/dm-best-practices.md from “1~N” to “1 to N” and states that N is the number of upstream data …
Out of Scope Changes check ✅ Passed The change is limited to the deployment-plan wording in dm/dm-best-practices.md. It directly addresses issue #23744 and introduces no unrelated implementation or documentation changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ti-chi-bot

ti-chi-bot Bot commented Sep 7, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hfxsd for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot Bot added missing-translation-status This PR does not have translation status info. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 7, 2026
@yahonda

yahonda commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@yahonda

yahonda commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@yahonda

yahonda commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.


Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 37 minutes.

@yahonda

yahonda commented Sep 14, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@yahonda
yahonda force-pushed the fix-dm-best-practices-undefined-n branch from 85e6d0a to b1bca01 Compare September 18, 2026 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-translation-status This PR does not have translation status info. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant