.*: refine `executeQuery` #6043

SeaRise · 2022-09-27T08:42:40Z

What problem does this PR solve?

Issue Number: ref #5900

Problem Summary:

use Flash::executeDAG to replace Interpreters::executeQueryImpl

What is changed and how it works?

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

ti-chi-bot · 2022-09-27T08:42:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bestwoody
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.

SeaRise · 2022-09-27T08:44:45Z

/run-all-tests

SeaRise · 2022-09-27T09:02:06Z

/run-unit-test

SeaRise · 2022-09-27T09:10:34Z

/run-integration-test

sre-bot · 2022-09-27T09:29:34Z

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGDriver.cpp         132                80    39.39%           5                 2    60.00%         149                48    67.79%          34                19    44.12%
Flash/executeQuery.cpp                   43                 6    86.05%           3                 0   100.00%          53                 0   100.00%          20                 5    75.00%
Interpreters/Context.cpp                535               290    45.79%         173                71    58.96%        1133               590    47.93%         286               200    30.07%
Interpreters/Context.h                   11                 5    54.55%          11                 5    54.55%          11                 5    54.55%           0                 0         -
Interpreters/executeQuery.cpp           200               163    18.50%          17                11    35.29%         418               359    14.11%         116               110     5.17%
TestUtils/ExecutorTestUtils.cpp          67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                   988               554    43.93%         229                91    60.26%        1977              1039    47.45%         506               340    32.81%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18361      7522             59.03%    214893  77424        63.97%

full coverage report (for internal network access only)

SeaRise · 2022-09-27T09:40:31Z

/run-integration-test

SeaRise · 2022-09-27T09:41:50Z

/hold

SeaRise · 2022-09-27T09:47:45Z

/hold cancel

SeaRise · 2022-09-27T09:53:06Z

/run-all-tests

SeaRise · 2022-09-27T10:09:01Z

/rebuild

SeaRise · 2022-09-27T10:09:12Z

/run-integration-test

SeaRise · 2022-09-27T10:10:46Z

/run-integration-test

dbms/src/Flash/executeQuery.cpp

sre-bot · 2022-09-27T10:21:23Z

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGDriver.cpp         132                80    39.39%           5                 2    60.00%         149                48    67.79%          34                19    44.12%
Flash/Mpp/MPPTask.cpp                   494               116    76.52%          24                 1    95.83%         433                85    80.37%         182                79    56.59%
Flash/executeQuery.cpp                   43                 6    86.05%           3                 0   100.00%          53                 0   100.00%          20                 5    75.00%
Interpreters/Context.cpp                535               290    45.79%         173                71    58.96%        1133               590    47.93%         286               200    30.07%
Interpreters/Context.h                   11                 5    54.55%          11                 5    54.55%          11                 5    54.55%           0                 0         -
Interpreters/executeQuery.cpp           200               163    18.50%          17                11    35.29%         418               359    14.11%         116               110     5.17%
TestUtils/ExecutorTestUtils.cpp          67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1482               670    54.79%         253                92    63.64%        2410              1124    53.36%         688               419    39.10%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18361      7521             59.04%    214888  77418        63.97%

full coverage report (for internal network access only)

windtalker · 2022-09-28T01:39:13Z

dbms/src/Flash/executeQuery.cpp

+    }
+}
+
+BlockIO executeDAG(IQuerySource & dag, Context & context, bool internal)


We can remove the IQuerySource related code since we don't reuse executeQuery?

Yes, but here is some relevant code that needs to be cleaned up.
We can remove the IQuerySource in the next pr.

In particular, the code related to the DAGQuerySource, perhaps we could remove the IQuerySource along with the DAGQueryBlock.

windtalker

LGTM

SeaRise · 2022-09-29T07:44:18Z

/run-all-tests

SeaRise · 2022-09-29T08:01:19Z

/merge

ti-chi-bot · 2022-09-29T08:01:20Z

@SeaRise: 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-09-29T08:01:22Z

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

Commit hash: 979b66b

sre-bot · 2022-09-29T08:16:31Z

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGDriver.cpp         132                80    39.39%           5                 2    60.00%         149                48    67.79%          34                19    44.12%
Flash/Mpp/MPPTask.cpp                   494               116    76.52%          24                 1    95.83%         433                85    80.37%         182                79    56.59%
Flash/executeQuery.cpp                   44                 6    86.36%           4                 0   100.00%          58                 0   100.00%          20                 5    75.00%
Interpreters/Context.cpp                535               290    45.79%         173                71    58.96%        1133               590    47.93%         286               200    30.07%
Interpreters/Context.h                   11                 5    54.55%          11                 5    54.55%          11                 5    54.55%           0                 0         -
Interpreters/executeQuery.cpp           200               163    18.50%          17                11    35.29%         418               359    14.11%         116               110     5.17%
TestUtils/ExecutorTestUtils.cpp          67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1483               670    54.82%         254                92    63.78%        2415              1124    53.46%         688               419    39.10%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18362      7521             59.04%    214893  77444        63.96%

full coverage report (for internal network access only)

sre-bot · 2022-09-29T08:36:01Z

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGDriver.cpp         132                80    39.39%           5                 2    60.00%         144                48    66.67%          34                19    44.12%
Flash/Mpp/MPPTask.cpp                   438                82    81.28%          22                 0   100.00%         395                61    84.56%         164                68    58.54%
Flash/executeQuery.cpp                   44                 6    86.36%           4                 0   100.00%          58                 0   100.00%          20                 5    75.00%
Interpreters/Context.cpp                535               291    45.61%         173                71    58.96%        1133               590    47.93%         286               202    29.37%
Interpreters/Context.h                   11                 5    54.55%          11                 5    54.55%          11                 5    54.55%           0                 0         -
Interpreters/executeQuery.cpp           200               163    18.50%          17                11    35.29%         418               359    14.11%         116               110     5.17%
TestUtils/ExecutorTestUtils.cpp          67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1427               637    55.36%         252                91    63.89%        2372              1100    53.63%         670               410    38.81%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18392      7481             59.32%    215305  77027        64.22%

full coverage report (for internal network access only)

sre-bot · 2022-09-29T08:43:45Z

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Flash/Coprocessor/DAGDriver.cpp         132                80    39.39%           5                 2    60.00%         149                48    67.79%          34                19    44.12%
Flash/Mpp/MPPTask.cpp                   494               115    76.72%          24                 1    95.83%         433                85    80.37%         182                78    57.14%
Flash/executeQuery.cpp                   44                 6    86.36%           4                 0   100.00%          58                 0   100.00%          20                 5    75.00%
Interpreters/Context.cpp                535               290    45.79%         173                71    58.96%        1133               590    47.93%         286               200    30.07%
Interpreters/Context.h                   11                 5    54.55%          11                 5    54.55%          11                 5    54.55%           0                 0         -
Interpreters/executeQuery.cpp           200               163    18.50%          17                11    35.29%         418               359    14.11%         116               110     5.17%
TestUtils/ExecutorTestUtils.cpp          67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1483               669    54.89%         254                92    63.78%        2415              1124    53.46%         688               418    39.24%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18362      7521             59.04%    214893  77455        63.96%

full coverage report (for internal network access only)

update

32919d1

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 27, 2022

SeaRise requested a review from windtalker September 27, 2022 09:28

u

dd39f74

SeaRise removed the request for review from windtalker September 27, 2022 09:41

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2022

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 27, 2022

fix

1d10d42

SeaRise requested a review from windtalker September 27, 2022 09:48

windtalker reviewed Sep 27, 2022

View reviewed changes

dbms/src/Flash/executeQuery.cpp Outdated Show resolved Hide resolved

windtalker reviewed Sep 27, 2022

View reviewed changes

dbms/src/Flash/executeQuery.cpp Outdated Show resolved Hide resolved

address comment

979b66b

SeaRise requested a review from windtalker September 27, 2022 10:43

windtalker reviewed Sep 28, 2022

View reviewed changes

SeaRise requested a review from windtalker September 28, 2022 01:48

windtalker approved these changes Sep 28, 2022

View reviewed changes

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

SeaRise requested review from ywqzzy and bestwoody September 28, 2022 02:18

bestwoody approved these changes Sep 29, 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 Sep 29, 2022

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

Merge branch 'master' into execute_query_for_flash

0d7a9d9

ti-chi-bot merged commit 84a71f0 into pingcap:master Sep 29, 2022

SeaRise deleted the execute_query_for_flash branch September 29, 2022 08:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

.*: refine `executeQuery` #6043

.*: refine `executeQuery` #6043

SeaRise commented Sep 27, 2022 •

edited

Loading

ti-chi-bot commented Sep 27, 2022 •

edited

Loading

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

sre-bot commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

sre-bot commented Sep 27, 2022

windtalker Sep 28, 2022

SeaRise Sep 28, 2022

SeaRise Sep 28, 2022

windtalker Sep 28, 2022

windtalker left a comment

SeaRise commented Sep 29, 2022

SeaRise commented Sep 29, 2022

ti-chi-bot commented Sep 29, 2022

ti-chi-bot commented Sep 29, 2022

sre-bot commented Sep 29, 2022

sre-bot commented Sep 29, 2022

sre-bot commented Sep 29, 2022

.*: refine executeQuery #6043

.*: refine executeQuery #6043

Conversation

SeaRise commented Sep 27, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Sep 27, 2022 • edited Loading

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

sre-bot commented Sep 27, 2022

Coverage for changed files

Coverage summary

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

SeaRise commented Sep 27, 2022

sre-bot commented Sep 27, 2022

Coverage for changed files

Coverage summary

windtalker Sep 28, 2022

Choose a reason for hiding this comment

SeaRise Sep 28, 2022

Choose a reason for hiding this comment

SeaRise Sep 28, 2022

Choose a reason for hiding this comment

windtalker Sep 28, 2022

Choose a reason for hiding this comment

windtalker left a comment

Choose a reason for hiding this comment

SeaRise commented Sep 29, 2022

SeaRise commented Sep 29, 2022

ti-chi-bot commented Sep 29, 2022

ti-chi-bot commented Sep 29, 2022

sre-bot commented Sep 29, 2022

Coverage for changed files

Coverage summary

sre-bot commented Sep 29, 2022

Coverage for changed files

Coverage summary

sre-bot commented Sep 29, 2022

Coverage for changed files

Coverage summary

.*: refine `executeQuery` #6043

.*: refine `executeQuery` #6043

SeaRise commented Sep 27, 2022 •

edited

Loading

ti-chi-bot commented Sep 27, 2022 •

edited

Loading