PageStorage v3 modules combine. by jiaqizho · Pull Request #3795 · pingcap/tiflash

jiaqizho · 2022-01-04T07:59:02Z

What problem does this PR solve?

Issue Number: a part of #3594

Problem Summary:

What is changed and how it works?

combine PageStorage v3 modules
port stress test into v3, now we can use stress test to test the V3 pagestorage

remain

some tests in pagestorage v3
stress test get some error when run HeavySkewWriteRead.cpp

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

call test: BlobStoreTest.testBlobStoreGcStats2 Error from - BlobStore::write - BlobStore::getBlobFile - DB::LRUCache - std::map The ptr in __hash:87 will be lost. And if i remove the `ro_ids` in BlobStore.h, it won't show again... Anyway `ro_ids` is useless.

ti-chi-bot · 2022-01-04T07:59:03Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

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>

Add test for VersionedEntries. Need merge by hand, please wait...

JaySon-Huang · 2022-01-19T07:18:44Z

dbms/src/Storages/Page/Page.h


 public:
-    inline bool isValid() const { return file_id != 0; }
+    inline bool isValid() const { return file_id != 0 && file_id != INVALID_BLOBFILE_ID; }


Maybe make INVAILD_BLOBFILE_ID be 0?

dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp

dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp

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

JaySon-Huang · 2022-01-19T07:50:50Z

This PR mixes too many changes in different components, suggest splitting these two changes into separate PRs:

Add some debugging utils for PageDirectory: snapshot thread_id, living_time since created; getMaxId/getAllPageIds/gcApply method changes
Changes in BlobStore::gc

flowbehappy · 2022-01-19T10:57:34Z

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


 bool PageStorageImpl::gc(bool not_skip, const WriteLimiterPtr & write_limiter, const ReadLimiterPtr & read_limiter)
 {
+    /// Get all pending external pages and BlobFiles. Note that we should get external pages before BlobFiles.


We should avoid this method being called by more than 1 thread at the same time.

Return false if we didn't do any job this time. So that the scheduler won't call it again in the near future.

flowbehappy · 2022-01-19T11:13:24Z

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


        // TBD: wal apply
    }
+


I think we should hold the lock instance in https://github.com/pingcap/tics/pull/3795/files#diff-1de4d86b0ebcbe77efe3ed6316f11350a64cdff662503b1eb9c48741d19a6b72R380

It is hard to locate which lines with the URL you post when the PR changes lots of lines. You should use "view file" to open the file and then copy the URL inside that file.

Do you mean this lock in this PR? PageStorage: Fix locks on BlobStore, PageDirectory #3897
https://github.com/pingcap/tics/blob/ca65a7b1b3b359c5b3624b7482fef6f69c01e3fc/dbms/src/Storages/Page/V3/PageDirectory.cpp#L285-L288

actually, I have fixed some locking issues in this pr.

In PageDirectory, due to added getSnapshotsStat, then we need to be added write_lock. Before we clear up snapshots in gc.

Fix max_cap may cause inaccurate issues in BlobStore. There is no need to add more locks in other methods of BlobStore, because this may lead to reentrancy problems or bring more lock problems, and we only need to ensure that write/read/gc happens at the same time, and there will be no concurrency problems.

If I'm missing something, I'd prefer to fix it in this PR, or after that, so I can use the adapted stress tool to verify it.

flowbehappy · 2022-01-19T11:20:03Z

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

+{
+extern const Metric PSMVCCNumSnapshots;
+} // namespace CurrentMetrics
+


Add a [[nodiscard]] mark to VersionedPageEntries::acquireLock.

ti-chi-bot · 2022-01-20T05:26:48Z

@jiaqizho: PR needs rebase.

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 kubernetes/test-infra repository.

jiaqizho · 2022-01-20T09:07:23Z

@JaySon-Huang @flowbehappy
split into #3905 #3907 #3908 #3909 #3913 PTAL if you have time to review~

jiaqizho and others added 15 commits December 30, 2021 11:43

[WIP] gc

a99e9e5

[WIP]PageStorage: Mvcc directory GC

eba3dbd

interface

ad95b12

update mvcc gc

42cdc6f

update

5439fc0

add align test

da9a3bc

add mvcc gc test

d5a9f1a

add getGcStats test

0c7d53e

finished gc

adf4db8

finished test

1da2a8a

update

993f69a

fix clang-tidy

abf6228

../dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp

ca5b1fb

Merge branch 'master' into mvcc-gc

8c61ff4

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

jiaqizho changed the title ~~Ps v3 combine~~ [WIP] PageStorage v3 modules combine. Jan 4, 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 Jan 4, 2022

jiaqizho and others added 10 commits January 4, 2022 17:41

update interface

a916d53

changed api

3f718cd

Add test for VersionedEntries

3423d47

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

fix

b79a84d

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

update

2dc0603

Merge branch 'mvcc-gc' into mvcc-gc

2dcface

Merge pull request #6 from JaySon-Huang/mvcc-gc

44f659f

Add test for VersionedEntries. Need merge by hand, please wait...

remove the epoch logical,make test won't failed

00979ce

update mvcc gc test

fbe0161

add tests

ca5610c

merge to master

7bcb83c

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2022

jiaqizho added 4 commits January 13, 2022 11:39

add getMaxId

893d339

update getallpages lock

4608cb6

update

a510e0c

fix stress test error

ce5213c

jiaqizho mentioned this pull request Jan 18, 2022

Refine the GC interface by PageEntriesEdit #3879

Merged

12 tasks

jiaqizho added 5 commits January 18, 2022 14:29

fix 512M limit

1037530

update

b9d9aae

add earse blob

d003012

add gc test

e607d50

fix memory bug

83a6848

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp Outdated Show resolved Hide resolved

dbms/src/Storages/Page/V3/tests/gtest_free_map.cpp Outdated Show resolved Hide resolved

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp Outdated Show resolved Hide resolved

dbms/src/Storages/Page/V3/tests/gtest_blob_store.cpp Outdated Show resolved Hide resolved

JaySon-Huang reviewed Jan 19, 2022

View reviewed changes

dbms/src/Storages/Page/V3/PageDirectory.cpp Outdated Show resolved Hide resolved

dbms/src/Storages/Page/V3/PageDirectory.cpp Outdated Show resolved Hide resolved

flowbehappy reviewed Jan 19, 2022

View reviewed changes

updated

bd76098

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2022

jiaqizho closed this Jan 20, 2022

Conversation

jiaqizho commented Jan 4, 2022 • edited by JaySon-Huang 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 Jan 4, 2022

Uh oh!

JaySon-Huang Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JaySon-Huang commented Jan 19, 2022

Uh oh!

flowbehappy Jan 19, 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!

flowbehappy Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

jiaqizho Jan 20, 2022

Choose a reason for hiding this comment

Uh oh!

flowbehappy Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

ti-chi-bot commented Jan 20, 2022

Uh oh!

jiaqizho commented Jan 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jiaqizho commented Jan 4, 2022 •

edited by JaySon-Huang

Loading

flowbehappy Jan 19, 2022 •

edited

Loading