Test: Support coprocessor test by ywqzzy · Pull Request #5957 · pingcap/tiflash

ywqzzy · 2022-09-20T06:56:28Z

What problem does this PR solve?

Issue Number: ref #4609

Problem Summary:

What is changed and how it works?

Add test API to run Coprocessor.

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-20T06:56:29Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

LittleFall
SeaRise

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.

dbms/src/Flash/FlashService.cpp

dbms/src/TestUtils/MPPTaskTestUtils.cpp

dbms/src/Interpreters/Context.h

ywqzzy · 2022-09-20T07:37:43Z

/run-all-tests

ywqzzy · 2022-09-20T08:04:51Z

/run-all-tests

ywqzzy · 2022-09-20T08:10:09Z

Note that if we want to make the coprocessor query run indefinitely, we can change MockSourceStream.h:65 to construct an infinite TableScanStream.

sre-bot · 2022-09-20T08:10:28Z

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/MockComputeServerManager.cpp            28                 7    75.00%          12                 2    83.33%          84                18    78.57%          18                 5    72.22%
Debug/dbgFuncCoprocessor.cpp                 441               323    26.76%          46                28    39.13%         936               617    34.08%         296               219    26.01%
Debug/dbgFuncCoprocessor.h                     1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/FlashService.cpp                       436               216    50.46%          25                 8    68.00%         364               167    54.12%         114                75    34.21%
Flash/tests/gtest_compute_server.cpp         194                56    71.13%           9                 0   100.00%         333                 0   100.00%          60                32    46.67%
Interpreters/Context.cpp                     538               299    44.42%         174                75    56.90%        1140               609    46.58%         286               203    29.02%
Interpreters/Context.h                        13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Server/MockComputeClient.h                     8                 2    75.00%           3                 0   100.00%          20                 6    70.00%           4                 2    50.00%
TestUtils/ExecutorTestUtils.cpp               67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
TestUtils/ExecutorTestUtils.h                  3                 1    66.67%           3                 1    66.67%          18                13    27.78%           0                 0         -
TestUtils/MPPTaskTestUtils.cpp                37                 7    81.08%          13                 2    84.62%         115                24    79.13%          18                 5    72.22%
TestUtils/MPPTaskTestUtils.h                   6                 1    83.33%           3                 0   100.00%          13                 3    76.92%           2                 1    50.00%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1772               927    47.69%         322               123    61.80%        3250              1499    53.88%         848               548    35.38%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18868      8083             57.16%    219608  83255        62.09%

full coverage report (for internal network access only)

LittleFall · 2022-09-20T07:17:01Z

dbms/src/Flash/tests/gtest_compute_server.cpp

+}
+CATCH
 } // namespace tests
 } // namespace DB


Suggested change

} // namespace DB

} // namespace DB

LittleFall · 2022-09-20T08:18:54Z

dbms/src/Flash/FlashService.cpp


-    if (!security_config->checkGrpcContext(grpc_context))
+    // For coprocessor test, we don't care about security config.
+    if (!context->isCopTest() && !security_config->checkGrpcContext(grpc_context))


Suggested change

if (!context->isCopTest() && !security_config->checkGrpcContext(grpc_context))

if (unlikely(!context->isCopTest() && !security_config->checkGrpcContext(grpc_context)))

LittleFall · 2022-09-20T08:25:57Z

dbms/src/Flash/FlashService.cpp

-        // For MPP test, we don't care about security config.
-        if (!context->isMPPTest())
+        // For MPP or Cop test, we don't care about security config.
+        if (likely(!(context->isMPPTest() || context->isCopTest())))


can it replace to !context->isTest()?

can it replace to !context->isTest()?

Yes!

test this github feature

LittleFall

see my previous review comment, rest LGTM

…ort_cop_test

JaySon-Huang · 2022-09-20T08:42:30Z

@LittleFall you can use "Request change" to dismiss the LGTM counting

ywqzzy · 2022-09-20T09:53:09Z

/merge

ti-chi-bot · 2022-09-20T09:53:10Z

@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 · 2022-09-20T09:53:12Z

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

Details

Commit hash: 06a5016

ywqzzy · 2022-09-20T10:18:00Z

/run-unit-tests

ywqzzy · 2022-09-20T10:36:55Z

/run-unit-tests

sre-bot · 2022-09-20T11:27:15Z

Coverage for changed files

Filename                                 Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Debug/MockComputeServerManager.cpp            28                 7    75.00%          12                 2    83.33%          84                18    78.57%          18                 5    72.22%
Debug/dbgFuncCoprocessor.cpp                 441               323    26.76%          46                28    39.13%         936               617    34.08%         296               219    26.01%
Debug/dbgFuncCoprocessor.h                     1                 0   100.00%           1                 0   100.00%           1                 0   100.00%           0                 0         -
Flash/FlashService.cpp                       434               217    50.00%          25                 8    68.00%         364               167    54.12%         112                77    31.25%
Flash/tests/gtest_compute_server.cpp         194                56    71.13%           9                 0   100.00%         333                 0   100.00%          60                32    46.67%
Interpreters/Context.cpp                     538               299    44.42%         174                75    56.90%        1140               609    46.58%         286               203    29.02%
Interpreters/Context.h                        13                 5    61.54%          13                 5    61.54%          13                 5    61.54%           0                 0         -
Server/MockComputeClient.h                     8                 2    75.00%           3                 0   100.00%          20                 6    70.00%           4                 2    50.00%
TestUtils/ExecutorTestUtils.cpp               67                10    85.07%          20                 2    90.00%         213                37    82.63%          50                 6    88.00%
TestUtils/ExecutorTestUtils.h                  3                 1    66.67%           3                 1    66.67%          18                13    27.78%           0                 0         -
TestUtils/MPPTaskTestUtils.cpp                37                 7    81.08%          13                 2    84.62%         115                24    79.13%          18                 5    72.22%
TestUtils/MPPTaskTestUtils.h                   6                 1    83.33%           3                 0   100.00%          13                 3    76.92%           2                 1    50.00%
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                       1770               928    47.57%         322               123    61.80%        3250              1499    53.88%         846               550    34.99%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18868      8084             57.15%    219611  83310        62.06%

full coverage report (for internal network access only)

ywqzzy added 4 commits September 19, 2022 16:12

test coprocessor.

00712f8

read response.

e1948a5

run cop tests.

5ba9ab2

refine

549e0b6

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 20, 2022

refine.

9784a5e

SeaRise self-requested a review September 20, 2022 07:00

SeaRise reviewed Sep 20, 2022

View reviewed changes

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

dbms/src/TestUtils/MPPTaskTestUtils.cpp Outdated Show resolved Hide resolved

dbms/src/Interpreters/Context.h Show resolved Hide resolved

refine.

b22744d

fmt

a8578a4

ywqzzy force-pushed the support_cop_test branch from 85790e7 to a8578a4 Compare September 20, 2022 07:57

LittleFall self-requested a review September 20, 2022 08:04

SeaRise approved these changes Sep 20, 2022

View reviewed changes

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

LittleFall previously approved these changes Sep 20, 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 20, 2022

LittleFall added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Sep 20, 2022

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

LittleFall approved these changes Sep 20, 2022

View reviewed changes

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

a698149

…ort_cop_test

address comments

b1c9a67

ywqzzy force-pushed the support_cop_test branch from 0db0ad5 to b1c9a67 Compare September 20, 2022 08:49

Merge branch 'master' into support_cop_test

06a5016

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

ti-chi-bot merged commit c003541 into pingcap:master Sep 20, 2022

LittleFall mentioned this pull request Sep 21, 2022

ti-chi won't decrease LGTM count when lgtm is dismissed. ti-community-infra/tichi#1034

Closed

1 task

ywqzzy mentioned this pull request Oct 8, 2022

Interpreter & Executor & MPP unit test framework #4609

Closed

54 tasks

	if (!context->isCopTest() && !security_config->checkGrpcContext(grpc_context))
	if (unlikely(!context->isCopTest() && !security_config->checkGrpcContext(grpc_context)))

Conversation

ywqzzy commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ywqzzy commented Sep 20, 2022

Uh oh!

ywqzzy commented Sep 20, 2022

Uh oh!

ywqzzy commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sre-bot commented Sep 20, 2022

Coverage for changed files

Coverage summary

Uh oh!

LittleFall Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

LittleFall Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

LittleFall Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

ywqzzy Sep 20, 2022

Choose a reason for hiding this comment

Uh oh!

LittleFall left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Sep 20, 2022

Uh oh!

ywqzzy commented Sep 20, 2022

Uh oh!

ti-chi-bot commented Sep 20, 2022

Uh oh!

ti-chi-bot commented Sep 20, 2022

Uh oh!

ywqzzy commented Sep 20, 2022

Uh oh!

ywqzzy commented Sep 20, 2022

Uh oh!

sre-bot commented Sep 20, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ywqzzy commented Sep 20, 2022 •

edited

Loading

ti-chi-bot commented Sep 20, 2022 •

edited

Loading

ywqzzy commented Sep 20, 2022 •

edited

Loading

LittleFall left a comment •

edited

Loading