Enrich the infomation about FullTableScan operator of TiFlash in `explain analyze` #6314

hongyunyan · 2022-11-15T01:45:28Z

What problem does this PR solve?

Issue Number: close #5926

Problem Summary:
Enrich the information about FullTableScan operator of TiFlash in explain analyze.

New information includes:

the count of packs read in the query
the count of packs skipped in the query
the count of rows read in the query
the count of rows skipped in the query
the time of reading related dmfile in the query
the time of loading the related rough set index in the query
the time of creating snapshot in the query

What is changed and how it works?

New class TableScanContext for the metrics of storage level
Enrich the Execution Summary based on the metrics in TableScanContext
Collect the metrics in TableScanContext and support gather the TableScanContext in ExecutionSummaryCollector

Related PRs:
pingcap/tipb#278 merged first)
pingcap/tidb#39216

Examples:

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
create table and insert 50w data, and then run the sql to check the metrics.
No code

Side effects

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

Documentation

Release note

None

…yunyan_add_statistic

ti-chi-bot · 2022-11-15T01:45:29Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

SeaRise
breezewish

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.

…yunyan_add_statistic

hongyunyan · 2022-11-21T05:06:15Z

/cc @SeaRise

hongyunyan · 2022-11-23T13:56:14Z

/run-all-tests

breezewish

The rest LGTM

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp

dbms/src/Flash/Coprocessor/ExecutionSummaryCollector.cpp

dbms/src/Flash/Coprocessor/tests/gtest_ti_remote_block_inputstream.cpp

dbms/src/Storages/DeltaMerge/DMContext.h

dbms/src/Storages/DeltaMerge/File/DMFileReader.h

dbms/src/Storages/DeltaMerge/ScanContext.h

dbms/src/Storages/StorageDeltaMerge.cpp

hongyunyan · 2022-11-25T07:31:52Z

/run-all-tests

hongyunyan · 2022-11-25T08:23:47Z

/run-all-tests

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp

dbms/src/Flash/Coprocessor/ExecutionSummaryCollector.cpp

hongyunyan · 2022-11-28T05:58:36Z

/run-all-tests

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp

dbms/src/Flash/Coprocessor/ExecutionSummary.h

hongyunyan · 2022-11-28T12:22:17Z

/merge

ti-chi-bot · 2022-11-28T12:22:18Z

@hongyunyan: 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-28T12:22:20Z

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

Commit hash: 6d2ee5a

JaySon-Huang

Sorry for the late reply. I think we can avoid the new StorageDeltaMerge::read interface

JaySon-Huang · 2022-12-07T04:01:44Z

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp

+            const auto & scan_context = dag_context.scan_context_map.at(table_scan.getTableScanExecutorID());
+
+            // We want to collect performance metrics in storage level, thus we need read with scan_context here.
+            // while IStorage::read() can't support it, and only StorageDeltaMerge support to read with scan_context to collect the information.
+            // Thus, storage must cast to StorageDeltaMergePtr here to call the corresponding read() function.
+            StorageDeltaMergePtr delta_merge_storage = std::dynamic_pointer_cast<StorageDeltaMerge>(storage);
+            RUNTIME_CHECK_MSG(delta_merge_storage != nullptr, "delta_merge_storage which cast from storage is null");
+            pipeline.streams = delta_merge_storage->read(required_columns, query_info, context, from_stage, max_block_size, max_streams, scan_context);


I think we can put scan_context into qeury_info.mvcc_query_info to avoid creating another StorageDeltaMerge::read interface

hongyunyan added 6 commits November 1, 2022 16:07

for test

a8d6662

for test

29fee70

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

eade0de

…yunyan_add_statistic

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

cee972c

…yunyan_add_statistic

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

60113be

…yunyan_add_statistic

add new full table scan context and support simple metrics

8e5f48b

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 15, 2022

hongyunyan added 5 commits November 15, 2022 09:55

for code format

9501ee6

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

84e837b

…yunyan_add_statistic

for code test

7b8ef7a

change code

7338b15

update proto

f8ce1e6

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

merge conflict

a48bb59

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

hongyunyan added 2 commits November 21, 2022 12:02

update code

50ae9f3

fix typo

860c0e5

hongyunyan changed the title ~~WIP: Enrich the infomation in explain analyze~~ Enrich the infomation in explain analyze Nov 21, 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 21, 2022

This was referenced Nov 21, 2022

Add executor execution summary detail pingcap/tipb#278

Merged

Explain : Support more info from tiflash in explain analyze pingcap/tidb#39216

Merged

hongyunyan requested review from breezewish and JaySon-Huang and removed request for breezewish November 21, 2022 05:05

hongyunyan changed the title ~~Enrich the infomation in explain analyze~~ Enrich the infomation about FullTableScan operator of TiFlash in explain analyze Nov 21, 2022

for sync temp states

ef4e8c5

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 23, 2022

hongyunyan added 2 commits November 23, 2022 21:30

update code

53336f5

update code

9b5c044

breezewish reviewed Nov 25, 2022

View reviewed changes

update code

556aa0b

update code

4bd981e

hongyunyan requested a review from SeaRise November 25, 2022 09:03

breezewish approved these changes Nov 25, 2022

View reviewed changes

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

SeaRise reviewed Nov 25, 2022

View reviewed changes

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp Outdated Show resolved Hide resolved

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp Outdated Show resolved Hide resolved

dbms/src/Flash/Coprocessor/ExecutionSummaryCollector.cpp Outdated Show resolved Hide resolved

update code

58045f0

SeaRise reviewed Nov 28, 2022

View reviewed changes

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp Outdated Show resolved Hide resolved

dbms/src/Flash/Coprocessor/DAGStorageInterpreter.cpp Outdated Show resolved Hide resolved

dbms/src/Flash/Coprocessor/ExecutionSummary.h Show resolved Hide resolved

fix comments

6d2ee5a

SeaRise approved these changes Nov 28, 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 28, 2022

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

ti-chi-bot added 2 commits November 28, 2022 20:22

Merge branch 'master' into hongyunyan_add_statistic

9fac86b

Merge branch 'master' into hongyunyan_add_statistic

f0b86da

ti-chi-bot merged commit 6811b93 into pingcap:master Nov 28, 2022

JaySon-Huang reviewed Dec 7, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enrich the infomation about FullTableScan operator of TiFlash in `explain analyze` #6314

Enrich the infomation about FullTableScan operator of TiFlash in `explain analyze` #6314

hongyunyan commented Nov 15, 2022 •

edited by flowbehappy

Loading

ti-chi-bot commented Nov 15, 2022 •

edited

Loading

hongyunyan commented Nov 21, 2022

hongyunyan commented Nov 23, 2022

breezewish left a comment

hongyunyan commented Nov 25, 2022

hongyunyan commented Nov 25, 2022

hongyunyan commented Nov 28, 2022

hongyunyan commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

JaySon-Huang left a comment

JaySon-Huang Dec 7, 2022

Enrich the infomation about FullTableScan operator of TiFlash in explain analyze #6314

Enrich the infomation about FullTableScan operator of TiFlash in explain analyze #6314

Conversation

hongyunyan commented Nov 15, 2022 • edited by flowbehappy Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

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

hongyunyan commented Nov 21, 2022

hongyunyan commented Nov 23, 2022

breezewish left a comment

Choose a reason for hiding this comment

hongyunyan commented Nov 25, 2022

hongyunyan commented Nov 25, 2022

hongyunyan commented Nov 28, 2022

hongyunyan commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

ti-chi-bot commented Nov 28, 2022

JaySon-Huang left a comment

Choose a reason for hiding this comment

JaySon-Huang Dec 7, 2022

Choose a reason for hiding this comment

Enrich the infomation about FullTableScan operator of TiFlash in `explain analyze` #6314

Enrich the infomation about FullTableScan operator of TiFlash in `explain analyze` #6314

hongyunyan commented Nov 15, 2022 •

edited by flowbehappy

Loading

ti-chi-bot commented Nov 15, 2022 •

edited

Loading