PageStorage: Fix lints on some files by JaySon-Huang · Pull Request #3952 · pingcap/tiflash

JaySon-Huang · 2022-01-27T04:20:11Z

What problem does this PR solve?

Issue Number: close #xxx

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: JaySon-Huang <tshent@qq.com>

ti-chi-bot · 2022-01-27T04:20:12Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

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.

Details

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

Signed-off-by: JaySon-Huang <tshent@qq.com>

jiaqizho · 2022-01-27T05:35:05Z

dbms/src/Storages/Page/V2/PageStorage.cpp

        // Legacy and checkpoint files will be removed from `page_files` after `tryCompact`.
        LegacyCompactor compactor(*this, write_limiter, read_limiter);
        PageFileSet page_files_to_archive;
+        auto files_to_compact = std::move(page_files);


just

PageFileSet page_files_rc; std::tie(page_files_rc, page_files_to_archive, gc_context.num_bytes_written_in_compact_legacy) = compactor.tryCompact(std::move(page_files), writing_files_snapshot);

and use it in layer?

compactor.tryCompact will take the files_to_compact and split it into page_files, page_files_to_archive . And later in line 1075, we need to apply compactor.tryMigrate on the page_files.
So we can't use a page_files_rc that only live inside this block.

https://github.com/pingcap/tics/blob/a48d12bf2f364da13b03eca7de778e4acb318c3e/dbms/src/Storages/Page/V2/PageStorage.cpp#L1060-L1075

Signed-off-by: JaySon-Huang <tshent@qq.com>

NOLINTBEGIN only effective in clang-14 Signed-off-by: JaySon-Huang <tshent@qq.com>

JaySon-Huang · 2022-01-27T13:35:29Z

/merge

ti-chi-bot · 2022-01-27T13:35:30Z

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

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-01-27T13:35:36Z

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

Details

Commit hash: 83dae16

sre-bot · 2022-01-27T13:48:08Z

Coverage for changed files

Filename                                                        Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Server/tests/gtest_server_config.cpp                               1660               289    82.59%          10                 0   100.00%         333                 3    99.10%         508               257    49.41%
Storages/Page/PageStorage.h                                          16                 8    50.00%          16                 8    50.00%          53                20    62.26%           0                 0         -
Storages/Page/V1/PageStorage.cpp                                    337               257    23.74%          23                13    43.48%         677               491    27.47%         264               204    22.73%
Storages/Page/V1/PageStorage.h                                       10                 8    20.00%          10                 8    20.00%          10                 8    20.00%           0                 0         -
Storages/Page/V2/PageStorage.cpp                                    518               146    71.81%          36                 3    91.67%         880               163    81.48%         350               144    58.86%
Storages/Page/V2/PageStorage.h                                       41                 9    78.05%          16                 3    81.25%          44                 3    93.18%          20                 9    55.00%
Storages/Page/V2/tests/gtest_data_compactor.cpp                     320                87    72.81%           1                 0   100.00%          83                 2    97.59%          98                46    53.06%
Storages/Page/V2/tests/gtest_page_storage_multi_writers.cpp         420               420     0.00%          22                22     0.00%         192               192     0.00%          94                94     0.00%
Storages/Transaction/RegionPersister.cpp                             85                25    70.59%          12                 2    83.33%         168                33    80.36%          46                21    54.35%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                              3407              1249    63.34%         146                59    59.59%        2440               915    62.50%        1380               775    43.84%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16270      9693             40.42%    177911  98228        44.79%

full coverage report (for internal network access only)

sre-bot · 2022-01-27T14:08:12Z

Coverage for changed files

Filename                                                        Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Server/tests/gtest_server_config.cpp                               1660               289    82.59%          10                 0   100.00%         333                 3    99.10%         508               257    49.41%
Storages/Page/PageStorage.h                                          16                 8    50.00%          16                 8    50.00%          53                20    62.26%           0                 0         -
Storages/Page/V1/PageStorage.cpp                                    337               257    23.74%          23                13    43.48%         677               491    27.47%         264               204    22.73%
Storages/Page/V1/PageStorage.h                                       10                 8    20.00%          10                 8    20.00%          10                 8    20.00%           0                 0         -
Storages/Page/V2/PageStorage.cpp                                    518               146    71.81%          36                 3    91.67%         880               163    81.48%         350               144    58.86%
Storages/Page/V2/PageStorage.h                                       41                 9    78.05%          16                 3    81.25%          44                 3    93.18%          20                 9    55.00%
Storages/Page/V2/tests/gtest_data_compactor.cpp                     320                87    72.81%           1                 0   100.00%          83                 2    97.59%          98                46    53.06%
Storages/Page/V2/tests/gtest_page_storage_multi_writers.cpp         420               420     0.00%          22                22     0.00%         192               192     0.00%          94                94     0.00%
Storages/Transaction/RegionPersister.cpp                             85                25    70.59%          12                 2    83.33%         168                33    80.36%          46                21    54.35%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                                              3407              1249    63.34%         146                59    59.59%        2440               915    62.50%        1380               775    43.84%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16270      9694             40.42%    177902  98229        44.78%

full coverage report (for internal network access only)

JaySon-Huang added 6 commits January 27, 2022 10:48

Remove traversePageEntries

cbab4c0

Signed-off-by: JaySon-Huang <tshent@qq.com>

Fix lints in PageStorage V2

f1f4678

Signed-off-by: JaySon-Huang <tshent@qq.com>

Fix lints in PageStorage V1

fbc2897

Signed-off-by: JaySon-Huang <tshent@qq.com>

Fix lints in PageStorage V3

53f4c35

Signed-off-by: JaySon-Huang <tshent@qq.com>

Refactor ut

7fc84e7

Signed-off-by: JaySon-Huang <tshent@qq.com>

Fix page_ctl

cfb3a05

Signed-off-by: JaySon-Huang <tshent@qq.com>

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 Jan 27, 2022

JaySon-Huang mentioned this pull request Jan 27, 2022

Implememt pagestorage v3 #3909

Merged

12 tasks

Format files

6ca1117

Signed-off-by: JaySon-Huang <tshent@qq.com>

jiaqizho reviewed Jan 27, 2022

View reviewed changes

JaySon-Huang added 2 commits January 27, 2022 14:06

Fix lints

65e8c16

Signed-off-by: JaySon-Huang <tshent@qq.com>

Fix lints

a48d12b

NOLINTBEGIN only effective in clang-14 Signed-off-by: JaySon-Huang <tshent@qq.com>

jiaqizho approved these changes Jan 27, 2022

View reviewed changes

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

Merge branch 'master' into fix_ps_lint

83dae16

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

JaySon-Huang mentioned this pull request Jan 27, 2022

Introduce PageStorage V3 to reduce CPU usage and write amplification #3594

Closed

76 tasks

Merge branch 'master' into fix_ps_lint

a9365d3

ti-chi-bot merged commit 997e42d into pingcap:master Jan 27, 2022

JaySon-Huang deleted the fix_ps_lint branch January 27, 2022 14:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PageStorage: Fix lints on some files#3952

PageStorage: Fix lints on some files#3952
ti-chi-bot merged 11 commits intopingcap:masterfrom
JaySon-Huang:fix_ps_lint

JaySon-Huang commented Jan 27, 2022

Uh oh!

ti-chi-bot commented Jan 27, 2022 •

edited

Loading

Uh oh!

jiaqizho Jan 27, 2022

Uh oh!

JaySon-Huang Jan 27, 2022

Uh oh!

JaySon-Huang commented Jan 27, 2022

Uh oh!

ti-chi-bot commented Jan 27, 2022

Uh oh!

ti-chi-bot commented Jan 27, 2022

Uh oh!

sre-bot commented Jan 27, 2022

Uh oh!

sre-bot commented Jan 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

JaySon-Huang commented Jan 27, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

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

Uh oh!

Uh oh!

jiaqizho Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Jan 27, 2022

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Jan 27, 2022

Uh oh!

ti-chi-bot commented Jan 27, 2022

Uh oh!

ti-chi-bot commented Jan 27, 2022

Uh oh!

sre-bot commented Jan 27, 2022

Coverage for changed files

Coverage summary

Uh oh!

sre-bot commented Jan 27, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ti-chi-bot commented Jan 27, 2022 •

edited

Loading