Support using FineGrainedShuffle Info for Join&Agg #6279

yibin87 · 2022-11-09T03:32:31Z

What problem does this PR solve?

Issue Number: ref #6157

Problem Summary:

What is changed and how it works?

FineGrainedShuffle info will be added in tidb side if the plan satisfies certain rules. In TiFlash side, both Join and Agg operators can benefit from these info:

Aggregation can parallelly process all streams without merging phase.
Join can build hash tables without any synchronizations, since rows with same key will be scattered to the same stream(It is guranteed by FineGrainedShuffleWriter).

A little refact of HashBaseWriteHelper to make parameters sequence more reasonable and other little changes.

Most changed logic is protected by enable_fine_grained_shuffle flag which is set false now in TiDB. Will add another separate PR to add gtests for FinedGrainedShuffleJoin&Agg then.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

None

Signed-off-by: yibin <[email protected]>

ti-chi-bot · 2022-11-09T03:32:33Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
windtalker

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.

…d_grained_agg Signed-off-by: yibin <[email protected]>

…sh into phase3_join_agg_fine_grain

Signed-off-by: yibin <[email protected]>

yibin87 · 2022-11-10T09:56:04Z

/run-unit-tests

yibin87 · 2022-11-10T09:56:14Z

/run-integration-test

Signed-off-by: yibin <[email protected]>

yibin87 · 2022-11-10T10:26:06Z

/run-all-tests

dbms/src/Interpreters/Join.cpp

dbms/src/Flash/Planner/plans/PhysicalAggregation.cpp

dbms/src/Flash/Planner/plans/PhysicalJoin.cpp

Signed-off-by: yibin <[email protected]>

windtalker · 2022-11-14T02:25:42Z

dbms/src/Flash/Mpp/ExchangeReceiver.cpp

@@ -429,7 +429,8 @@ ExchangeReceiverBase<RPCContext>::ExchangeReceiverBase(
    {
        if (enableFineGrainedShuffle(fine_grained_shuffle_stream_count_))
        {
-            for (size_t i = 0; i < max_streams_; ++i)
+            size_t channel_count = std::min(max_streams_, fine_grained_shuffle_stream_count_);


max_streams_ in ExchangeReceiver is actually useless, I think we can replace max_streams and fine_grained_shuffle_stream_count by output_stream_count and enable_fine_grained_shuffle?

Make sense, updated

windtalker · 2022-11-14T03:11:11Z

dbms/src/Flash/Coprocessor/DAGQueryBlockInterpreter.cpp

+    {
+        auto receiver_source_num_info = getReceiverSourceNumInfo(*build_pipeline.firstStream());
+        RUNTIME_CHECK(receiver_source_num_info.first == 1);
+        shuffle_partition_num = receiver_source_num_info.second;


Why shuffle_partition_num is set to source_num?

It was a wrong design, fixed it.

…e reproducing by hash and fine_grained_shuffle_stream_count Signed-off-by: yibin <[email protected]>

Signed-off-by: yibin <[email protected]>

windtalker · 2022-11-14T08:04:09Z

dbms/src/Flash/Mpp/HashBaseWriterHelper.cpp

@@ -41,44 +41,94 @@ std::vector<MutableColumns> createDestColumns(const Block & sample_block, size_t
    return dest_tbl_cols;
 }

+void fillSelector(size_t rows,
+                  const WeakHash32 & hash,
+                  uint32_t num_bucket,


better to rename num_bucket to num_part?

windtalker · 2022-11-14T08:33:17Z

dbms/src/Interpreters/Join.cpp

+    if (enable_fine_grained_shuffle && rows > 0)
+    {
+        /// TODO: consider adding a virtual column in Sender side to avoid computing cost and potential inconsistency by heterogeneous envs
+        /// Note: 1. Not sure, if inconsistency will do happen in heterogeneous envs


Why there maybe problems in heterogeneous envs?

Added comments to make it clear.

Signed-off-by: yibin <[email protected]>

yibin87 · 2022-11-15T01:28:58Z

/run-all-tests

Signed-off-by: yibin <[email protected]>

dbms/src/DataStreams/MockExchangeReceiverInputStream.cpp

dbms/src/Flash/Mpp/HashBaseWriterHelper.cpp

dbms/src/Flash/Mpp/FineGrainedShuffleWriter.cpp

Signed-off-by: yibin <[email protected]>

SeaRise

Almost LGTM

dbms/src/Interpreters/Join.cpp

SeaRise · 2022-11-15T07:09:24Z

Will add another separate PR to add gtests for FinedGrainedShuffleJoin&Agg then

We can add ut in

Signed-off-by: yibin <[email protected]>

SeaRise

LGTM

windtalker

LGTM

yibin87 · 2022-11-15T09:20:55Z

/merge

ti-chi-bot · 2022-11-15T09:20:56Z

@yibin87: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2022-11-15T09:20:57Z

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

Commit hash: 3c29db0

Support using FineGrained Info for Join&Agg

8329137

Signed-off-by: yibin <[email protected]>

Merge branch 'master' into phase3_join_agg_fine_grain

02db910

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2022

yibin87 added 2 commits November 10, 2022 17:05

Fix prob side segment_index issue, support no-plan mode, support fine…

20776e5

…d_grained_agg Signed-off-by: yibin <[email protected]>

Merge branch 'phase3_join_agg_fine_grain' of github.com:yibin87/tifla…

35769fa

…sh into phase3_join_agg_fine_grain

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 10, 2022

Merge branch 'master' into phase3_join_agg_fine_grain

745f64e

yibin87 changed the title ~~[WIP] Support using FineGrainedShuffle Info for Join&Agg~~ Support using FineGrainedShuffle Info for Join&Agg Nov 10, 2022

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 Nov 10, 2022

yibin87 added 2 commits November 10, 2022 17:15

Little change

6205ff5

Signed-off-by: yibin <[email protected]>

Fix build error

a0c9f47

Signed-off-by: yibin <[email protected]>

yibin87 added 3 commits November 10, 2022 18:09

Little refact

518dee6

Signed-off-by: yibin <[email protected]>

Add comments

74d8f78

Signed-off-by: yibin <[email protected]>

Add some comments

75824b6

Signed-off-by: yibin <[email protected]>

yibin87 requested review from SeaRise, fuzhe1989, windtalker and LittleFall November 10, 2022 10:27

SeaRise reviewed Nov 10, 2022

View reviewed changes

Changes due to review comments

535eaa0

Signed-off-by: yibin <[email protected]>

ti-chi-bot removed the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 11, 2022

windtalker reviewed Nov 14, 2022

View reviewed changes

yibin87 added 2 commits November 14, 2022 15:09

Modify FineGrainedShuffle hash=>stream_idx mapping alogrithm to enabl…

b621b1c

…e reproducing by hash and fine_grained_shuffle_stream_count Signed-off-by: yibin <[email protected]>

Fix little issue

e131536

Signed-off-by: yibin <[email protected]>

SeaRise self-requested a review November 14, 2022 07:24

windtalker reviewed Nov 14, 2022

View reviewed changes

Little changes to review comments

1d1f380

Signed-off-by: yibin <[email protected]>

yibin87 requested a review from windtalker November 15, 2022 01:14

Fix file format issue

97d7a7f

Signed-off-by: yibin <[email protected]>

Fix static analyze issue

bfe76c1

Signed-off-by: yibin <[email protected]>

SeaRise reviewed Nov 15, 2022

View reviewed changes

Changes to review comments

016ed85

Signed-off-by: yibin <[email protected]>

yibin87 requested a review from SeaRise November 15, 2022 05:38

SeaRise reviewed Nov 15, 2022

View reviewed changes

dbms/src/Interpreters/Join.cpp Outdated Show resolved Hide resolved

Changes according to review comments

712114c

Signed-off-by: yibin <[email protected]>

yibin87 requested a review from SeaRise November 15, 2022 07:46

SeaRise approved these changes Nov 15, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 15, 2022

Merge branch 'master' into phase3_join_agg_fine_grain

3c29db0

windtalker approved these changes Nov 15, 2022

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 Nov 15, 2022

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 15, 2022

ti-chi-bot merged commit e77d01c into pingcap:master Nov 15, 2022

yibin87 mentioned this pull request Nov 15, 2022

Join & Aggregation Fine Grained Partition Optimization #6157

Closed

6 tasks

yibin87 mentioned this pull request Aug 3, 2023

Expand the functionality of Local Runtime Filter #7891

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support using FineGrainedShuffle Info for Join&Agg #6279

Support using FineGrainedShuffle Info for Join&Agg #6279

yibin87 commented Nov 9, 2022 •

edited

Loading

ti-chi-bot commented Nov 9, 2022 •

edited

Loading

yibin87 commented Nov 10, 2022

yibin87 commented Nov 10, 2022

yibin87 commented Nov 10, 2022

windtalker Nov 14, 2022

yibin87 Nov 15, 2022 •

edited

Loading

windtalker Nov 14, 2022

yibin87 Nov 15, 2022 •

edited

Loading

windtalker Nov 14, 2022

yibin87 Nov 15, 2022

windtalker Nov 14, 2022

yibin87 Nov 15, 2022 •

edited

Loading

yibin87 commented Nov 15, 2022

SeaRise left a comment

SeaRise commented Nov 15, 2022 •

edited

Loading

SeaRise left a comment

windtalker left a comment

yibin87 commented Nov 15, 2022

ti-chi-bot commented Nov 15, 2022

ti-chi-bot commented Nov 15, 2022

Support using FineGrainedShuffle Info for Join&Agg #6279

Support using FineGrainedShuffle Info for Join&Agg #6279

Conversation

yibin87 commented Nov 9, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 9, 2022 • edited Loading

yibin87 commented Nov 10, 2022

yibin87 commented Nov 10, 2022

yibin87 commented Nov 10, 2022

windtalker Nov 14, 2022

Choose a reason for hiding this comment

yibin87 Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

windtalker Nov 14, 2022

Choose a reason for hiding this comment

yibin87 Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

windtalker Nov 14, 2022

Choose a reason for hiding this comment

yibin87 Nov 15, 2022

Choose a reason for hiding this comment

windtalker Nov 14, 2022

Choose a reason for hiding this comment

yibin87 Nov 15, 2022 • edited Loading

Choose a reason for hiding this comment

yibin87 commented Nov 15, 2022

SeaRise left a comment

Choose a reason for hiding this comment

SeaRise commented Nov 15, 2022 • edited Loading

SeaRise left a comment

Choose a reason for hiding this comment

windtalker left a comment

Choose a reason for hiding this comment

yibin87 commented Nov 15, 2022

ti-chi-bot commented Nov 15, 2022

ti-chi-bot commented Nov 15, 2022

yibin87 commented Nov 9, 2022 •

edited

Loading

ti-chi-bot commented Nov 9, 2022 •

edited

Loading

yibin87 Nov 15, 2022 •

edited

Loading

yibin87 Nov 15, 2022 •

edited

Loading

yibin87 Nov 15, 2022 •

edited

Loading

SeaRise commented Nov 15, 2022 •

edited

Loading