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

refactor(meta): persist hummock version checkpoint in object store #8631

Merged
merged 11 commits into from
Mar 29, 2023

Conversation

zwang28
Copy link
Contributor

@zwang28 zwang28 commented Mar 18, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

  1. Instead of storing hummock version checkpoint in meta store, we store it in object store, so that we can support much larger hummock version.
  2. Instead of tracking stale SSTs by hummock version delta logs, we duplicate those info in version checkpoint, so that delta logs GC won’t be blocked by unavailable GC worker or long-running pinned version.

more detail here.

Also adapt meta backup to this change, because it has to read/write to both meta store and object store now.

Checklist For Contributors

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
  • All checks passed in ./risedev check (or alias, ./risedev c)

Checklist For Reviewers

  • I have requested macro/micro-benchmarks as this PR can affect performance substantially, and the results are shown.

Documentation

  • My PR DOES NOT contain user-facing changes.
Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • Installation and deployment
  • Connector (sources & sinks)
  • SQL commands, functions, and operators
  • RisingWave cluster configuration changes
  • Other (please specify in the release note below)

Release note

@zwang28 zwang28 force-pushed the wangzheng/ckpt branch 2 times, most recently from fe019ab to 6316464 Compare March 20, 2023 06:52
@zwang28 zwang28 marked this pull request as ready for review March 20, 2023 07:05
@zwang28 zwang28 requested review from wenym1 and hzxa21 March 20, 2023 07:05
@zwang28 zwang28 force-pushed the wangzheng/ckpt branch 2 times, most recently from 1027737 to 02532e2 Compare March 20, 2023 11:42
@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Merging #8631 (e5e2498) into main (eeb2c39) will decrease coverage by 0.08%.
The diff coverage is 74.47%.

@@            Coverage Diff             @@
##             main    #8631      +/-   ##
==========================================
- Coverage   70.81%   70.73%   -0.08%     
==========================================
  Files        1171     1172       +1     
  Lines      194337   194549     +212     
==========================================
+ Hits       137619   137621       +2     
- Misses      56718    56928     +210     
Flag Coverage Δ
rust 70.73% <74.47%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/meta/src/hummock/error.rs 16.66% <0.00%> (-2.39%) ⬇️
src/meta/src/hummock/mod.rs 13.26% <0.00%> (-8.96%) ⬇️
src/meta/src/lib.rs 0.90% <0.00%> (-0.06%) ⬇️
src/meta/src/rpc/server.rs 0.00% <0.00%> (ø)
src/object_store/src/object/s3.rs 3.83% <0.00%> (+0.02%) ⬆️
src/storage/backup/src/error.rs 0.00% <ø> (ø)
src/meta/src/backup_restore/backup_manager.rs 14.28% <19.04%> (+0.11%) ⬆️
src/storage/src/hummock/sstable_store.rs 64.99% <54.54%> (-0.33%) ⬇️
...c/meta/src/backup_restore/meta_snapshot_builder.rs 87.87% <71.42%> (-4.64%) ⬇️
src/meta/src/hummock/manager/mod.rs 75.75% <81.81%> (-1.17%) ⬇️
... and 14 more

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@zwang28
Copy link
Contributor Author

zwang28 commented Mar 22, 2023

As --state-store is not yet mandatory during cluster initialization, this PR should also prevent user from using memory object store for checkpoint by mistake when --state-store is not set.
fix by #8704

@zwang28
Copy link
Contributor Author

zwang28 commented Mar 22, 2023

We can further make external reader (java binding) independent of meta service, if we

  • include table schemas in hummock version checkpoint as well.
  • enable S3 versioning to control the lifetime of deleted objects, so that external reader can read without pinning a hummock version, within the retention time configured in S3.

@wenym1 @hzxa21

@zwang28 zwang28 requested review from Little-Wallace and Li0k March 22, 2023 09:18
.into_iter()
.next()
.ok_or_else(|| anyhow!("hummock version checkpoint not found in meta store"))?;
let mut redo_state = hummock_version_checkpoint;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut redo_state = hummock_version_checkpoint;
let mut redo_state = hummock_version_checkpoint_builder.await?;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hummock_version_checkpoint_builder.await? is called before self.meta_store.snapshot().await by intention, to ensure resulted version is <= meta store snapshot, because subsequently we can always get a matching version for that meta store snapshot by replaying version delta.

src/meta/src/hummock/manager/checkpoint.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@hzxa21 hzxa21 left a comment

Choose a reason for hiding this comment

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

Rest LGTM. Great job!

src/meta/src/backup_restore/restore.rs Outdated Show resolved Hide resolved
src/meta/src/hummock/manager/checkpoint.rs Show resolved Hide resolved
src/meta/src/hummock/manager/gc.rs Show resolved Hide resolved
src/meta/src/hummock/manager/versioning.rs Outdated Show resolved Hide resolved
src/storage/backup/src/lib.rs Outdated Show resolved Hide resolved
zwang28 added 6 commits March 28, 2023 20:34
# Conflicts:
#	dashboard/proto/gen/hummock.ts
#	src/meta/src/hummock/manager/mod.rs
#	src/meta/src/hummock/manager/versioning.rs
#	src/storage/hummock_sdk/src/lib.rs
#	src/storage/src/hummock/sstable_store.rs
@zwang28 zwang28 enabled auto-merge March 29, 2023 05:46
@zwang28 zwang28 added this pull request to the merge queue Mar 29, 2023
Merged via the queue into main with commit bc99528 Mar 29, 2023
@zwang28 zwang28 deleted the wangzheng/ckpt branch March 29, 2023 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants