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

explain: Add joins #3922

Merged
merged 35 commits into from Nov 4, 2020
Merged

explain: Add joins #3922

merged 35 commits into from Nov 4, 2020

Conversation

ghost
Copy link

@ghost ghost commented Sep 17, 2020

What is changed, added or deleted? (Required)

Fixes pingcap/tidb#18576

This is a followup PR to #3858 - but they can be reviewed independently, and merge out of order.

This PR does not add this page to the TOC. I will only do that once all PRs are merged (this is one ~3-4 followup PRs).

The content here is based on/duplicates query-execution-plan. I will merge all of the replacement pages, and then update query-execution-plan (soon to be called explain-overview) as a last step, and in the same PR which adds this page to the TOC.

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

  • master (the latest development version)
  • 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)?

  • This PR is translated from:
  • Other reference link(s):

Do your changes match any of the following descriptions?

  • Delete files
  • Change aliases
  • Have version specific changes
  • Might cause conflicts

@ghost ghost added status/PTAL This PR is ready for reviewing. needs-cherry-pick-4.0 labels Sep 17, 2020
@ghost ghost mentioned this pull request Sep 17, 2020
9 tasks
@TomShawn TomShawn self-assigned this Sep 17, 2020
@TomShawn TomShawn added size/large Changes of a large size. translation/doing This PR's assignee is translating this PR. labels Sep 17, 2020
@ghost
Copy link
Author

ghost commented Sep 17, 2020

I have improved the examples to be reproducible. I would like to expand this to explain more about hash joins, but I hit a bug on pingcap/tidb#20085 This has been fixed. The examples now show a hash join on disk.

explain-joins.md Outdated Show resolved Hide resolved
explain-joins.md Outdated Show resolved Hide resolved
explain-joins.md Outdated Show resolved Hide resolved
explain-joins.md Outdated Show resolved Hide resolved
@ti-srebot
Copy link
Contributor

@SunRunAway, @zz-jason, @TomShawn, PTAL.

@SunRunAway
Copy link
Contributor

Should the related variables be involved? like tidb_hash_join_concurrency, tidb_index_join_batch_size, tidb_index_lookup_join_concurrency

@ghost
Copy link
Author

ghost commented Sep 22, 2020

Should the related variables be involved? like tidb_hash_join_concurrency, tidb_index_join_batch_size, tidb_index_lookup_join_concurrency

@SunRunAway I have added this in ad7c61d PTAL. Thx :-)

@ghost ghost mentioned this pull request Sep 23, 2020
9 tasks
@ti-srebot
Copy link
Contributor

@SunRunAway, @zz-jason, @TomShawn, PTAL.

explain-joins.md Outdated

### Runtime Statistics

If [`tidb_mem_quota_query`](/system-variables.md#tidb_mem_quota_query) (default: 1GB) is exceeded, TiDB will attempt to use temporary storage provided that `oom-use-tmp-storage=TRUE` (default). This means that the hash table used as part of the hash join is created on disk. Runtime statistics, such as memory usage are visible in the `execution info` of `EXPLAIN ANALYZE`. The following example shows the output of `EXPLAIN ANALYZE` with a 1GB (default) `tidb_mem_quota_query` quota, and a 500MB quota. At 500MB the hash table spills to disk:
Copy link
Member

Choose a reason for hiding this comment

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

This means that the hash table used as part of the hash join is created on disk.

@SunRunAway please confirm: is the build side data cached in memory spilled to disk or the hash table itself is spilled to disk?

Copy link
Author

Choose a reason for hiding this comment

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

I think we should update https://docs.pingcap.com/tidb/stable/system-variables#tidb_mem_quota_query as well so its a bit clearer what this does.

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, it is that build side data cached in memory spilled to disk, but we should also consider the spilling for hash table in the future.

Copy link
Member

@zz-jason zz-jason Sep 29, 2020

Choose a reason for hiding this comment

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

It's better to clearly describe the current behavior on the document. we can update the document once we spilled hash table to disk.

BTW, for performance reason, it's not recommended to spill hash table to disk. There should be other methods to achieve the same goal.

Copy link
Author

Choose a reason for hiding this comment

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

I have attempted to clarify this in bcb16dd - PTAL, thx :-)

Null not nil and others added 2 commits November 3, 2020 07:28
Co-authored-by: TomShawn <41534398+TomShawn@users.noreply.github.com>
Copy link
Contributor

@TomShawn TomShawn 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-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 4, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 4, 2020
@TomShawn TomShawn added the status/can-merge Indicates a PR has been approved by a committer. label Nov 4, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@nullnotnil merge failed.

@TomShawn TomShawn merged commit b108b9e into pingcap:master Nov 4, 2020
ti-srebot pushed a commit to ti-srebot/docs that referenced this pull request Nov 4, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot ti-srebot mentioned this pull request Nov 4, 2020
9 tasks
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #4147

@TomShawn TomShawn added the requires-followup This PR requires a follow-up task after being merged. label Nov 4, 2020
@TomShawn
Copy link
Contributor

TomShawn commented Nov 4, 2020

Follow-up: TOC

TomShawn pushed a commit that referenced this pull request Nov 4, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

Co-authored-by: Null not nil <67764674+nullnotnil@users.noreply.github.com>
@TomShawn TomShawn assigned CharLotteiu and unassigned TomShawn Nov 4, 2020
@ti-srebot ti-srebot mentioned this pull request Nov 9, 2020
9 tasks
@TomShawn TomShawn removed the requires-followup This PR requires a follow-up task after being merged. label Nov 16, 2020
@TomShawn TomShawn 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 Nov 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/large Changes of a large size. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. status/PTAL This PR is ready for reviewing. translation/done This PR has been translated from English into Chinese and updated to pingcap/docs-cn in a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation for users about Joins
6 participants