keep delete entry when dump snapshot by lidezhu · Pull Request #5357 · pingcap/tiflash

lidezhu · 2022-07-12T07:03:37Z

What problem does this PR solve?

Issue Number: close #5230

Problem Summary: when enable ps v3, after doing large transaction update and restart tiflash multiple times, tiflash cannot restart correctly.

Update:
For example:

put 5, version=<100, 0>
full GC start
del 5, version=<120, 0> => written to log_2_0
full GC end, move page 5 to a new location
upsert 5, version=<100, 1> => written to log_3_0

What is changed and how it works?

Previously, when dump snapshot for PageDirectory, we will remove obsolete entries if it is marked deleted. But the entry may have an upsert entry in later wal file which is not included in the snapshot. So if we remove the delete mark for the entry, the later upsert entry cannot be deleted after restart.
So we keep the last valid entry and the delete entry when dump snapshot.

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-07-12T07:03:38Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JaySon-Huang
flowbehappy

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.

lidezhu · 2022-07-12T07:08:44Z

/rebuild

lidezhu · 2022-07-12T07:21:37Z

/rebuild

lidezhu · 2022-07-12T07:23:33Z

/rebuild

lidezhu · 2022-07-12T07:38:03Z

/run-all-tests

lidezhu · 2022-07-12T07:43:49Z

/run-unit-test

lidezhu · 2022-07-12T08:00:22Z

/run-unit-test

lidezhu · 2022-07-12T08:16:38Z

/run-unit-test

sre-bot · 2022-07-12T08:35:44Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                        584               155    73.46%          56                 2    96.43%        1207               261    78.38%         368               134    63.59%
LogFile/LogFilename.h                  9                 0   100.00%           3                 0   100.00%          18                 3    83.33%           4                 0   100.00%
PageDirectory.cpp                    590                74    87.46%          32                 0   100.00%        1167               145    87.57%         448                90    79.91%
PageDirectory.h                       25                 3    88.00%          25                 3    88.00%         104                10    90.38%           0                 0         -
PageDirectoryFactory.cpp              52                 2    96.15%           6                 0   100.00%         127                 3    97.64%          52                 3    94.23%
PageDirectoryFactory.h                 2                 0   100.00%           2                 0   100.00%           8                 0   100.00%           0                 0         -
tests/gtest_page_storage.cpp        4094              1405    65.68%          51                 7    86.27%        1280               154    87.97%        1296               722    44.29%
tests/gtest_wal_log.cpp             2523               370    85.33%          49                 0   100.00%         529                 1    99.81%         772               351    54.53%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               7879              2009    74.50%         224                12    94.64%        4440               577    87.00%        2940              1300    55.78%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18513      9569             48.31%    208837  96323        53.88%

full coverage report (for internal network access only)

lidezhu · 2022-07-12T09:22:22Z

/rebuild

lidezhu · 2022-07-13T01:35:46Z

/rebuild

JaySon-Huang

Basically lgtm

Co-authored-by: JaySon <tshent@qq.com>

JaySon-Huang

LGTM

JaySon-Huang · 2022-07-14T07:09:39Z

We also need a cherry-pick on the release-6.1 branch? In case we may suggest some users to turn V3 on in v6.1.1 or later

… fix-blobstat-not-found

lidezhu · 2022-07-14T09:08:40Z

/merge

ti-chi-bot · 2022-07-14T09:08:40Z

@lidezhu: 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-07-14T09:08:42Z

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

Details

Commit hash: b4ab254

sre-bot · 2022-07-14T09:28:44Z

Coverage for changed files

Filename                         Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
BlobStore.cpp                        584               155    73.46%          56                 2    96.43%        1207               261    78.38%         368               134    63.59%
LogFile/LogFilename.h                  9                 0   100.00%           3                 0   100.00%          18                 3    83.33%           4                 0   100.00%
PageDirectory.cpp                    590                74    87.46%          32                 0   100.00%        1167               145    87.57%         448                89    80.13%
PageDirectory.h                       25                 3    88.00%          25                 3    88.00%         104                10    90.38%           0                 0         -
PageDirectoryFactory.cpp              52                 2    96.15%           6                 0   100.00%         127                 3    97.64%          52                 3    94.23%
PageDirectoryFactory.h                 2                 0   100.00%           2                 0   100.00%           8                 0   100.00%           0                 0         -
tests/gtest_page_storage.cpp        4094              1405    65.68%          51                 7    86.27%        1280               154    87.97%        1296               722    44.29%
tests/gtest_wal_log.cpp             2523               370    85.33%          49                 0   100.00%         529                 1    99.81%         772               351    54.53%
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                               7879              2009    74.50%         224                12    94.64%        4440               577    87.00%        2940              1299    55.82%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18532      9573             48.34%    208921  96388        53.86%

full coverage report (for internal network access only)

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2022-07-14T09:35:44Z

In response to a cherrypick label: new pull request created: #5370.

close #5230

lidezhu added 2 commits July 12, 2022 15:01

keep delete entry when dump snapshot

fa91722

add gtest for dump snapshot

0467bad

ti-chi-bot added do-not-merge/needs-linked-issue 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. and removed do-not-merge/needs-linked-issue labels Jul 12, 2022

Merge branch 'master' into fix-blobstat-not-found

d8e75a6

format

41d807e

Merge branch 'master' into fix-blobstat-not-found

315e20a

lidezhu requested review from JaySon-Huang, flowbehappy, hehechen and jiaqizho July 12, 2022 08:43

flowbehappy approved these changes Jul 13, 2022

View reviewed changes

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

jiaqizho reviewed Jul 13, 2022

View reviewed changes

Comment thread dbms/src/Storages/Page/V3/BlobStore.cpp

Comment thread dbms/src/Storages/Page/V3/BlobStore.cpp

Comment thread dbms/src/Storages/Page/V3/tests/gtest_page_storage.cpp

small fix for variable name

4878969

JaySon-Huang reviewed Jul 14, 2022

View reviewed changes

Comment thread dbms/src/Storages/Page/V3/BlobStore.cpp

Comment thread dbms/src/Storages/Page/V3/PageDirectoryFactory.cpp Outdated

Comment thread dbms/src/Storages/Page/V3/PageDirectory.h Outdated

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

6cd02b8

Co-authored-by: JaySon <tshent@qq.com>

lidezhu and others added 2 commits July 14, 2022 14:46

Update dbms/src/Storages/Page/V3/BlobStore.cpp

7a42bf1

Co-authored-by: JaySon <tshent@qq.com>

small fix for comment

b6732db

JaySon-Huang approved these changes Jul 14, 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 Jul 14, 2022

lidezhu added the needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. label Jul 14, 2022

lidezhu and others added 3 commits July 14, 2022 15:14

Merge branch 'master' into fix-blobstat-not-found

1bccb56

format

061ed6d

Merge branch 'fix-blobstat-not-found' of github.com:lidezhu/tics into…

b4ab254

… fix-blobstat-not-found

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

ti-chi-bot merged commit c40c262 into pingcap:master Jul 14, 2022

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jul 14, 2022

This is an automated cherry-pick of pingcap#5357

265dab5

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Jul 14, 2022

keep delete entry when dump snapshot (#5357) #5370

Merged

12 tasks

lidezhu deleted the fix-blobstat-not-found branch July 14, 2022 09:39

ti-chi-bot added a commit that referenced this pull request Aug 17, 2022

keep delete entry when dump snapshot (#5357) (#5370)

6feacc2

close #5230

This was referenced Oct 19, 2022

Compaction on PageStorage WAL causes OOM #6163

Closed

PageStorage: Fix peak memory usage when running GC on PageDirectory #6168

Merged

ti-chi-bot mentioned this pull request Oct 26, 2022

PageStorage: Fix peak memory usage when running GC on PageDirectory (#6168) #6184

Closed

12 tasks

Conversation

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

Uh oh!

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

sre-bot commented Jul 12, 2022

Coverage for changed files

Coverage summary

Uh oh!

lidezhu commented Jul 12, 2022

Uh oh!

lidezhu commented Jul 13, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JaySon-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JaySon-Huang left a comment

Choose a reason for hiding this comment

Uh oh!

JaySon-Huang commented Jul 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lidezhu commented Jul 14, 2022

Uh oh!

ti-chi-bot commented Jul 14, 2022

Uh oh!

ti-chi-bot commented Jul 14, 2022

Uh oh!

sre-bot commented Jul 14, 2022

Coverage for changed files

Coverage summary

Uh oh!

ti-chi-bot commented Jul 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lidezhu commented Jul 12, 2022 •

edited by JaySon-Huang

Loading

ti-chi-bot commented Jul 12, 2022 •

edited

Loading

JaySon-Huang commented Jul 14, 2022 •

edited

Loading