Add metric to record end to end time to pre-handle snapshot #3978

CalvinNeo · 2022-02-08T09:30:23Z

Signed-off-by: CalvinNeo [email protected]

What problem does this PR solve?

Issue Number: close #3759

Problem Summary:
We have no end to end metrics of prehandling snapshot, it may be hard to diagnostic some problems without this metrics.

What is changed and how it works?

Add a metrics recording end to end time of prehandling snapshot

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

Add metric to record end to end time to pre-handle snapshot

Signed-off-by: CalvinNeo <[email protected]>

ti-chi-bot · 2022-02-08T09:30:24Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang
jiaqizho

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.

Signed-off-by: CalvinNeo <[email protected]>

JaySon-Huang · 2022-02-09T10:58:01Z

dbms/src/Storages/Transaction/ApplySnapshot.cpp

@@ -297,6 +298,9 @@ std::vector<UInt64> KVStore::preHandleSSTsToDTFiles(
    // Use failpoint to change the expected_block_size for some test cases
    fiu_do_on(FailPoints::force_set_sst_to_dtfile_block_size, { expected_block_size = 3; });

+    Stopwatch watch;
+    SCOPE_EXIT({ GET_METRIC(tiflash_raft_command_duration_seconds, type_apply_snapshot_predecode_e2e).Observe(watch.elapsedSeconds()); });


Maybe just use type_apply_snapshot_predecode in this line and remove ...
https://github.com/pingcap/tics/blob/b8f711014a9760efbd97eeac7e35bbd4403e1f9f/dbms/src/Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp#L82
... instead of adding a "e2e" panel?

/cc @flowbehappy

I think it may be confusing, especially when we using this metrics, we need to first check the version, and then choose the way to interpret this metric.
Meanwhile, I think the original metric is also useful, since it records the consumed time other than schema builder, which I think is also worth recording.

Signed-off-by: CalvinNeo <[email protected]>

CalvinNeo · 2022-02-28T09:22:44Z

According to discussion, I change "Snapshot Predecode Duration" to record end to end time, and "Snapshot Predecode SST to DT Duration" to record SST -> DT time when using DT rather block method. /cc @JaySon-Huang @flowbehappy

CalvinNeo · 2022-03-01T07:45:07Z

/run-all-tests

sre-bot · 2022-03-01T07:58:06Z

Coverage for changed files

Filename                                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/TiFlashMetrics.h                                        18                 0   100.00%          11                 0   100.00%          47                 1    97.87%           8                 0   100.00%
Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp          83                83     0.00%           8                 8     0.00%         148               148     0.00%          46                46     0.00%
Storages/Transaction/ApplySnapshot.cpp                        304               123    59.54%          13                 4    69.23%         471               214    54.56%         150                88    41.33%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                         405               206    49.14%          32                12    62.50%         666               363    45.50%         204               134    34.31%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16715      9481             43.28%    186706  95656        48.77%

full coverage report (for internal network access only)

JaySon-Huang

LGTM

Signed-off-by: CalvinNeo <[email protected]>

jiaqizho

LGTM

CalvinNeo · 2022-03-14T07:04:18Z

/merge

ti-chi-bot · 2022-03-14T07:04:19Z

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

sre-bot · 2022-03-14T07:22:00Z

Coverage for changed files

Filename                                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/TiFlashMetrics.h                                        18                 0   100.00%          11                 0   100.00%          47                 1    97.87%           8                 0   100.00%
Flash/Mpp/MPPTunnel.h                                           8                 8     0.00%           8                 8     0.00%          14                14     0.00%           0                 0         -
Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp          83                83     0.00%           8                 8     0.00%         148               148     0.00%          46                46     0.00%
Storages/Transaction/ApplySnapshot.cpp                        304               123    59.54%          13                 4    69.23%         471               214    54.56%         150                88    41.33%
Storages/Transaction/SchemaBuilder.cpp                        835               794     4.91%          46                42     8.70%        1030               958     6.99%         488               468     4.10%
Storages/Transaction/SchemaGetter.h                             1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                        1249              1009    19.22%          87                63    27.59%        1711              1336    21.92%         692               602    13.01%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16853      9456             43.89%    189609  95854        49.45%

full coverage report (for internal network access only)

CalvinNeo · 2022-03-14T07:32:06Z

/merge

ti-chi-bot · 2022-03-14T07:32:07Z

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

CalvinNeo · 2022-03-14T07:32:22Z

/rebuild

sre-bot · 2022-03-14T07:56:12Z

Coverage for changed files

Filename                                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/TiFlashMetrics.h                                        18                 0   100.00%          11                 0   100.00%          47                 1    97.87%           8                 0   100.00%
Flash/Mpp/MPPTunnel.h                                           8                 8     0.00%           8                 8     0.00%          14                14     0.00%           0                 0         -
Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp          83                83     0.00%           8                 8     0.00%         148               148     0.00%          46                46     0.00%
Storages/Transaction/ApplySnapshot.cpp                        304               123    59.54%          13                 4    69.23%         471               214    54.56%         150                88    41.33%
Storages/Transaction/SchemaBuilder.cpp                        835               794     4.91%          46                42     8.70%        1030               958     6.99%         488               468     4.10%
Storages/Transaction/SchemaGetter.h                             1                 1     0.00%           1                 1     0.00%           1                 1     0.00%           0                 0         -
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                        1249              1009    19.22%          87                63    27.59%        1711              1336    21.92%         692               602    13.01%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16853      9456             43.89%    189609  95875        49.44%

full coverage report (for internal network access only)

JaySon-Huang · 2022-03-14T08:03:28Z

/merge cancel

JaySon-Huang · 2022-03-14T08:11:17Z

@CalvinNeo should pick this commit JaySon-Huang@58968b2 to fix the changes from #4099

Signed-off-by: CalvinNeo <[email protected]>

JaySon-Huang · 2022-03-22T04:00:39Z

/merge

ti-chi-bot · 2022-03-22T04:00:40Z

@JaySon-Huang: 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-03-22T04:00:46Z

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

Commit hash: 83e57ac

sre-bot · 2022-03-22T04:19:29Z

Coverage for changed files

Filename                                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/TiFlashMetrics.h                                        18                 0   100.00%          11                 0   100.00%          47                 1    97.87%           8                 0   100.00%
Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp          83                83     0.00%           8                 8     0.00%         148               148     0.00%          46                46     0.00%
Storages/Transaction/ApplySnapshot.cpp                        304               123    59.54%          13                 4    69.23%         471               214    54.56%         150                88    41.33%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                         405               206    49.14%          32                12    62.50%         666               363    45.50%         204               134    34.31%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17040      9576             43.80%    191887  97041        49.43%

full coverage report (for internal network access only)

JaySon-Huang · 2022-03-22T04:53:37Z

/rebuild

CalvinNeo · 2022-03-22T05:52:14Z

/run-integration-test

ti-chi-bot · 2022-03-22T06:06:42Z

@CalvinNeo: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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.

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.

sre-bot · 2022-03-22T06:28:32Z

Coverage for changed files

Filename                                                  Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Common/TiFlashMetrics.h                                        18                 0   100.00%          11                 0   100.00%          47                 1    97.87%           8                 0   100.00%
Storages/DeltaMerge/SSTFilesToDTFilesOutputStream.cpp          83                83     0.00%           8                 8     0.00%         148               148     0.00%          46                46     0.00%
Storages/Transaction/ApplySnapshot.cpp                        304               123    59.54%          13                 4    69.23%         471               214    54.56%         150                88    41.33%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                         405               206    49.14%          32                12    62.50%         666               363    45.50%         204               134    34.31%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17040      9576             43.80%    191887  97040        49.43%

full coverage report (for internal network access only)

add type_apply_snapshot_predecode_e2e

be1f11f

Signed-off-by: CalvinNeo <[email protected]>

CalvinNeo self-assigned this Feb 8, 2022

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 8, 2022

CalvinNeo changed the title ~~Add metric to record end to end time to pre-handle snapshot~~ [WIP]Add metric to record end to end time to pre-handle snapshot Feb 8, 2022

ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 8, 2022

add metrics

b8f7110

Signed-off-by: CalvinNeo <[email protected]>

ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Feb 9, 2022

CalvinNeo changed the title ~~[WIP]Add metric to record end to end time to pre-handle snapshot~~ Add metric to record end to end time to pre-handle snapshot Feb 9, 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 Feb 9, 2022

CalvinNeo requested a review from JaySon-Huang February 9, 2022 06:32

JaySon-Huang reviewed Feb 9, 2022

View reviewed changes

ti-chi-bot requested a review from flowbehappy February 9, 2022 10:58

Merge branch 'master' into fix3759

f29c5e5

ti-chi-bot added the do-not-merge/needs-triage-completed label Feb 28, 2022

Rename

5fed309

Signed-off-by: CalvinNeo <[email protected]>

ti-chi-bot removed the do-not-merge/needs-triage-completed label Feb 28, 2022

CalvinNeo requested a review from JaySon-Huang February 28, 2022 09:22

Merge branch 'master' into fix3759

11189db

JaySon-Huang approved these changes Mar 2, 2022

View reviewed changes

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

CalvinNeo and others added 2 commits March 8, 2022 16:07

fix

f876cb0

Signed-off-by: CalvinNeo <[email protected]>

Merge branch 'master' into fix3759

c08a440

jiaqizho approved these changes Mar 14, 2022

View reviewed changes

ti-chi-bot removed the status/LGT1 Indicates that a PR has LGTM 1. label Mar 14, 2022

ti-chi-bot added the status/LGT2 Indicates that a PR has LGTM 2. label Mar 14, 2022

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

Merge branch 'master' into fix3759

511081c

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Mar 14, 2022

CalvinNeo and others added 4 commits March 15, 2022 12:37

Merge branch 'master' into fix3759

91a1a19

Merge branch 'master' into fix3759

1c42c13

Merge branch 'fix3759' of github.com:CalvinNeo/tics into fix3759

1562e92

f

83e57ac

Signed-off-by: CalvinNeo <[email protected]>

JaySon-Huang approved these changes Mar 22, 2022

View reviewed changes

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

Merge branch 'master' into fix3759

84dadde

Merge branch 'master' into fix3759

ffd3e40

ti-chi-bot merged commit 2b71569 into pingcap:master Mar 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add metric to record end to end time to pre-handle snapshot #3978

Add metric to record end to end time to pre-handle snapshot #3978

CalvinNeo commented Feb 8, 2022 •

edited

Loading

ti-chi-bot commented Feb 8, 2022 •

edited

Loading

JaySon-Huang Feb 9, 2022 •

edited

Loading

CalvinNeo Feb 9, 2022

CalvinNeo commented Feb 28, 2022 •

edited

Loading

CalvinNeo commented Mar 1, 2022

sre-bot commented Mar 1, 2022

JaySon-Huang left a comment

jiaqizho left a comment

CalvinNeo commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022

sre-bot commented Mar 14, 2022

CalvinNeo commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022

CalvinNeo commented Mar 14, 2022

sre-bot commented Mar 14, 2022

JaySon-Huang commented Mar 14, 2022

JaySon-Huang commented Mar 14, 2022 •

edited

Loading

JaySon-Huang commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

sre-bot commented Mar 22, 2022

JaySon-Huang commented Mar 22, 2022

CalvinNeo commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

sre-bot commented Mar 22, 2022

Add metric to record end to end time to pre-handle snapshot #3978

Add metric to record end to end time to pre-handle snapshot #3978

Conversation

CalvinNeo commented Feb 8, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Feb 8, 2022 • edited Loading

JaySon-Huang Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

CalvinNeo Feb 9, 2022

Choose a reason for hiding this comment

CalvinNeo commented Feb 28, 2022 • edited Loading

CalvinNeo commented Mar 1, 2022

sre-bot commented Mar 1, 2022

Coverage for changed files

Coverage summary

JaySon-Huang left a comment

Choose a reason for hiding this comment

jiaqizho left a comment

Choose a reason for hiding this comment

CalvinNeo commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022

sre-bot commented Mar 14, 2022

Coverage for changed files

Coverage summary

CalvinNeo commented Mar 14, 2022

ti-chi-bot commented Mar 14, 2022

CalvinNeo commented Mar 14, 2022

sre-bot commented Mar 14, 2022

Coverage for changed files

Coverage summary

JaySon-Huang commented Mar 14, 2022

JaySon-Huang commented Mar 14, 2022 • edited Loading

JaySon-Huang commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

sre-bot commented Mar 22, 2022

Coverage for changed files

Coverage summary

JaySon-Huang commented Mar 22, 2022

CalvinNeo commented Mar 22, 2022

ti-chi-bot commented Mar 22, 2022

sre-bot commented Mar 22, 2022

Coverage for changed files

Coverage summary

CalvinNeo commented Feb 8, 2022 •

edited

Loading

ti-chi-bot commented Feb 8, 2022 •

edited

Loading

JaySon-Huang Feb 9, 2022 •

edited

Loading

CalvinNeo commented Feb 28, 2022 •

edited

Loading

JaySon-Huang commented Mar 14, 2022 •

edited

Loading