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(hummock manager): load mem hummock version with version delt… #3529

Merged
merged 5 commits into from
Jun 29, 2022

Conversation

soundOfDestiny
Copy link
Contributor

…a instead of version

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

PLEASE DO NOT LEAVE THIS EMPTY !!!

load mem hummock version with version delta instead of version
rebuild version from version deltas
this PR is successor of #3503

Checklist

Refer to a related PR or issue link (optional)

@codecov
Copy link

codecov bot commented Jun 28, 2022

Codecov Report

Merging #3529 (ad59372) into main (c4c7ab6) will decrease coverage by 0.00%.
The diff coverage is 65.46%.

@@            Coverage Diff             @@
##             main    #3529      +/-   ##
==========================================
- Coverage   74.45%   74.44%   -0.01%     
==========================================
  Files         770      770              
  Lines      108517   108600      +83     
==========================================
+ Hits        80792    80850      +58     
- Misses      27725    27750      +25     
Flag Coverage Δ
rust 74.44% <65.46%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
src/meta/src/hummock/hummock_manager.rs 84.00% <38.59%> (-2.48%) ⬇️
...ck_sdk/src/compaction_group/hummock_version_ext.rs 85.22% <81.42%> (-14.78%) ⬇️
src/meta/src/hummock/compaction/mod.rs 90.24% <100.00%> (+10.04%) ⬆️
src/meta/src/manager/id.rs 95.50% <0.00%> (-0.57%) ⬇️
src/frontend/src/expr/utils.rs 98.74% <0.00%> (-0.51%) ⬇️
src/storage/src/hummock/local_version_manager.rs 79.27% <0.00%> (+0.12%) ⬆️
src/connector/src/filesystem/file_common.rs 80.80% <0.00%> (+0.44%) ⬆️
src/meta/src/hummock/mock_hummock_meta_client.rs 41.50% <0.00%> (+0.94%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@soundOfDestiny soundOfDestiny requested a review from zwang28 June 28, 2022 11:54
@soundOfDestiny soundOfDestiny force-pushed the zl_appdelta branch 10 times, most recently from 7317ac8 to b272f6f Compare June 29, 2022 02:12
@soundOfDestiny soundOfDestiny force-pushed the zl_appdelta branch 2 times, most recently from 78536a0 to b80ec6e Compare June 29, 2022 04:29
Copy link
Contributor

@Little-Wallace Little-Wallace 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

Copy link
Contributor

@Little-Wallace Little-Wallace left a comment

Choose a reason for hiding this comment

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

LGTM

@Little-Wallace
Copy link
Contributor

Little-Wallace commented Jun 29, 2022

Would we clean deltas in another PR?

@mergify mergify bot merged commit 6548c8f into main Jun 29, 2022
@mergify mergify bot deleted the zl_appdelta branch June 29, 2022 17:46
@soundOfDestiny
Copy link
Contributor Author

Would we clean deltas in another PR?

here is a scheme:
In every restart, we delete all VersionDeltas and put a Version into meta store

is this OK?

@Little-Wallace
Copy link
Contributor

Would we clean deltas in another PR?

here is a scheme: In every restart, we delete all VersionDeltas and put a Version into meta store

is this OK?

No, it is not enough. If the meta-node lives a long time without restarts, we may append a lot of VersionDeltas to ETCD and it will cause a long time to recover during next restart.

huangjw806 pushed a commit that referenced this pull request Jul 5, 2022
#3529)

* refactor(hummock manager): load mem hummock version with version delta instead of version

* add hummock version apply trait

* refactor apply_compact_result

* hide_l0_remove_pos

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.

2 participants