Add dumpIncrementalCheckpoint utility #6928

breezewish · 2023-03-02T04:15:24Z

What problem does this PR solve?

Issue Number: ref #6827

Problem Summary:

What is changed and how it works?

Add UniversalPageStorage::dumpIncrementalCheckpoint.

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: Wish <breezewish@outlook.com>

ti-chi-bot · 2023-03-02T04:15:26Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang
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.

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

JaySon-Huang · 2023-03-02T05:40:58Z

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

+template <>
+typename PageDirectory<universal::PageDirectoryTrait>::DumpCheckpointResult
+PageDirectory<universal::PageDirectoryTrait>::dumpIncrementalCheckpoint(const DumpCheckpointOptions & options)


What about moving this method to UniversalPageStorage? This method involves both getting snapshot entries from PageDirectory and reading page data from the BlobStore
And we don't need another DumpCheckpointOptions in PageDirectory

Good suggestion! I have moved to UniversalPageStorage.

Signed-off-by: Wish <breezewish@outlook.com>

breezewish · 2023-03-02T16:45:32Z

/run-all-tests

Signed-off-by: Wish <breezewish@outlook.com>

lidezhu

LGTM

Signed-off-by: Wish <breezewish@outlook.com>

JaySon-Huang

LGTM with small comment

JaySon-Huang · 2023-03-03T03:20:01Z

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

+        return;
+    }
+
+    RUNTIME_CHECK(iter->first.sequence == edit.version.sequence); // TODO: If there is a full GC this may be false?


Suggested change

RUNTIME_CHECK(iter->first.sequence == edit.version.sequence); // TODO: If there is a full GC this may be false?

RUNTIME_CHECK(iter->first.sequence == edit.version.sequence, iter->first.sequence, edit.version.sequence); // TODO: If there is a full GC this may be false?

I think this won't happen, but showing the actual value if this do happen is better

JaySon-Huang · 2023-03-03T03:24:42Z

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

+class CPWriteDataSource;
+using CPWriteDataSourcePtr = std::shared_ptr<CPWriteDataSource>;


these seem useless?

Signed-off-by: Wish <breezewish@outlook.com>

breezewish · 2023-03-03T03:32:47Z

/merge

ti-chi-bot · 2023-03-03T03:32:48Z

@breezewish: 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 · 2023-03-03T03:32:50Z

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

Commit hash: 5a7ad7b

Signed-off-by: Wish <breezewish@outlook.com>

…tiflash into wenxuan/p1/3_dump_checkpoint

breezewish · 2023-03-03T04:01:58Z

/merge

ti-chi-bot · 2023-03-03T04:01:59Z

@breezewish: 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 · 2023-03-03T04:02:01Z

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

Commit hash: 3f4cf41

Signed-off-by: Wish <breezewish@outlook.com>

breezewish · 2023-03-03T05:46:44Z

/merge

ti-chi-bot · 2023-03-03T05:46:45Z

@breezewish: 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 · 2023-03-03T05:46:47Z

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

Commit hash: 7645f39

flowbehappy · 2023-03-03T06:45:34Z

/merge

ti-chi-bot · 2023-03-03T06:45:35Z

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

JaySon-Huang · 2023-03-03T07:26:00Z

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

@@ -473,7 +473,7 @@ std::optional<PageEntryV3> VersionedPageEntries<Trait>::getLastEntry(std::option
    auto page_lock = acquireLock();
    if (type == EditRecordType::VAR_ENTRY)
    {
-        for (auto it_r = entries.rbegin(); it_r != entries.rend(); it_r++)
+        for (const auto & entrie : std::ranges::reverse_view(entries))


Suggested change

for (const auto & entrie : std::ranges::reverse_view(entries))

for (auto it_r = entries.rbegin(); it_r != entries.rend(); it_r++)

keep it not change because we don't support clang 15 in CI by now

This reverts commit 7645f39.

Signed-off-by: Wish <breezewish@outlook.com>

breezewish · 2023-03-03T07:37:11Z

/merge

ti-chi-bot · 2023-03-03T07:37:13Z

@breezewish: 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 · 2023-03-03T07:37:14Z

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

Commit hash: ed0cbfb

ti-chi-bot · 2023-03-03T08:59:20Z

@breezewish: 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

trigger some heavy tests which will not run always when PR updated.

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.

lidezhu · 2023-03-03T09:39:43Z

/run-unit-test

lidezhu · 2023-03-03T09:41:25Z

/run-integration-test

JaySon-Huang · 2023-03-03T10:00:09Z

/run-integration-test

Add dumpIncrementalCheckpoint

efc4cb7

Signed-off-by: Wish <breezewish@outlook.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 Mar 2, 2023

JaySon-Huang reviewed Mar 2, 2023

View reviewed changes

breezewish added 2 commits March 2, 2023 17:41

Add const qualifier

1ed97ce

Signed-off-by: Wish <breezewish@outlook.com>

Add tests and fix GC related problems

235bc4b

Signed-off-by: Wish <breezewish@outlook.com>

ti-chi-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 2, 2023

Move dumpCheckpoint to universal ps

52ce735

Signed-off-by: Wish <breezewish@outlook.com>

lidezhu approved these changes Mar 3, 2023

View reviewed changes

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

Remove unused

3eb30f1

Signed-off-by: Wish <breezewish@outlook.com>

JaySon-Huang approved these changes Mar 3, 2023

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 3, 2023

Address comments

5a7ad7b

Signed-off-by: Wish <breezewish@outlook.com>

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

ti-chi-bot and others added 3 commits March 3, 2023 11:32

Merge branch 'master' into wenxuan/p1/3_dump_checkpoint

1d56278

Fix for clang-15

71bc807

Signed-off-by: Wish <breezewish@outlook.com>

Merge branch 'wenxuan/p1/3_dump_checkpoint' of github.com:breezewish/…

3f4cf41

…tiflash into wenxuan/p1/3_dump_checkpoint

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

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

Fix static analysis

7645f39

Signed-off-by: Wish <breezewish@outlook.com>

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

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

JaySon-Huang reviewed Mar 3, 2023

View reviewed changes

breezewish added 2 commits March 3, 2023 15:33

Revert "Fix static analysis"

39649ae

This reverts commit 7645f39.

Skip lint

ed0cbfb

Signed-off-by: Wish <breezewish@outlook.com>

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

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

ti-chi-bot added 2 commits March 3, 2023 15:59

Merge branch 'master' into wenxuan/p1/3_dump_checkpoint

da17116

Merge branch 'master' into wenxuan/p1/3_dump_checkpoint

f468f33

ti-chi-bot merged commit 7949001 into pingcap:master Mar 3, 2023

breezewish deleted the wenxuan/p1/3_dump_checkpoint branch March 3, 2023 14:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dumpIncrementalCheckpoint utility #6928

Add dumpIncrementalCheckpoint utility #6928

breezewish commented Mar 2, 2023 •

edited

Loading

ti-chi-bot commented Mar 2, 2023 •

edited

Loading

JaySon-Huang Mar 2, 2023 •

edited

Loading

breezewish Mar 3, 2023

breezewish commented Mar 2, 2023

lidezhu left a comment

JaySon-Huang left a comment

JaySon-Huang Mar 3, 2023

JaySon-Huang Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

flowbehappy commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

JaySon-Huang Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

lidezhu commented Mar 3, 2023

lidezhu commented Mar 3, 2023

JaySon-Huang commented Mar 3, 2023

	RUNTIME_CHECK(iter->first.sequence == edit.version.sequence); // TODO: If there is a full GC this may be false?
	RUNTIME_CHECK(iter->first.sequence == edit.version.sequence, iter->first.sequence, edit.version.sequence); // TODO: If there is a full GC this may be false?

		class CPWriteDataSource;
		using CPWriteDataSourcePtr = std::shared_ptr<CPWriteDataSource>;

	for (const auto & entrie : std::ranges::reverse_view(entries))
	for (auto it_r = entries.rbegin(); it_r != entries.rend(); it_r++)

Add dumpIncrementalCheckpoint utility #6928

Add dumpIncrementalCheckpoint utility #6928

Conversation

breezewish commented Mar 2, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Mar 2, 2023 • edited Loading

JaySon-Huang Mar 2, 2023 • edited Loading

Choose a reason for hiding this comment

breezewish Mar 3, 2023

Choose a reason for hiding this comment

breezewish commented Mar 2, 2023

lidezhu left a comment

Choose a reason for hiding this comment

JaySon-Huang left a comment

Choose a reason for hiding this comment

JaySon-Huang Mar 3, 2023

Choose a reason for hiding this comment

JaySon-Huang Mar 3, 2023

Choose a reason for hiding this comment

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

flowbehappy commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

JaySon-Huang Mar 3, 2023

Choose a reason for hiding this comment

breezewish commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

ti-chi-bot commented Mar 3, 2023

lidezhu commented Mar 3, 2023

lidezhu commented Mar 3, 2023

JaySon-Huang commented Mar 3, 2023

breezewish commented Mar 2, 2023 •

edited

Loading

ti-chi-bot commented Mar 2, 2023 •

edited

Loading

JaySon-Huang Mar 2, 2023 •

edited

Loading