PageStorage: Fine grained lock on mvcc map by JaySon-Huang · Pull Request #4137 · pingcap/tiflash

JaySon-Huang · 2022-02-24T10:14:40Z

What problem does this PR solve?

Issue Number: ref #3594

Problem Summary:
Now function PageDirectory::gc and PageDirectory::getEntriesByBlobIds take the lock on the whole mvcc map for the iterating.
By utilizing std::map::insert that "No iterators or references are invalidated" https://en.cppreference.com/w/cpp/container/map/insert, we can refine the lock granularity to be more smaller.

What is changed and how it works?

Refine the lock granularity in function PageDirectory::gc and PageDirectory::getEntriesByBlobIds
Replace some test macro for EXPECT_TRUE(isSameEntry(...

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-02-24T10:14:41Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

jiaqizho
lidezhu

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.

JaySon-Huang · 2022-02-24T10:14:55Z

/run-unit-test

sre-bot · 2022-02-24T10:25:00Z

Coverage for changed files

Filename                                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                                     343               119    65.31%          39                 6    84.62%         729               220    69.82%         208                85    59.13%
BlobStore.h                                         7                 2    71.43%           7                 2    71.43%          25                11    56.00%           0                 0         -
PageDirectory.cpp                                 192                43    77.60%          23                 4    82.61%         445               101    77.30%         142                38    73.24%
PageDirectory.h                                    16                 4    75.00%          14                 4    71.43%          52                15    71.15%           2                 1    50.00%
PageEntry.h                                         1                 1     0.00%           1                 1     0.00%           3                 3     0.00%           0                 0         -
tests/entries_helper.h                             66                37    43.94%          11                 4    63.64%         240               114    52.50%          32                21    34.38%
tests/gtest_blob_store.cpp                       3478               559    83.93%          16                 0   100.00%         691                 0   100.00%        1080               508    52.96%
tests/gtest_collapsing_page_directory.cpp         373                77    79.36%           4                 0   100.00%         102                 0   100.00%         110                52    52.73%
tests/gtest_page_directory.cpp                   4720               750    84.11%          26                 0   100.00%         855                 6    99.30%        1454               728    49.93%
tests/gtest_wal_store.cpp                        1690               254    84.97%          19                 4    78.95%         436                 9    97.94%         546               267    51.10%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                           10886              1846    83.04%         160                25    84.38%        3578               479    86.61%        3574              1700    52.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16687      9492             43.12%    184835  95847        48.14%

full coverage report (for internal network access only)

JaySon-Huang · 2022-02-24T10:25:17Z

/run-unit-test

sre-bot · 2022-02-24T10:36:12Z

Coverage for changed files

Filename                                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                                     343               119    65.31%          39                 6    84.62%         729               220    69.82%         208                85    59.13%
BlobStore.h                                         7                 2    71.43%           7                 2    71.43%          25                11    56.00%           0                 0         -
PageDirectory.cpp                                 192                43    77.60%          23                 4    82.61%         445               101    77.30%         142                38    73.24%
PageDirectory.h                                    16                 4    75.00%          14                 4    71.43%          52                15    71.15%           2                 1    50.00%
PageEntry.h                                         1                 1     0.00%           1                 1     0.00%           3                 3     0.00%           0                 0         -
tests/entries_helper.h                             66                37    43.94%          11                 4    63.64%         240               114    52.50%          32                21    34.38%
tests/gtest_blob_store.cpp                       3478               559    83.93%          16                 0   100.00%         691                 0   100.00%        1080               508    52.96%
tests/gtest_collapsing_page_directory.cpp         373                77    79.36%           4                 0   100.00%         102                 0   100.00%         110                52    52.73%
tests/gtest_page_directory.cpp                   4720               750    84.11%          26                 0   100.00%         855                 6    99.30%        1454               728    49.93%
tests/gtest_wal_store.cpp                        1690               254    84.97%          19                 4    78.95%         436                 9    97.94%         546               267    51.10%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                           10886              1846    83.04%         160                25    84.38%        3578               479    86.61%        3574              1700    52.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16687      9492             43.12%    184835  95869        48.13%

full coverage report (for internal network access only)

jiaqizho · 2022-02-28T05:07:50Z

dbms/src/Storages/Page/V3/PageDirectory.cpp

+PageDirectory::getEntriesByBlobIds(const std::vector<BlobFileId> & blob_ids) const
 {
-    std::map<BlobFileId, PageIdAndVersionedEntries> blob_versioned_entries;
+    std::unordered_set<BlobFileId> blob_id_set;


why we need change vector to set here?

I use it for these checking in line 575 to 581:
https://github.com/pingcap/tics/blob/7e4acdcebe5e9a3a38ceb5a896db89616a4b4673/dbms/src/Storages/Page/V3/PageDirectory.cpp#L575-L581

jiaqizho

LGTM

JaySon-Huang · 2022-03-01T10:20:40Z

/merge

ti-chi-bot · 2022-03-01T10:20:41Z

@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-03-01T10:20:46Z

@JaySon-Huang: /merge in this pull request requires 2 approval(s).

Details

In response to this:

/merge

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-01T10:31:10Z

Coverage for changed files

Filename                                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                                     343               119    65.31%          39                 6    84.62%         729               220    69.82%         208                85    59.13%
BlobStore.h                                         7                 2    71.43%           7                 2    71.43%          25                11    56.00%           0                 0         -
PageDirectory.cpp                                 192                43    77.60%          23                 4    82.61%         446               101    77.35%         142                38    73.24%
PageDirectory.h                                    16                 4    75.00%          14                 4    71.43%          52                15    71.15%           2                 1    50.00%
PageEntry.h                                         1                 1     0.00%           1                 1     0.00%           3                 3     0.00%           0                 0         -
tests/entries_helper.h                             66                37    43.94%          11                 4    63.64%         240               114    52.50%          32                21    34.38%
tests/gtest_blob_store.cpp                       3478               559    83.93%          16                 0   100.00%         691                 0   100.00%        1080               508    52.96%
tests/gtest_collapsing_page_directory.cpp         373                77    79.36%           4                 0   100.00%         102                 0   100.00%         110                52    52.73%
tests/gtest_page_directory.cpp                   4720               750    84.11%          26                 0   100.00%         855                 6    99.30%        1454               728    49.93%
tests/gtest_wal_store.cpp                        1690               254    84.97%          19                 4    78.95%         436                 9    97.94%         546               267    51.10%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                           10886              1846    83.04%         160                25    84.38%        3579               479    86.62%        3574              1700    52.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16687      9492             43.12%    184836  95832        48.15%

full coverage report (for internal network access only)

JaySon-Huang · 2022-03-02T03:01:33Z

/merge

ti-chi-bot · 2022-03-02T03:01:33Z

@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-03-02T03:01:39Z

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

Details

Commit hash: 997930f

sre-bot · 2022-03-02T03:17:25Z

Coverage for changed files

Filename                                      Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                                     343               119    65.31%          39                 6    84.62%         729               220    69.82%         208                85    59.13%
BlobStore.h                                         7                 2    71.43%           7                 2    71.43%          25                11    56.00%           0                 0         -
PageDirectory.cpp                                 192                43    77.60%          23                 4    82.61%         446               101    77.35%         142                38    73.24%
PageDirectory.h                                    16                 4    75.00%          14                 4    71.43%          52                15    71.15%           2                 1    50.00%
PageEntry.h                                         1                 1     0.00%           1                 1     0.00%           3                 3     0.00%           0                 0         -
tests/entries_helper.h                             66                37    43.94%          11                 4    63.64%         240               114    52.50%          32                21    34.38%
tests/gtest_blob_store.cpp                       3478               559    83.93%          16                 0   100.00%         691                 0   100.00%        1080               508    52.96%
tests/gtest_collapsing_page_directory.cpp         373                77    79.36%           4                 0   100.00%          95                 0   100.00%         110                52    52.73%
tests/gtest_page_directory.cpp                   4720               750    84.11%          26                 0   100.00%         855                 6    99.30%        1454               728    49.93%
tests/gtest_wal_store.cpp                        1690               254    84.97%          19                 4    78.95%         436                 9    97.94%         546               267    51.10%
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                           10886              1846    83.04%         160                25    84.38%        3572               479    86.59%        3574              1700    52.43%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
16696      9467             43.30%    186829  95625        48.82%

full coverage report (for internal network access only)

JaySon-Huang added 2 commits February 24, 2022 15:01

Add suppression for running asan

bf15079

Simpify some test

80a6bec

JaySon-Huang requested a review from jiaqizho February 24, 2022 10:14

ti-chi-bot added 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. labels Feb 24, 2022

fine grained lock on mvcc map

a906d66

JaySon-Huang force-pushed the ps-fine-grained-lock branch from 8e4d5ca to a906d66 Compare February 24, 2022 10:18

Add comments

f030a20

refine the lock in gcApply

7e4acdc

jiaqizho reviewed Feb 28, 2022

View reviewed changes

jiaqizho approved these changes Mar 1, 2022

View reviewed changes

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

Merge branch 'master' into ps-fine-grained-lock

997930f

lidezhu approved these changes Mar 2, 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 Mar 2, 2022

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

ti-chi-bot merged commit 6611d54 into pingcap:master Mar 2, 2022

JaySon-Huang deleted the ps-fine-grained-lock branch March 2, 2022 06:11

Conversation

JaySon-Huang commented Feb 24, 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 Feb 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JaySon-Huang commented Feb 24, 2022

Uh oh!

sre-bot commented Feb 24, 2022

Coverage for changed files

Coverage summary

Uh oh!

JaySon-Huang commented Feb 24, 2022

Uh oh!

sre-bot commented Feb 24, 2022

Coverage for changed files

Coverage summary

Uh oh!

jiaqizho Feb 28, 2022

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Feb 28, 2022 • 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!

jiaqizho left a comment

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Mar 1, 2022

Uh oh!

ti-chi-bot commented Mar 1, 2022

Uh oh!

ti-chi-bot commented Mar 1, 2022

Uh oh!

sre-bot commented Mar 1, 2022

Coverage for changed files

Coverage summary

Uh oh!

JaySon-Huang commented Mar 2, 2022

Uh oh!

ti-chi-bot commented Mar 2, 2022

Uh oh!

ti-chi-bot commented Mar 2, 2022

Uh oh!

sre-bot commented Mar 2, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JaySon-Huang commented Feb 24, 2022 •

edited

Loading

ti-chi-bot commented Feb 24, 2022 •

edited

Loading

JaySon-Huang Feb 28, 2022 •

edited

Loading