Add some debug friendly utilities for dag request based test framework #4698

yibin87 · 2022-04-18T12:01:50Z

Signed-off-by: yibin huyibin@pingcap.com

What problem does this PR solve?

Issue Number: close #4304

Problem Summary:

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

Signed-off-by: yibin <huyibin@pingcap.com>

ti-chi-bot · 2022-04-18T12:01:52Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

windtalker
ywqzzy

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.

windtalker

LGTM

ywqzzy · 2022-04-19T04:51:19Z

dbms/src/Debug/dbgFuncCoprocessor.cpp

+        for (auto & it : failed_req_msg_vec)
+        {
+            String msg = fmt::format("request {} failed, msg: {}", it.first, it.second);
+            if (first)
+            {
+                merged_msg = msg;
+                first = false;
+            }
+            else
+            {
+                merged_msg = fmt::format("{}\n{}", merged_msg, msg);
+            }
+        }


How about use FmtBuffer?

Nice sugguestion!

ywqzzy · 2022-04-19T05:36:28Z

dbms/src/Debug/dbgFuncCoprocessor.cpp

+
+    bool unequal_flag = false;
+    DAGProperties properties = getDAGProperties("");
+    std::__1::vector<std::__1::pair<DecodedTiKVKeyPtr, DecodedTiKVKeyPtr>> key_ranges = CoprocessorHandler::GenCopKeyRange(req.ranges());


To satisfy my curiosity, Could you tell me why use the "std::__1" namespace?

Ahoh, they were introduced by CLion's extracting method functionality, I'll change it.

dbms/src/Debug/dbgFuncCoprocessor.cpp

Signed-off-by: yibin <huyibin@pingcap.com>

ywqzzy · 2022-04-19T08:01:16Z

dbms/src/Debug/dbgFuncCoprocessor.cpp

@@ -378,28 +378,29 @@ void dbgFuncTiDBQueryFromNaturalDag(Context & context, const ASTs & args, DBGInv
    if (!failed_req_msg_vec.empty())
    {
        output("Invalid");
-        String merged_msg;
+        FmtBuffer msg_buffer;


Sorry, I didn't speak clear that JoinStr function with FmtBuffer can be better

Yeah, that's exactly what I needed. Used joinStr to simplify this process.

Signed-off-by: yibin <huyibin@pingcap.com>

ywqzzy

LGTM

yibin87 · 2022-04-21T02:26:34Z

/merge

ti-chi-bot · 2022-04-21T02:26:35Z

@yibin87: 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-04-21T02:26:36Z

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

Commit hash: 9daae88

sre-bot · 2022-04-21T02:41:20Z

Coverage for changed files

Filename                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
dbgFuncCoprocessor.cpp            417               337    19.18%          37                27    27.03%         866               709    18.13%         294               248    15.65%
dbgNaturalDag.cpp                  84                84     0.00%          11                11     0.00%         165               165     0.00%          48                48     0.00%
dbgNaturalDag.h                     6                 6     0.00%           6                 6     0.00%          16                16     0.00%           0                 0         -
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                             507               427    15.78%          54                44    18.52%        1047               890    15.00%         342               296    13.45%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17132      9492             44.59%    192856  96268        50.08%

full coverage report (for internal network access only)

Add some debug friendly utilities for dag request

1dd2307

Signed-off-by: yibin <huyibin@pingcap.com>

yibin87 requested review from windtalker and ichn-hu April 18, 2022 12:01

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 Apr 18, 2022

yibin87 changed the title ~~Add some debug friendly utilities for dag request~~ Add some debug friendly utilities for dag request based test framework Apr 18, 2022

ywqzzy self-requested a review April 18, 2022 13:04

windtalker approved these changes Apr 19, 2022

View reviewed changes

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

ywqzzy reviewed Apr 19, 2022

View reviewed changes

ywqzzy self-requested a review April 19, 2022 07:17

Change to review comments

084e3dc

Signed-off-by: yibin <huyibin@pingcap.com>

ywqzzy reviewed Apr 19, 2022

View reviewed changes

yibin87 added 2 commits April 21, 2022 09:01

Use fmt_buffer join str to simplify str join operation

567f395

Signed-off-by: yibin <huyibin@pingcap.com>

Fix format issue

9daae88

Signed-off-by: yibin <huyibin@pingcap.com>

yibin87 requested a review from ywqzzy April 21, 2022 01:20

ywqzzy approved these changes Apr 21, 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 Apr 21, 2022

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

Merge branch 'master' into master

b3006bc

ti-chi-bot merged commit 6fe26bd into pingcap:master Apr 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some debug friendly utilities for dag request based test framework #4698

Add some debug friendly utilities for dag request based test framework #4698

yibin87 commented Apr 18, 2022

ti-chi-bot commented Apr 18, 2022 •

edited

Loading

windtalker left a comment

ywqzzy Apr 19, 2022

yibin87 Apr 19, 2022

ywqzzy Apr 19, 2022

yibin87 Apr 19, 2022

ywqzzy Apr 19, 2022

ywqzzy Apr 19, 2022

yibin87 Apr 21, 2022

ywqzzy left a comment

yibin87 commented Apr 21, 2022

ti-chi-bot commented Apr 21, 2022

ti-chi-bot commented Apr 21, 2022

sre-bot commented Apr 21, 2022

Add some debug friendly utilities for dag request based test framework #4698

Add some debug friendly utilities for dag request based test framework #4698

Conversation

yibin87 commented Apr 18, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Apr 18, 2022 • edited Loading

windtalker left a comment

Choose a reason for hiding this comment

ywqzzy Apr 19, 2022

Choose a reason for hiding this comment

yibin87 Apr 19, 2022

Choose a reason for hiding this comment

ywqzzy Apr 19, 2022

Choose a reason for hiding this comment

yibin87 Apr 19, 2022

Choose a reason for hiding this comment

ywqzzy Apr 19, 2022

Choose a reason for hiding this comment

ywqzzy Apr 19, 2022

Choose a reason for hiding this comment

yibin87 Apr 21, 2022

Choose a reason for hiding this comment

ywqzzy left a comment

Choose a reason for hiding this comment

yibin87 commented Apr 21, 2022

ti-chi-bot commented Apr 21, 2022

ti-chi-bot commented Apr 21, 2022

sre-bot commented Apr 21, 2022

Coverage for changed files

Coverage summary

ti-chi-bot commented Apr 18, 2022 •

edited

Loading