Skip to content

Test: Test CancelMPPTask in UT#5732

Merged
ywqzzy merged 41 commits intopingcap:masterfrom
ywqdev:test_cancel_simple
Sep 8, 2022
Merged

Test: Test CancelMPPTask in UT#5732
ywqzzy merged 41 commits intopingcap:masterfrom
ywqdev:test_cancel_simple

Conversation

@ywqzzy
Copy link
Contributor

@ywqzzy ywqzzy commented Aug 30, 2022 •
edited
Loading

What problem does this PR solve?

Issue Number: ref #4609

Problem Summary:

What is changed and how it works?

  1. Start multiple compute services using different Context in UT.
  2. Run tasks and compare result. In this condition, the root node should create multiple ExchangeReceiver to fetch result from different tasks, the merge the result.
  3. Run tasks, then cancel all tasks of one query, and ensure all tasks of the query is aborted.

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

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Aug 30, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • SeaRise
  • yibin87

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.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels Aug 30, 2022
@ywqzzy ywqzzy changed the title [DNM]Test: Test cancel task in UT. [DNM]Test: Test cancel task in UT Aug 30, 2022
@ti-chi-bot ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 30, 2022
@ywqzzy ywqzzy changed the title [DNM]Test: Test cancel task in UT [DNM]Test: Test CancelMPPTask in UT Aug 30, 2022
ywqzzy
ywqzzy commented Aug 30, 2022
View reviewed changes
dbms/src/Flash/tests/gtest_cancel_tasks.cpp Outdated
.scan("test_db", "test_table_1")
.aggregation({Max(col("s1"))}, {col("s2"), col("s3")})
.project({"max(s1)"}));
MockComputeServerManager::instance().cancelQuery(start_ts);
Copy link
Contributor Author

@ywqzzy ywqzzy Aug 30, 2022

Choose a reason for hiding this comment

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

Can write the test more concise

@yibin87
Copy link
Contributor

yibin87 commented Sep 5, 2022 •
edited
Loading

I know that, gtest will defaultly start many background threads for stroage usage. Do the PR suffer from this also? And are these threads really needed?

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 5, 2022

I know that, gtest will defaultly start many background threads for stroage usage. Do the PR suffer from this also? And are these threads really needed?

Yes, I didn't change anything related to storage UT. But in this pr, we don't need to use storage components. Maybe I can simply disable storage related threads in every UT in compute engine UT.

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 5, 2022

I know that, gtest will defaultly start many background threads for stroage usage. Do the PR suffer from this also? And are these threads really needed?

Yes, I didn't change anything related to storage UT. But in this pr, we don't need to use storage components. Maybe I can simply disable storage related threads in every UT in compute engine UT.

Tracked this issue in #5782

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 5, 2022

/rebuild

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 5, 2022

/build

@ywqzzy ywqzzy removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 5, 2022
@ywqzzy ywqzzy requested a review from SeaRise September 5, 2022 10:33
yibin87
yibin87 approved these changes Sep 7, 2022
View reviewed changes
Copy link
Contributor

@yibin87 yibin87 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-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 7, 2022
@ti-chi-bot 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 7, 2022
windtalker
windtalker reviewed Sep 7, 2022
View reviewed changes
ywqzzy
ywqzzy commented Sep 7, 2022
View reviewed changes
@ywqzzy ywqzzy requested a review from windtalker September 7, 2022 08:18
@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 8, 2022

/merge

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 8, 2022

@ywqzzy: 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.

Details

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
Copy link
Member

ti-chi-bot commented Sep 8, 2022

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

DetailsCommit hash: ef5c774

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 8, 2022
@sre-bot
Copy link
Collaborator

sre-bot commented Sep 8, 2022

Coverage for changed files

Filename                                          Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataStreams/MockTableScanBlockInputStream.cpp          15                 0   100.00%           3                 0   100.00%          32                 1    96.88%          12                 1    91.67%
DataStreams/MockTableScanBlockInputStream.h             2                 0   100.00%           2                 0   100.00%           4                 0   100.00%           0                 0         -
Debug/MockComputeServerManager.cpp                     28                 7    75.00%          12                 2    83.33%          84                18    78.57%          18                 5    72.22%
Debug/astToExecutor.cpp                               604               132    78.15%          54                 3    94.44%        1617               353    78.17%         608               177    70.89%
Debug/dbgFuncCoprocessor.cpp                          441               329    25.40%          46                30    34.78%         936               636    32.05%         296               225    23.99%
Debug/dbgFuncCoprocessor.h                              1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/Coprocessor/MockSourceStream.h                   29                 3    89.66%           1                 0   100.00%          42                 1    97.62%          16                 2    87.50%
Flash/FlashService.cpp                                444               266    40.09%          22                10    54.55%         345               190    44.93%         114                83    27.19%
Flash/FlashService.h                                    5                 3    40.00%           4                 2    50.00%          14                 7    50.00%           0                 0         -
Flash/Mpp/MPPTaskManager.cpp                          140                 8    94.29%          16                 1    93.75%         212                23    89.15%          62                18    70.97%
Flash/Mpp/MPPTaskManager.h                              1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/tests/gtest_compute_server.cpp                  183                49    73.22%           8                 0   100.00%         319                 0   100.00%          56                28    50.00%
Interpreters/Context.cpp                              536               300    44.03%         172                75    56.40%        1134               613    45.94%         286               204    28.67%
Interpreters/Context.h                                 13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Server/FlashGrpcServerHolder.cpp                      221                68    69.23%           8                 0   100.00%         161                30    81.37%          60                25    58.33%
TestUtils/ExecutorTestUtils.cpp                        48                 7    85.42%          14                 1    92.86%         127                11    91.34%          42                 5    88.10%
TestUtils/ExecutorTestUtils.h                           3                 1    66.67%           3                 1    66.67%          18                13    27.78%           0                 0         -
TestUtils/MPPTaskTestUtils.cpp                         31                 4    87.10%          11                 2    81.82%          83                15    81.93%          14                 2    85.71%
TestUtils/MPPTaskTestUtils.h                            6                 1    83.33%           3                 0   100.00%          13                 3    76.92%           2                 1    50.00%
TestUtils/TiFlashTestEnv.cpp                           47                 5    89.36%           8                 0   100.00%         116                11    90.52%          24                 6    75.00%
TestUtils/TiFlashTestEnv.h                             19                 2    89.47%           6                 0   100.00%          29                 3    89.66%          12                 3    75.00%
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                2817              1190    57.76%         408               132    67.65%        5301              1933    63.54%        1622               785    51.60%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18685      8130             56.49%    216121  83545        61.34%

full coverage report (for internal network access only)

@ywqzzy
Copy link
Contributor Author

ywqzzy commented Sep 8, 2022

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Sep 8, 2022

Coverage for changed files

Filename                                          Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DataStreams/MockTableScanBlockInputStream.cpp          15                 0   100.00%           3                 0   100.00%          32                 1    96.88%          12                 1    91.67%
DataStreams/MockTableScanBlockInputStream.h             2                 0   100.00%           2                 0   100.00%           4                 0   100.00%           0                 0         -
Debug/MockComputeServerManager.cpp                     28                 7    75.00%          12                 2    83.33%          84                18    78.57%          18                 5    72.22%
Debug/astToExecutor.cpp                               604               132    78.15%          54                 3    94.44%        1617               353    78.17%         608               177    70.89%
Debug/dbgFuncCoprocessor.cpp                          441               329    25.40%          46                30    34.78%         936               636    32.05%         296               225    23.99%
Debug/dbgFuncCoprocessor.h                              1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/Coprocessor/MockSourceStream.h                   29                 3    89.66%           1                 0   100.00%          42                 1    97.62%          16                 2    87.50%
Flash/FlashService.cpp                                444               255    42.57%          22                10    54.55%         345               183    46.96%         114                79    30.70%
Flash/FlashService.h                                    5                 3    40.00%           4                 2    50.00%          14                 7    50.00%           0                 0         -
Flash/Mpp/MPPTaskManager.cpp                          140                 8    94.29%          16                 1    93.75%         212                23    89.15%          62                18    70.97%
Flash/Mpp/MPPTaskManager.h                              1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/tests/gtest_compute_server.cpp                  183                49    73.22%           8                 0   100.00%         319                 0   100.00%          56                28    50.00%
Interpreters/Context.cpp                              536               300    44.03%         172                75    56.40%        1134               613    45.94%         286               204    28.67%
Interpreters/Context.h                                 13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Server/FlashGrpcServerHolder.cpp                      221                68    69.23%           8                 0   100.00%         161                30    81.37%          60                25    58.33%
TestUtils/ExecutorTestUtils.cpp                        48                 7    85.42%          14                 1    92.86%         127                11    91.34%          42                 5    88.10%
TestUtils/ExecutorTestUtils.h                           3                 1    66.67%           3                 1    66.67%          18                13    27.78%           0                 0         -
TestUtils/MPPTaskTestUtils.cpp                         31                 4    87.10%          11                 2    81.82%          83                15    81.93%          14                 2    85.71%
TestUtils/MPPTaskTestUtils.h                            6                 1    83.33%           3                 0   100.00%          13                 3    76.92%           2                 1    50.00%
TestUtils/TiFlashTestEnv.cpp                           47                 5    89.36%           8                 0   100.00%         116                11    90.52%          24                 6    75.00%
TestUtils/TiFlashTestEnv.h                             19                 2    89.47%           6                 0   100.00%          29                 3    89.66%          12                 3    75.00%
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                2817              1179    58.15%         408               132    67.65%        5301              1926    63.67%        1622               781    51.85%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18686      8130             56.49%    216173  83553        61.35%

full coverage report (for internal network access only)

@ywqzzy ywqzzy merged commit 235dd1b into pingcap:master Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yibin87 yibin87 yibin87 approved these changes

@xzhangxian1008 xzhangxian1008 Awaiting requested review from xzhangxian1008

@windtalker windtalker Awaiting requested review from windtalker

+1 more reviewer

@SeaRise SeaRise SeaRise approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ywqzzy @ti-chi-bot @sre-bot @yibin87 @windtalker @SeaRise