Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: region manifest checkpoint #1202

Merged

Conversation

killme2008
Copy link
Contributor

@killme2008 killme2008 commented Mar 20, 2023

I hereby agree to the terms of the GreptimeDB CLA

What's changed and what's your intention?

Impl region manifest checkpoint #170 . It compacts manifest actions and creates a snapshot for them. It will reduce disk consumption and speed up region recovery.

Main changes:

  • Region manifest snapshot structures such as RegionManifestData and RegionSnapshot etc. Please refer to src/storage/src/manifest/action.rs.
  • Adds Checkpointer trait and adds it into ManifestImpl.
  • Adds RegionManifestCheckpointer to do checkpointing for region manifest. When saving manifest actions every ten times, it will try to do a checkpoint.
  • Recovers region manifest from the checkpointed snapshot.

Checklist

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.

Refer to a related PR or issue link (optional)

#170

@killme2008 killme2008 changed the title Feature/region manifest checkpoint feat: region manifest checkpoint Mar 20, 2023
@killme2008 killme2008 force-pushed the feature/region-manifest-checkpoint branch from 74227b5 to 76eb90c Compare March 20, 2023 13:32
@killme2008 killme2008 force-pushed the feature/region-manifest-checkpoint branch from 76eb90c to f0d5163 Compare March 21, 2023 10:08
@killme2008 killme2008 marked this pull request as ready for review March 21, 2023 13:16
@killme2008 killme2008 requested review from evenyag, v0y4g3r and waynexia and removed request for evenyag and v0y4g3r March 21, 2023 13:24
@killme2008 killme2008 mentioned this pull request Mar 21, 2023
2 tasks
@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Merging #1202 (bcc7931) into develop (f351ee7) will decrease coverage by 0.39%.
The diff coverage is 92.89%.

❗ Current head bcc7931 differs from pull request most recent head ad418aa. Consider uploading reports for the commit ad418aa to get more accurate results

@@             Coverage Diff             @@
##           develop    #1202      +/-   ##
===========================================
- Coverage    85.34%   84.95%   -0.39%     
===========================================
  Files          494      495       +1     
  Lines        72513    72949     +436     
===========================================
+ Hits         61888    61977      +89     
- Misses       10625    10972     +347     

@killme2008 killme2008 requested review from evenyag and v0y4g3r March 21, 2023 14:54
src/storage/src/version.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/checkpoint.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Show resolved Hide resolved
src/storage/src/manifest/region.rs Outdated Show resolved Hide resolved
killme2008 and others added 3 commits March 22, 2023 15:24
Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
killme2008 and others added 2 commits March 22, 2023 15:25
Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
@killme2008
Copy link
Contributor Author

@waynexia PTAL

Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

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

LGTM

src/storage/src/manifest/checkpoint.rs Show resolved Hide resolved
src/storage/src/manifest/impl_.rs Show resolved Hide resolved
src/storage/src/manifest/impl_.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Show resolved Hide resolved
src/storage/src/manifest/region.rs Outdated Show resolved Hide resolved
src/storage/src/manifest/region.rs Show resolved Hide resolved
src/storage/src/region/writer.rs Outdated Show resolved Hide resolved
@killme2008
Copy link
Contributor Author

@evenyag PTAL, thank you.

Copy link
Contributor

@evenyag evenyag left a comment

Choose a reason for hiding this comment

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

LGTM

@evenyag evenyag merged commit 4f15b26 into GreptimeTeam:develop Mar 27, 2023
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
* chore: adds log when manifest protocol is changed

* chore: refactor region manifest

* temp commit

* feat: impl region manifest checkpoint

* feat: recover region version from manifest snapshot

* test: adds region snapshot test

* test: region manifest checkpoint

* test: alter region with manifest checkpoint

* fix: revert storage api

* feat: delete old snapshot

* refactor: manifest log storage

* Update src/storage/src/version.rs

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>

* Update src/storage/src/manifest/checkpoint.rs

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>

* Update src/storage/src/manifest/region.rs

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>

* Update src/storage/src/manifest/region.rs

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>

* chore: by CR comments

* refactor: by CR comments

* fix: typo

* chore: tweak start_version

---------

Co-authored-by: Ruihang Xia <waynestxia@gmail.com>
@killme2008 killme2008 deleted the feature/region-manifest-checkpoint branch January 16, 2024 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants