Skip to content

PageStorage skip non continuous sequence safely#2435

Merged
ti-srebot merged 9 commits intopingcap:masterfrom
JaySon-Huang:safe_skip_non_continuous_sequence
Jul 28, 2021
Merged

PageStorage skip non continuous sequence safely#2435
ti-srebot merged 9 commits intopingcap:masterfrom
JaySon-Huang:safe_skip_non_continuous_sequence

Conversation

@JaySon-Huang
Copy link
Contributor

@JaySon-Huang JaySon-Huang commented Jul 19, 2021 •
edited
Loading

What problem does this PR solve?

Issue Number: close #2317, related to #2312, #2356

Problem Summary:
After #2312 and #2356, we introduce "Formal" PageFiles into LegacyCompactor::collectPageFilesToCompact.
So we may read files that are writing by other threads during LegacyCompactor::collectPageFilesToCompact. We need further protection on those writable files while applying GC, or we may scan some data in the middle of writing.

What is changed and how it works?

  • Save the meta offset of writable files after restore and write
  • Save the last persisted sequence of writable files after write
  • Set the last persisted sequence of writable of the overall persisted sequence after restore
  • When reading meta from those writable files, only read to the meta offset or to the min persisted sequence at maximum
  • Add some failpoint to generate non-continuous sequences

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch:

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • Enable random_slow_page_storage_write and random_exception_after_page_storage_sequence_acquired to generate non-continuous sequence
    • Wait until the meta get compacted by LegacyCompactor
    • Restart TiFlash to check whether we can read / write

Side effects

Release note

  • No release note

@JaySon-Huang JaySon-Huang mentioned this pull request Jul 19, 2021
Merged
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jul 19, 2021

/run-all-tests

@JaySon-Huang JaySon-Huang changed the title Skip non continuous sequence safely PageStorage skip non continuous sequence safely Jul 19, 2021
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jul 20, 2021

/run-all-tests

@JaySon-Huang JaySon-Huang mentioned this pull request Jul 21, 2021
Merged
@JaySon-Huang
calculating num of legacy independently
779a391 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
reading writable files
bc44a29 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Add failpoint for testing skip non-continuous sequence
ae9ba69 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Stop Legacy compactor by the min persisted sequence
d11ba98 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Fix other bugs
37bb927 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang
Fix the min persisted sequence
707fac6 
Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang JaySon-Huang force-pushed the safe_skip_non_continuous_sequence branch from 9047f83 to 707fac6 Compare July 23, 2021 11:36
@JaySon-Huang JaySon-Huang added the type/bugfix This PR fixes a bug. label Jul 26, 2021
@JaySon-Huang JaySon-Huang requested a review from flowbehappy July 26, 2021 02:59
flowbehappy
flowbehappy approved these changes Jul 27, 2021
View reviewed changes
Copy link
Contributor

@flowbehappy flowbehappy 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-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 27, 2021
@flowbehappy
Copy link
Contributor

flowbehappy commented Jul 28, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 28, 2021
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jul 28, 2021

Your auto merge job has been accepted, waiting for:

  • 2509
  • 2504

@ti-srebot
Copy link
Collaborator

ti-srebot commented Jul 28, 2021

/run-all-tests

@JaySon-Huang JaySon-Huang removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 28, 2021
@JaySon-Huang
Copy link
Contributor Author

JaySon-Huang commented Jul 28, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 28, 2021
@ti-srebot
Copy link
Collaborator

ti-srebot commented Jul 28, 2021

/run-all-tests

@ti-srebot ti-srebot merged commit 3fe3768 into pingcap:master Jul 28, 2021
@JaySon-Huang JaySon-Huang deleted the safe_skip_non_continuous_sequence branch July 28, 2021 11:55
@flowbehappy flowbehappy added this to the v5.1.2 milestone Aug 4, 2021
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Aug 4, 2021
@JaySon-Huang
PageStorage skip non continuous sequence safely (pingcap#2435)
aca0d26
@JaySon-Huang JaySon-Huang mentioned this pull request Aug 4, 2021
Merged
JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Aug 4, 2021
@JaySon-Huang
PageStorage skip non continuous sequence safely (pingcap#2435)
123e382
@JaySon-Huang JaySon-Huang mentioned this pull request Aug 4, 2021
Merged
flowbehappy pushed a commit that referenced this pull request Aug 4, 2021
@JaySon-Huang
5.1.1 hotfix patch for PageStorage layer (#2562)
1d6103a 
* Ignore sequence hole among PageFile meta (#2312)

* Fix bug for GC may skip unexpected WriteBatches (#2356)

* Add length check while running PageStorage GC (#2394)

* PageStorage skip non continuous sequence safely (#2435)

* Fix PageStorage GC with high valid rate PageFile (#2436)

* More debug info for DeltaTree (query_id, snapshot lifetime) (#2431)

* Fix deadlock on `removeExpiredSnapshots` (#2461)

* Add grafana panels for write throughput per instance (#2524)
This was referenced Aug 4, 2021
Merged
Merged
JaySon-Huang pushed a commit that referenced this pull request Aug 5, 2021
@ti-srebot
Fix bugs for PageStorage GC (release-4.0) (#2335)
c4fb00a 
* Ignore sequence hole among PageFile meta (#2312)
* Fix bug for GC may skip unexpected WriteBatches (#2356)
* Add length check while running PageStorage GC (#2394)
* PageStorage skip non continuous sequence safely (#2435)
* Fix PageStorage GC with high valid rate PageFile (#2436)

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
@JaySon-Huang JaySon-Huang removed this from the v5.1.2 milestone Aug 19, 2021
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

Assignees

@JaySon-Huang JaySon-Huang

Labels

status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PageStorage can not GC legacy files due to non-continuous sequence number

3 participants

@JaySon-Huang @flowbehappy @ti-srebot