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

Consider removing blob index deletion marker #165

Open
Connor1996 opened this issue May 26, 2020 · 2 comments
Open

Consider removing blob index deletion marker #165

Connor1996 opened this issue May 26, 2020 · 2 comments
Labels
sig/engine Component: Engine (SIG)

Comments

@Connor1996
Copy link
Member

Connor1996 commented May 26, 2020

image
As mentioned in the comment, before we use the deletion marker as a workaround for the empty result after merge. But actually, the result shouldn't be empty because it would expose stale versions of the key. Instead, it should output the delete type which provides two profits:

  • reduce code complexity
  • rocksdb compaction can help remove the delete record in the bottommost level instead of keeping the deletion marker forever

As we already make the merge operator support changing value type https://github.com/tikv/rocksdb/blob/6.4.tikv/include/rocksdb/merge_operator.h#L121,
seems we can remove the blob index deletion marker.

/cc @tabokie, not sure whether there was any other consideration before.
Also, please push facebook/rocksdb#6447 to be merged in upstream

@Connor1996 Connor1996 added the sig/engine Component: Engine (SIG) label May 26, 2020
@tabokie
Copy link
Member

tabokie commented May 26, 2020

@Connor1996 Currently merge operator doesn't support output delete type. I didn't implement it because there are certain complications from upper level: deleted value (in general a non-concrete result) requires a different set of processing logic, which is often seperated from the merge procedure. I'll attend to this once available.

@Connor1996
Copy link
Member Author

So the API support type changes, but the rocksdb implementation is not supported yet. Got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/engine Component: Engine (SIG)
Projects
None yet
Development

No branches or pull requests

2 participants