Skip to content

clear obsolete v2 data for tables that totally transform to ps v3#5373

Merged
ti-chi-bot merged 2 commits intopingcap:masterfrom
lidezhu:clear-obsolete-v2-data
Jul 18, 2022
Merged

clear obsolete v2 data for tables that totally transform to ps v3#5373
ti-chi-bot merged 2 commits intopingcap:masterfrom
lidezhu:clear-obsolete-v2-data

Conversation

@lidezhu
Copy link
Contributor

@lidezhu lidezhu commented Jul 14, 2022 •
edited
Loading

What problem does this PR solve?

Issue Number: close #5336

Problem Summary: After transform from all data from ps v2 to ps v3, there are still some v2 data left which may consume unnecessary disk space.

What is changed and how it works?

When the valid pages in ps v2 is 0, we write a mark file to indicate that ps v2 is deleted. And try to drop v2 data.
And at later restart, if we find that there is are ps v2 delete mark file, we will not restore ps v2 and may try to clear obsolete v2 data if previous drop process was interrupted.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  1. load data in ps v2 format;
  2. edit config to use ps v3 format;
  3. run sql alter table ... compact tiflash replica; to transform all data from ps v2 to ps v3;
  4. restart again to check whether the ps v2 data in the table data path is deleted.

Before restart:
image
After restart and run compact:
image

  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jul 14, 2022 •
edited
Loading

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

@ti-chi-bot ti-chi-bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed do-not-merge/needs-linked-issue labels Jul 14, 2022
@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 15, 2022

/rebuild

@lidezhu lidezhu force-pushed the clear-obsolete-v2-data branch 2 times, most recently from f204e42 to 5b74225 Compare July 15, 2022 02:12
JaySon-Huang
JaySon-Huang reviewed Jul 15, 2022
View reviewed changes
@flowbehappy
Copy link
Contributor

flowbehappy commented Jul 15, 2022

It looks like we use two strategies for tables' PS and kvstore's PS, can you clarify each of them? @lidezhu

flowbehappy
flowbehappy reviewed Jul 15, 2022
View reviewed changes
@lidezhu lidezhu force-pushed the clear-obsolete-v2-data branch 2 times, most recently from 92b5d91 to 9062fb5 Compare July 16, 2022 07:05
@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 18, 2022 •
edited
Loading

It looks like we use two strategies for tables' PS and kvstore's PS, can you clarify each of them? @lidezhu

kvstore's ps v2 will just run once at restart, but table's ps v2 will always run if there is some data left in log storage at restart. So we can safely use some aggressive gc config for kvstore, but we must be carful with tables's ps.

Now ps v2 will rotate writing page file if no valid pages in non writeing page files.

@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 18, 2022

/run-all-tests

@sre-bot
Copy link
Collaborator

sre-bot commented Jul 18, 2022

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DeltaMerge/StoragePool.cpp              237               108    54.43%          30                 8    73.33%         506               198    60.87%         156                85    45.51%
DeltaMerge/StoragePool.h                 11                 1    90.91%          11                 1    90.91%          31                 9    70.97%           0                 0         -
Page/PageStorage.cpp                    149                37    75.17%          50                10    80.00%         410               111    72.93%         108                28    74.07%
Page/PageStorage.h                       21                 4    80.95%          21                 4    80.95%          93                12    87.10%           0                 0         -
Page/V2/PageStorage.cpp                 578               189    67.30%          39                 4    89.74%         998               214    78.56%         394               181    54.06%
Page/V2/PageStorage.h                    49                13    73.47%          20                 3    85.00%          48                 4    91.67%          22                13    40.91%
PathPool.cpp                            409               150    63.33%          82                22    73.17%         805               234    70.93%         228               105    53.95%
PathPool.h                               14                 2    85.71%          14                 2    85.71%          16                 2    87.50%           0                 0         -
Transaction/RegionPersister.cpp         160                80    50.00%          16                 3    81.25%         301               122    59.47%          84                46    45.24%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1628               584    64.13%         283                57    79.86%        3208               906    71.76%         992               458    53.83%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18600      9577             48.51%    211941  96495        54.47%

full coverage report (for internal network access only)

@flowbehappy
Copy link
Contributor

flowbehappy commented Jul 18, 2022

It looks like we use two strategies for tables' PS and kvstore's PS, can you clarify each of them? @lidezhu

kvstore's ps v2 will just run once at restart, but table's ps v2 will always run if there is some data left in log storage at restart. So we can safely use some aggressive gc config for kvstore, but we must be carful with tables's ps.

Now ps v2 will rotate writing page file if no valid pages in non writeing page files.

Great! And I think it will be better to put this information in PR comment. @lidezhu

@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 18, 2022

It looks like we use two strategies for tables' PS and kvstore's PS, can you clarify each of them? @lidezhu

kvstore's ps v2 will just run once at restart, but table's ps v2 will always run if there is some data left in log storage at restart. So we can safely use some aggressive gc config for kvstore, but we must be carful with tables's ps.
Now ps v2 will rotate writing page file if no valid pages in non writeing page files.

Great! And I think it will be better to put this information in PR comment. @lidezhu

Done.

JaySon-Huang
JaySon-Huang reviewed Jul 18, 2022
View reviewed changes
@lidezhu lidezhu force-pushed the clear-obsolete-v2-data branch from a700c8e to 1b2f221 Compare July 18, 2022 07:01
JaySon-Huang
JaySon-Huang approved these changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@JaySon-Huang JaySon-Huang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 18, 2022
@ti-chi-bot 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 18, 2022
@lidezhu @JaySon-Huang
Update dbms/src/Storages/Page/V2/PageStorage.cpp
0443b9d 
Co-authored-by: JaySon <tshent@qq.com>
@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 18, 2022

/merge

@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jul 18, 2022

@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
Copy link
Member

ti-chi-bot commented Jul 18, 2022

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

DetailsCommit hash: 0443b9d

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 18, 2022
@sre-bot
Copy link
Collaborator

sre-bot commented Jul 18, 2022

Coverage for changed files

Filename                            Regions    Missed Regions     Cover   Functions  Missed Functions  Executed       Lines      Missed Lines     Cover    Branches   Missed Branches     Cover
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
DeltaMerge/StoragePool.cpp              237               108    54.43%          30                 8    73.33%         506               198    60.87%         156                85    45.51%
DeltaMerge/StoragePool.h                 11                 1    90.91%          11                 1    90.91%          31                 9    70.97%           0                 0         -
Page/PageStorage.cpp                    149                37    75.17%          50                10    80.00%         410               111    72.93%         108                28    74.07%
Page/PageStorage.h                       21                 4    80.95%          21                 4    80.95%          93                12    87.10%           0                 0         -
Page/V2/PageStorage.cpp                 586               198    66.21%          39                 4    89.74%        1026               229    77.68%         398               185    53.52%
Page/V2/PageStorage.h                    49                13    73.47%          20                 3    85.00%          48                 4    91.67%          22                13    40.91%
PathPool.cpp                            409               141    65.53%          82                21    74.39%         805               217    73.04%         228               100    56.14%
PathPool.h                               14                 2    85.71%          14                 2    85.71%          16                 2    87.50%           0                 0         -
Transaction/RegionPersister.cpp         160                80    50.00%          16                 3    81.25%         301               122    59.47%          84                46    45.24%
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
TOTAL                                  1636               584    64.30%         283                56    80.21%        3236               904    72.06%         996               457    54.12%

Coverage summary

Functions  MissedFunctions  Executed  Lines   MissedLines  Cover
18600      9574             48.53%    211969  96435        54.51%

full coverage report (for internal network access only)

@lidezhu
Copy link
Contributor Author

lidezhu commented Jul 18, 2022

/run-integration-test

@ti-chi-bot ti-chi-bot merged commit 79c3936 into pingcap:master Jul 18, 2022
@lidezhu lidezhu deleted the clear-obsolete-v2-data branch July 18, 2022 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@flowbehappy flowbehappy flowbehappy approved these changes

@JaySon-Huang JaySon-Huang JaySon-Huang approved these changes

Assignees

No one assigned

Labels

release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove the data of PageStorage V2 after all data has been migrated to V3

5 participants

@lidezhu @ti-chi-bot @flowbehappy @sre-bot @JaySon-Huang