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: add base classes for user-specified compaction #731

Merged
merged 14 commits into from
May 28, 2021

Conversation

levy5307
Copy link
Contributor

@levy5307 levy5307 commented May 20, 2021

What problem does this PR solve?

RFC: #738

Add base classes for user-specified compaction.
compaction_operation represents the compaction operation, such as update ttl/delete key
compaction_filter_rule represents the compaction rule to filter the keys which are stored in rocksdb.

Checklist

Tests
  • Unit test
  • No code actually. I will add unit tests in later pull requests.

@acelyc111
Copy link
Member

acelyc111 commented May 22, 2021

What problem does this PR solve?

Add base classes for user-specified compaction.
compaction_operation represents the compaction operation, such as update ttl/delete key
compaction_filter_rule represents the compaction rule to filter the keys which are stored in rocksdb.

Better to add a proposal for this feature, even a Chinese doc.

src/server/compaction_filter_rule.h Outdated Show resolved Hide resolved
src/server/compaction_operation.cpp Outdated Show resolved Hide resolved
src/server/compaction_operation.h Show resolved Hide resolved
@hycdong
Copy link
Contributor

hycdong commented May 24, 2021

I agree with @acelyc111 , expecting your proposal and detailed descriptions and comments.

virtual ~compaction_filter_rule() = default;

// TODO(zhaoliwei): we can use `value_filed` to replace existing_value in the later,
// after the refactor of value schema
Copy link
Contributor

Choose a reason for hiding this comment

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

The refactor of value schema has already been merged, you can update it in next pull request :-)

Copy link
Contributor Author

@levy5307 levy5307 May 28, 2021

Choose a reason for hiding this comment

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

Yes, I will refactor it in the later

@acelyc111 acelyc111 merged commit 376dfc8 into apache:master May 28, 2021
@hycdong hycdong mentioned this pull request Sep 23, 2021
acelyc111 pushed a commit that referenced this pull request Jun 23, 2022
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