add docs for enforce mpp by Joyinqin · Pull Request #5811 · pingcap/docs

Joyinqin · 2021-06-18T10:02:36Z

First-time contributors' checklist

I've signed Contributor License Agreement that's required for repo owners to accept my contribution.

What is changed, added or deleted? (Required)

add docs for enforce mpp

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

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

This PR is translated from: add docs for enforce mpp docs-cn#6250
Other reference 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

hanfei1991 · 2021-06-21T04:20:58Z

+
+- Scope: SESSION
+- Default value: `OFF`
+- This variable is used to control whether to ignore the optimizer cost estimation, and to force the use of TiFlash's MPP mode to execute the queries. You can set the following values:


comma "," is usually not used before "and" in a compound sentence.

"to execute the queries" is a little redundant to me. Besides, "force to do sth" is more natural to me. How about "force to use TiFlash MPP mode"?

I'm not an English expert. These are only personal suggestions.

hanfei1991 · 2021-06-21T04:22:29Z


+### `enforce-mpp`
+
+ Determines whether to ignore the optimizer cost estimation, and force TiFlash's MPP mode to execute queries.


and to force?

hanfei1991 · 2021-06-21T04:26:35Z

+set @@session.tidb_enforce_mpp=0;
+```
+
+If you want to intelligently choose whether to use the MPP mode (by default) using the cost estimation of TiDB optimizer, you can execute the following statement:


This condition seems too long to read. How about replacing using the cost est of TiDB optimizer by 'by TiDB optimizer'? That makes sense to readers

hanfei1991 · 2021-06-21T04:27:05Z

+set @@session.tidb_enforce_mpp=0;
+```
+
+If you want TiDB to ignore the optimizer's cost estimates and force to choose the MPP mode, you can execute the following statement:


and to force?

hanfei1991 · 2021-06-21T04:28:00Z

+set @@session.tidb_enforce_mpp=1;
 ```

+The initial value of the Session variable `tidb_enforce_mpp` is equal to the[`enforce-mpp`](/tidb-configuration-file.md#enforce-mpp) configuration item value of this tidb-server instance (which is `false` by default). In a TiDB cluster, if several tidb-server instances only perform analytical queries, to ensure that they can choose MPP mode, you can change their [`enforce-mpp`](/tidb-configuration-file.md#enforce-mpp) configuration value to `true`.


is "Session" need to capitalize the first letter?

When I first read the sentence "In a TIDB cluster ...", I'm quite confused. I think you want to express "If you want to make several tidb servers only serve analytical quries in the cluster and to force them to use MPP mode," I think this way is clearer.

hanfei1991 · 2021-06-21T04:39:43Z

+>
+> When `tidb_enforce_mpp=1` takes effect, the TiDB optimizer will ignore the cost estimation to choose MPP mode. However, TiDB will not choose the MPP mode if other factors that do not support the MPP mode occur, such as no TiFlash copy, the replication of TiFlash copies is not completed, and statements contain operators or functions that are not supported by the MPP mode.
+> 
+> If the TiDB optimizer cannot select MPP mode due to reasons other than cost estimation, when you use the `EXPLAIN` statement to view the execution plan, a warning is returned to explain the reason, for example:


sorry to be frank, this sentence is quite messy, so I think we have to reorganize the sentence. How about "When TiDB optimizer choose non-MPP plan unexpectly, we can use explain statement to check out the plan and the warnings which explain why it doesn't apply MPP mode". For example

hanfei1991 · 2021-06-21T04:42:30Z

I think the use of "the" is not accurate in this document. We don't say "the TiFlash engine" or "the TiDB optimizer" because there is only one TiFlash engine and TiDB optimizer. I'm not English expert so that's only my intuition.

hanfei1991 · 2021-06-21T04:55:56Z

+
+> **Note:**
+>
+> When `tidb_enforce_mpp=1` takes effect, the TiDB optimizer will ignore the cost estimation to choose MPP mode. However, TiDB will not choose the MPP mode if other factors that do not support the MPP mode occur, such as no TiFlash copy, the replication of TiFlash copies is not completed, and statements contain operators or functions that are not supported by the MPP mode.


It's not natural to use "copy" to describe the follower role in a cluster. The most often used term is "replica". "if other factors that do not support the MPP mode occur" is a little awkward. If I wrote it, I would say "However, there are still some cases that block MPP mode, such as missing TiFlash replica for some tables ..."

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

Joyinqin · 2021-06-21T12:15:32Z

@TomShawn @hanfei1991 @LittleFall PTAL again, thx~

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

TomShawn

LGTM

ti-chi-bot · 2021-06-22T07:55:08Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

TomShawn

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.

Details

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

ti-chi-bot · 2021-06-22T08:43:49Z

@hanfei1991: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

Details

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

TomShawn · 2021-06-22T08:44:50Z

/remove-status LGT1
/status LGT2

TomShawn · 2021-06-22T08:45:00Z

/merge

ti-chi-bot · 2021-06-22T08:45:02Z

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

Details

Commit hash: 559bf35

ti-chi-bot · 2021-06-22T08:48:01Z

In response to a cherrypick label: new pull request created: #5819.

add docs for enforce mpp

91ceb39

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2021

ti-chi-bot requested a review from TomShawn June 18, 2021 10:02

ti-chi-bot added missing-translation-status This PR does not have translation status info. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 18, 2021

Joyinqin requested a review from LittleFall June 18, 2021 10:03

fix links

00d7995

ti-chi-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 18, 2021

Joyinqin added 3 commits June 21, 2021 10:01

Merge branch 'master' into add-mpp

172ed0b

update

f87615d

Update use-tiflash.md

4572fbf

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 21, 2021

Joyinqin marked this pull request as ready for review June 21, 2021 03:11

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 21, 2021

hanfei1991 reviewed Jun 21, 2021

View reviewed changes

TomShawn added the status/require-change Needs the author to address comments. label Jun 21, 2021

refine

321226f

hanfei1991 reviewed Jun 21, 2021

View reviewed changes

update

b556a79

Joyinqin and others added 2 commits June 21, 2021 19:31

Apply suggestions from code review

389b45c

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

update suggestions

df3a111

Update system-variables.md

b5d633c

TomShawn reviewed Jun 22, 2021

View reviewed changes

Comment thread system-variables.md Outdated

TomShawn reviewed Jun 22, 2021

View reviewed changes

Comment thread tidb-configuration-file.md Outdated

TomShawn reviewed Jun 22, 2021

View reviewed changes

Comment thread tiflash/use-tiflash.md Outdated

TomShawn reviewed Jun 22, 2021

View reviewed changes

Comment thread tiflash/use-tiflash.md Outdated

Apply suggestions from code review

a9eb738

Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>

TomShawn added the needs-cherry-pick-release-5.1 label Jun 22, 2021

TomShawn approved these changes Jun 22, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 22, 2021

TomShawn removed the status/require-change Needs the author to address comments. label Jun 22, 2021

hanfei1991 reviewed Jun 22, 2021

View reviewed changes

Comment thread tiflash/use-tiflash.md Outdated

TomShawn added 2 commits June 22, 2021 16:28

Update tiflash/use-tiflash.md

1ad21c8

Update tiflash/use-tiflash.md

559bf35

TomShawn requested a review from hanfei1991 June 22, 2021 08:42

hanfei1991 approved these changes Jun 22, 2021

View reviewed changes

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 Jun 22, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jun 22, 2021

ti-chi-bot merged commit e0f8e00 into pingcap:master Jun 22, 2021

ti-chi-bot mentioned this pull request Jun 22, 2021

add docs for enforce mpp (#5811) #5819

Merged

12 tasks

ti-chi-bot added a commit that referenced this pull request Jun 22, 2021

add docs for enforce mpp (#5811) (#5819)

ff81e85


		### `enforce-mpp`

		+ Determines whether to ignore the optimizer cost estimation, and force TiFlash's MPP mode to execute queries.

Conversation

Joyinqin commented Jun 18, 2021

First-time contributors' checklist

What is changed, added or deleted? (Required)

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

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

Do your changes match any of the following descriptions?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hanfei1991 commented Jun 21, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joyinqin commented Jun 21, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomShawn left a comment

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot commented Jun 22, 2021

Uh oh!

Uh oh!

ti-chi-bot commented Jun 22, 2021

Uh oh!

TomShawn commented Jun 22, 2021

Uh oh!

TomShawn commented Jun 22, 2021

Uh oh!

ti-chi-bot commented Jun 22, 2021

Uh oh!

ti-chi-bot commented Jun 22, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants