PageStorage: Add mix mode by jiaqizho · Pull Request #4726 · pingcap/tiflash

jiaqizho · 2022-04-21T02:35:24Z

Signed-off-by: jiaqizho zhoujiaqi@pingcap.com

What problem does this PR solve?

Issue Number: ref #3594

Problem Summary:

When user already has some data which used PageStorage V2 format. Then it can't convert to V3

What is changed and how it works?

Changed PageWriter/PageReader to allow users used V3 format for write/read and V2 format for read-only.
When the user contains V2 format data. after set format_version = 4， Then we will be running PageStorage in Mix Mode. Which will both deal with V3 and V2 format data.
We also allow user running ONLY_V2/ONLY_V3(which means only single version of page data format):
- PageFormat::V2 + isPageStorageV3Existed is false + whatever isPageStorageV2Existed true or false = ONLY_V2
- PageFormat::V2 + isPageStorageV3Existed is true + whatever isPageStorageV2Existed true or false = ERROR Config
- PageFormat::V3 + isPageStorageV2Existed is true + whatever isPageStorageV3Existed true or false = MIX_MODE
- PageFormat::V3 + isPageStorageV2Existed is false + whatever isPageStorageV3Existed true or false = ONLY_V3
The data path for KVStore(RegionPersister) is changed:
- $DATA_DIR/kvstore for kvstore (PageStorage V2/V1)
- $DATA_DIR/page/kvstore for kvstore (PageStorage V3)
- The hidden configuration storage.raft.dir/raft.kvstore_path will be ignored

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-04-21T02:35:25Z

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

Details

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

jiaqizho · 2022-04-27T11:03:29Z

/run-all-tests

sre-bot · 2022-04-27T11:29:45Z

Coverage for changed files

too many lines from llvm-cov, please refer to full report instead

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17292      9374             45.79%    195466  95826        50.98%

full coverage report (for internal network access only)

jiaqizho · 2022-04-27T14:05:38Z

/run-all-tests

sre-bot · 2022-04-27T14:14:33Z

Coverage for changed files

too many lines from llvm-cov, please refer to full report instead

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
17292      9376             45.78%    195463  95866        50.95%

full coverage report (for internal network access only)

jiaqizho · 2022-04-27T14:32:45Z

pre PR : #4736

dbms/src/Storages/DeltaMerge/StoragePool.cpp

dbms/src/Storages/Page/PageStorage.h

dbms/src/Storages/DeltaMerge/StoragePool.cpp

dbms/src/Storages/Page/PageStorage.h

Signed-off-by: jiaqizho <zhoujiaqi@pingcap.com>

JaySon-Huang · 2022-05-11T08:37:46Z

PTAL at https://github.com/jiaqizho/tiflash/pull/11.
Use inherite to hind the switch .. case statements

hide lots of switch by inherite

JaySon-Huang

LGTM

dbms/src/Storages/Page/PageStorage.cpp

JaySon-Huang · 2022-05-11T09:26:39Z

/run-all-tests

sre-bot · 2022-05-11T10:10:00Z

Coverage for changed files

too many lines from llvm-cov, please refer to full report instead

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18183      9847             45.85%    202958  98591        51.42%

full coverage report (for internal network access only)

dbms/src/Storages/DeltaMerge/StoragePool.cpp

hehechen · 2022-05-11T15:59:36Z

dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store.cpp

    }

-    std::pair<RowKeyRange, std::vector<PageId>> genDMFile(DMContext & context, const Block & block)
+    std::pair<RowKeyRange, PageIds> genDMFile(DMContext & context, const Block & block)


I think we should add UT to test delta merge storage in mix mode

I prefer to add it in the follow-up PR. Cause writing ut with GlobalPathPool is not easy.

otherwise, already have test cover in V3/gtest_page_storage_mix_mode.cpp.

…mix-mode

lidezhu

LGTM

jiaqizho · 2022-05-12T01:49:06Z

/merge

ti-chi-bot · 2022-05-12T01:49:06Z

@jiaqizho: 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-05-12T01:49:08Z

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

Details

Commit hash: d2a6ae0

ti-chi-bot · 2022-05-12T01:49:21Z

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

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.

sre-bot · 2022-05-12T02:19:18Z

Coverage for changed files

too many lines from llvm-cov, please refer to full report instead

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18152      9817             45.92%    202639  98303        51.49%

full coverage report (for internal network access only)

pingcap deleted a comment from sre-bot Apr 25, 2022

pingcap deleted a comment from sre-bot Apr 26, 2022

pingcap deleted a comment from sre-bot Apr 27, 2022

jiaqizho changed the title ~~[WIP]PageStorage: Add mix mode~~ PageStorage: Add mix mode Apr 27, 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 Apr 27, 2022

jiaqizho requested review from JaySon-Huang, flowbehappy and lidezhu and removed request for lidezhu April 27, 2022 14:29

flowbehappy reviewed Apr 28, 2022

View reviewed changes

dbms/src/Storages/DeltaMerge/StoragePool.cpp Show resolved Hide resolved

flowbehappy reviewed Apr 28, 2022

View reviewed changes

dbms/src/Storages/Page/PageStorage.h Outdated Show resolved Hide resolved

flowbehappy reviewed Apr 28, 2022

View reviewed changes

dbms/src/Storages/DeltaMerge/StoragePool.cpp Show resolved Hide resolved

flowbehappy reviewed Apr 28, 2022

View reviewed changes

dbms/src/Storages/Page/PageStorage.h Outdated Show resolved Hide resolved

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

Add v3 mix mode

e658272

Signed-off-by: jiaqizho <zhoujiaqi@pingcap.com>

jiaqizho and others added 4 commits May 11, 2022 16:38

Merge pull request #11 from JaySon-Huang/v2-v3-mix-mode

c56e9f9

hide lots of switch by inherite

fix restore not right

2721ca4

update

5a73c86

fix format

ad25e86

JaySon-Huang approved these changes May 11, 2022

View reviewed changes

dbms/src/Storages/Page/PageStorage.cpp Outdated Show resolved Hide resolved

dbms/src/Storages/Page/PageStorage.cpp Outdated Show resolved Hide resolved

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

JaySon-Huang and others added 2 commits May 11, 2022 17:15

Apply suggestions from code review

b66007d

Merge branch 'master' into v2-v3-mix-mode

4e2a10f

jiaqizho requested a review from hehechen May 11, 2022 10:20

lidezhu reviewed May 11, 2022

View reviewed changes

dbms/src/Storages/DeltaMerge/StoragePool.cpp Outdated Show resolved Hide resolved

dbms/src/Storages/DeltaMerge/StoragePool.cpp Show resolved Hide resolved

hehechen reviewed May 11, 2022

View reviewed changes

jiaqizho added 2 commits May 12, 2022 09:04

update

93793bd

Merge branch 'v2-v3-mix-mode' of github.com:jiaqizho/tics into v2-v3-…

d2a6ae0

…mix-mode

jiaqizho requested review from hehechen and lidezhu May 12, 2022 01:06

lidezhu approved these changes May 12, 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 May 12, 2022

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

Merge branch 'master' into v2-v3-mix-mode

15f5c34

ti-chi-bot merged commit 506df23 into pingcap:master May 12, 2022

hehechen mentioned this pull request May 12, 2022

Add metrics for PageStorage V3 RunMode #4870

Closed

Conversation

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

Uh oh!

Uh oh!

jiaqizho commented Apr 27, 2022

Uh oh!

sre-bot commented Apr 27, 2022

Coverage for changed files

Coverage summary

Uh oh!

jiaqizho commented Apr 27, 2022

Uh oh!

sre-bot commented Apr 27, 2022

Coverage for changed files

Coverage summary

Uh oh!

jiaqizho commented Apr 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JaySon-Huang commented May 11, 2022

Uh oh!

JaySon-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JaySon-Huang commented May 11, 2022

Uh oh!

sre-bot commented May 11, 2022

Coverage for changed files

Coverage summary

Uh oh!

Uh oh!

Uh oh!

hehechen May 11, 2022

Choose a reason for hiding this comment

Uh oh!

jiaqizho May 12, 2022

Choose a reason for hiding this comment

Uh oh!

lidezhu left a comment

Choose a reason for hiding this comment

Uh oh!

jiaqizho commented May 12, 2022

Uh oh!

ti-chi-bot commented May 12, 2022

Uh oh!

ti-chi-bot commented May 12, 2022

Uh oh!

ti-chi-bot commented May 12, 2022

Uh oh!

sre-bot commented May 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jiaqizho commented Apr 21, 2022 •

edited by JaySon-Huang

Loading

ti-chi-bot commented Apr 21, 2022 •

edited

Loading