PageStorage: Add V3 framework#3593
Conversation
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. DetailsReviewer can indicate their review by submitting an approval review. |
| using PageMap = std::map<PageId, Page>; | ||
| using PageHandler = std::function<void(PageId page_id, const Page &)>; | ||
|
|
||
| // TODO: Move it into V2 |
There was a problem hiding this comment.
PageEntry used by Page/PageStorage.h, if we moved it into v2. we may need use using to redirect this class.
There was a problem hiding this comment.
Yes, I may refactor out PageStorage::getEntry and make it an internal class inside PageStorageV2/V1 in another PR.
| record.type = WriteBatch::WriteType::PUT; | ||
| record.page_id = page_id; | ||
| record.entry = entry; | ||
| records.emplace_back(record); |
There was a problem hiding this comment.
not sure we need to use std::move(record) in here.
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
|
LGTM, just a little suggestion: change the |
| { | ||
| // Persist Page data to BlobStore | ||
| PageEntriesEdit edit(write_batch.getWrites().size()); | ||
| // { |
There was a problem hiding this comment.
rm annotation in this method pls. cause it should be moved to blobstore.
The todo-list for the following two changes has been added to #3594 and I will take care of them.
|
|
/run-all-tests |
|
LGTM |
|
/merge |
|
@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 If you have any questions about the PR merge process, please refer to pr process. DetailsInstructions 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. |
|
This pull request has been accepted and is ready to merge. DetailsCommit hash: 9dad156 |
|
Coverage detail: https://ci-internal.pingcap.net/job/tics_ghpr_unit_test/282/cobertura/ lines: 40.3% (44439 out of 110245) |
Signed-off-by: JaySon-Huang jayson.hjs@gmail.com
What problem does this PR solve?
Issue Number: related to #3594
Problem Summary:
Add some data structure / class interface for PageStorage V3 inside.
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note