auto pass through hashagg #9167

guo-shaoge · 2024-06-27T13:55:55Z

What problem does this PR solve?

Issue Number: ref #9196

Problem Summary:

What is changed and how it works?

This PR mainly includes four modifications:

AutoPassThroughHashAggContext: Includes the state control logic, with five states: Init, Adjust, Selective, PreHashAgg, and PassThrough. State transitions occur based on the pre-aggregation effect of each block.
AutoPassThroughHashAggBlockInputStream/AutoPassThroughAggregateTransform: Executor logic that call the AutoPassThroughHashAggContext. It can support sending blocks in advance.
Aggregator: support only_lookup and collect_hit_rate
AutoPassThroughHashHelper: Help generate agg func column by original column.

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

support auto pass through hashagg

Signed-off-by: guo-shaoge <shaoge1994@163.com>

2. auto_pass: support context Signed-off-by: guo-shaoge <shaoge1994@163.com>

Signed-off-by: guo-shaoge <shaoge1994@163.com>

…ough_hashagg

Signed-off-by: guo-shaoge <shaoge1994@163.com>

del only_lookup template for HashMethodBase Signed-off-by: guo-shaoge <shaoge1994@163.com>

Signed-off-by: guo-shaoge <shaoge1994@163.com>

windtalker · 2024-08-05T00:27:21Z

dbms/src/Flash/Planner/PhysicalPlan.cpp

@@ -312,10 +313,20 @@ String PhysicalPlan::toString() const
    return PhysicalPlanVisitor::visitToString(root_node);
 }

+void recursiveSetBlockInputStreamParent(BlockInputStreamPtr self, const IBlockInputStream * parent)
+{


I think you need first check if parent is set, otherwise it may meet performance issues like #4494

windtalker · 2024-08-05T00:46:47Z

dbms/src/Flash/tests/gtest_compute_server.cpp

@@ -12,12 +12,17 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.

+#include <AggregateFunctions/AggregateFunctionFactory.h>


Is this todo finished?

windtalker · 2024-08-05T00:59:51Z

dbms/src/Operators/AutoPassThroughHashAggContext.cpp

+{
+void AutoPassThroughHashAggContext::onBlockAuto(Block & block)
+{
+    RUNTIME_CHECK_MSG(!already_start_to_get_data, "Shouldn't insert into HashMap if already start to get data");


aggregator->executoeOnBlock() could only aggregate part of the data in block, is this case considered?

I add some comments about this: https://github.com/pingcap/tiflash/pull/9167/files#diff-7a1eb53e61cb04f74b8789e3f95315eb173b7af0b9c2c151348471a773bf2af4R73

windtalker · 2024-08-05T01:01:19Z

dbms/src/Operators/AutoPassThroughHashAggContext.cpp

+void AutoPassThroughHashAggContext::forceState()
+{
+    if (many_data[0]->need_spill)
+        state = State::PassThrough;


will the hash table be converted to block after check need_spill is true?

When need_spill is true, follow will happend:

stop insert new data into hashtable

will convert hashtable to block in advance(getDataInAdvance()) to help reduce memory.

windtalker · 2024-08-05T01:03:55Z

dbms/src/Operators/AutoPassThroughHashAggHelper.cpp

+#undef DISPATCH_NUMBER_INNER
+#undef DISPATCH_NUMBER_OUTER
+#undef FOR_NUMBER_TYPES_INNER
+#undef FOR_NUMBER_TYPES_OUTER


you mean when user input is sum(long), TiDB will convert it to sum(cast(long to decimal))?

Signed-off-by: guo-shaoge <shaoge1994@163.com>

windtalker · 2024-08-06T09:02:00Z

dbms/src/Flash/Planner/PhysicalPlan.cpp

 void PhysicalPlan::buildBlockInputStream(DAGPipeline & pipeline, Context & context, size_t max_streams)
 {
    RUNTIME_CHECK(root_node);
    root_node->buildBlockInputStream(pipeline, context, max_streams);
+    pipeline.transform([](auto & stream) { recursiveSetBlockInputStreamParent(stream, nullptr); });


I think you need call this recursiveSetBlockInputStreamParent in BlockInputStreamPtr Planner::execute()

Signed-off-by: guo-shaoge <shaoge1994@163.com>

close pingcap#8982 Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

This reverts commit c3be1a9.

windtalker

LGTM

ti-chi-bot · 2024-08-07T10:07:31Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: windtalker, yibin87

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [windtalker,yibin87]

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 · 2024-08-07T10:07:31Z

[LGTM Timeline notifier]

Timeline:

2024-07-31 08:02:45.257739586 +0000 UTC m=+342881.537787657: ☑️ agreed by yibin87.
2024-08-07 10:07:30.752590965 +0000 UTC m=+432980.619690053: ☑️ agreed by windtalker.

guo-shaoge added 21 commits June 3, 2024 10:45

selective: updateWeakHash32 done

cd6cbea

Signed-off-by: guo-shaoge <shaoge1994@163.com>

selective: scatterTo

bf2af85

Signed-off-by: guo-shaoge <shaoge1994@163.com>

selective: scatter

2e3662d

Signed-off-by: guo-shaoge <shaoge1994@163.com>

mpp writer

dab162f

Signed-off-by: guo-shaoge <shaoge1994@163.com>

1. Aggregator: support collect_hit_rate and only_lookup

fd21bf1

2. auto_pass: support context Signed-off-by: guo-shaoge <shaoge1994@163.com>

execution: support auto pass operator/stream

5e803e2

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix wrong check

957c8e1

Signed-off-by: guo-shaoge <shaoge1994@163.com>

interpter support auto pass through

82883fe

Signed-off-by: guo-shaoge <shaoge1994@163.com>

unit test

7b5320e

Signed-off-by: guo-shaoge <shaoge1994@163.com>

Merge branch 'master' of github.com:pingcap/tiflash into auto_passthr…

f9105ec

…ough_hashagg

unit test for auto pass context

b469fa2

Signed-off-by: guo-shaoge <shaoge1994@163.com>

unit test for executor

59422ca

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

0db738e

Signed-off-by: guo-shaoge <shaoge1994@163.com>

getPassThroughBlock && integration unit test

a7e8036

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix AutoPassThroughHashAggContext/ColumnString todo

3b588c2

Signed-off-by: guo-shaoge <shaoge1994@163.com>

updateWeakHash32 declaration

6d646d9

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix lookupImpl && fix getNotFoundRows && getPassThroughBlock()

4df0268

Signed-off-by: guo-shaoge <shaoge1994@163.com>

use findKey for only_lookup;

605c4ba

del only_lookup template for HashMethodBase Signed-off-by: guo-shaoge <shaoge1994@163.com>

row_limit_unit

3aa40cc

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix some todo

1db168a

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

c30b4bf

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jun 27, 2024

guo-shaoge changed the title ~~Auto passthrough hashagg~~ auto pass through hashagg Jun 27, 2024

guo-shaoge added 4 commits June 28, 2024 15:47

tipb

047f4ed

Signed-off-by: guo-shaoge <shaoge1994@163.com>

del setHitBit

c0018b7

Signed-off-by: guo-shaoge <shaoge1994@163.com>

statistics

a7d444b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

9fb0cb1

Signed-off-by: guo-shaoge <shaoge1994@163.com>

ti-chi-bot bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jun 30, 2024

guo-shaoge added 3 commits July 31, 2024 19:43

aggresive pass through policy

cb8098c

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine stream

411633a

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fix cycle reference

8a508a1

Signed-off-by: guo-shaoge <shaoge1994@163.com>

windtalker reviewed Aug 5, 2024

View reviewed changes

refine setParent; add spillable case

c75ac8c

Signed-off-by: guo-shaoge <shaoge1994@163.com>

guo-shaoge requested a review from windtalker August 5, 2024 15:58

guo-shaoge added 11 commits August 6, 2024 00:13

refine forceState()

dcfd99c

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine case(spill not-spill)

8c5960b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine

1cd2063

Signed-off-by: guo-shaoge <shaoge1994@163.com>

del spill case

a8bf0f0

Signed-off-by: guo-shaoge <shaoge1994@163.com>

clear spill callback for auto pass

4c04f55

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine spill case

c02793f

Signed-off-by: guo-shaoge <shaoge1994@163.com>

refine comment

cd9ed17

Signed-off-by: guo-shaoge <shaoge1994@163.com>

update continusous limit logic

19bf804

Signed-off-by: guo-shaoge <shaoge1994@163.com>

finish spillable stage

35f5fc7

Signed-off-by: guo-shaoge <shaoge1994@163.com>

fmt

0100f6b

Signed-off-by: guo-shaoge <shaoge1994@163.com>

Merge branch 'master' into auto_passthrough_hashagg

76a1e34

windtalker reviewed Aug 7, 2024

View reviewed changes

guo-shaoge and others added 4 commits August 7, 2024 17:55

fix recursiveSetBlockInputStreamParent

a5af40c

Signed-off-by: guo-shaoge <shaoge1994@163.com>

Compression: add microbenchmark (pingcap#9293)

c3be1a9

close pingcap#8982 Signed-off-by: Lloyd-Pottiger <yan1579196623@gmail.com>

Revert "Compression: add microbenchmark (pingcap#9293)"

7c49f3f

This reverts commit c3be1a9.

Merge branch 'master' into auto_passthrough_hashagg

d1447f2

guo-shaoge requested a review from windtalker August 7, 2024 10:01

windtalker approved these changes Aug 7, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Aug 7, 2024

ti-chi-bot bot merged commit 1289b43 into pingcap:master Aug 7, 2024
5 checks passed

JaySon-Huang deleted the auto_passthrough_hashagg branch August 8, 2024 02:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

auto pass through hashagg #9167

auto pass through hashagg #9167

guo-shaoge commented Jun 27, 2024 •

edited

Loading

windtalker Aug 5, 2024

guo-shaoge Aug 5, 2024

windtalker Aug 5, 2024

windtalker Aug 5, 2024

guo-shaoge Aug 5, 2024

windtalker Aug 5, 2024

guo-shaoge Aug 5, 2024

windtalker Aug 5, 2024

windtalker Aug 6, 2024

guo-shaoge Aug 7, 2024

windtalker left a comment

ti-chi-bot bot commented Aug 7, 2024

ti-chi-bot bot commented Aug 7, 2024

auto pass through hashagg #9167

auto pass through hashagg #9167

Conversation

guo-shaoge commented Jun 27, 2024 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

windtalker left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Aug 7, 2024

ti-chi-bot bot commented Aug 7, 2024

[LGTM Timeline notifier]

guo-shaoge commented Jun 27, 2024 •

edited

Loading