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.

Sign up for GitHub

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

Jump to bottom

feat: compaction integration #997

Merged
merged 8 commits into from
Feb 15, 2023
Merged

feat: compaction integration #997

merged 8 commits into from
Feb 15, 2023

Conversation

v0y4g3r
Copy link
Contributor

@v0y4g3r v0y4g3r commented Feb 14, 2023 •
edited
Loading

I hereby agree to the terms of the GreptimeDB CLA

  • Integrate compaction logic, triggers compaction on flush when SST num in level 0 reach some threshold.
  • Allow AccessLayer trait to accept an enum of data source so it can now accept both rows from memtable or chunk reader.
  • A bunch of mock components for compaction that can be used in tests.
  • Add some compaction related configs.

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)

#930

@v0y4g3r v0y4g3r force-pushed the feat/compaction-integrate branch from aaa221f to 10d9377 Compare February 14, 2023 09:57
@v0y4g3r v0y4g3r force-pushed the feat/compaction-integrate branch from 10d9377 to c04f2d4 Compare February 14, 2023 09:57
@v0y4g3r v0y4g3r changed the title feat: compaction integrate feat: compaction integration Feb 14, 2023
@v0y4g3r v0y4g3r marked this pull request as ready for review February 14, 2023 10:00
@v0y4g3r v0y4g3r requested review from evenyag, waynexia, killme2008 and fengjiachun and removed request for evenyag and waynexia February 14, 2023 10:00
@codecov
Copy link

codecov bot commented Feb 14, 2023 •
edited
Loading

Codecov Report

Merging #997 (daf2924) into develop (374acc8) will decrease coverage by 0.30%.
The diff coverage is 78.98%.

@@             Coverage Diff             @@
##           develop     #997      +/-   ##
===========================================
- Coverage    86.24%   85.95%   -0.30%     
===========================================
  Files          432      433       +1     
  Lines        62494    62663     +169     
===========================================
- Hits         53897    53861      -36     
- Misses        8597     8802     +205
Flag Coverage Δ
rust 85.95% <78.98%> (-0.30%) ⬇️

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

Impacted Files Coverage Δ
src/script/src/python/builtins.rs 87.12% <ø> (ø)
src/storage/src/compaction/rate_limit.rs 100.00% <ø> (ø)
src/storage/src/compaction/strategy.rs 85.36% <0.00%> (-2.08%) ⬇️
src/storage/src/compaction/writer.rs 100.00% <ø> (ø)
src/storage/src/lib.rs 100.00% <ø> (ø)
src/storage/src/compaction/noop.rs 19.23% <19.23%> (ø)
src/storage/src/compaction/task.rs 11.00% <26.66%> (+3.53%) ⬆️
src/storage/src/compaction/picker.rs 42.30% <60.00%> (+15.77%) ⬆️
src/storage/src/compaction/scheduler.rs 89.00% <61.90%> (-3.07%) ⬇️
src/storage/src/config.rs 75.00% <75.00%> (-25.00%) ⬇️
... and 31 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

evenyag
evenyag reviewed Feb 14, 2023
View reviewed changes
config/datanode.example.toml Outdated Show resolved Hide resolved
src/common/time/src/range.rs Show resolved Hide resolved
src/datanode/src/datanode.rs Outdated Show resolved Hide resolved
src/storage/src/compaction.rs Show resolved Hide resolved
src/storage/src/compaction/scheduler.rs Outdated Show resolved Hide resolved
src/storage/src/flush.rs Outdated Show resolved Hide resolved
src/storage/src/region.rs Show resolved Hide resolved
src/storage/src/region/writer.rs Outdated Show resolved Hide resolved
src/storage/src/region/writer.rs Outdated Show resolved Hide resolved
src/storage/src/sst/parquet.rs Show resolved Hide resolved
@v0y4g3r v0y4g3r mentioned this pull request Feb 14, 2023
Closed
15 tasks
@v0y4g3r v0y4g3r force-pushed the feat/compaction-integrate branch from e24605c to c9fdf4a Compare February 14, 2023 13:53
@v0y4g3r v0y4g3r force-pushed the feat/compaction-integrate branch from c9fdf4a to 00685df Compare February 14, 2023 13:57
evenyag
evenyag approved these changes Feb 14, 2023
View reviewed changes
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

waynexia
waynexia approved these changes Feb 15, 2023
View reviewed changes
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.

L6TM

src/storage/src/compaction/noop.rs Outdated Show resolved Hide resolved
src/storage/src/test_util/config_util.rs Outdated Show resolved Hide resolved
@v0y4g3r v0y4g3r merged commit 75b8afe into GreptimeTeam:develop Feb 15, 2023
@v0y4g3r v0y4g3r added this to the v0.1 milestone Feb 15, 2023
paomian pushed a commit to paomian/greptimedb that referenced this pull request Oct 19, 2023
@v0y4g3r @paomian
feat: compaction integration (GreptimeTeam#997)
1ce112e 
* feat: trigger compaction on flush

* chore: rebase develop

* feat: add config item max_file_in_level0 and remove compaction_after_flush

* fix: cr comments

* chore: add unit test to cover Timestamp::new_inclusive

* fix: workaround to fix future is not Sync

* fix: future is not sync

* fix: some cr comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@waynexia waynexia waynexia approved these changes

@evenyag evenyag evenyag approved these changes

@killme2008 killme2008 Awaiting requested review from killme2008

@fengjiachun fengjiachun Awaiting requested review from fengjiachun

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.1
Development

Successfully merging this pull request may close these issues.
3 participants
@v0y4g3r @evenyag @waynexia