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

PartialMergeMulti does not support a Noop output #3655

Open
adamretter opened this issue Mar 26, 2018 · 0 comments
Open

PartialMergeMulti does not support a Noop output #3655

adamretter opened this issue Mar 26, 2018 · 0 comments

Comments

@adamretter
Copy link
Collaborator

adamretter commented Mar 26, 2018

It seems at the moment that MergeOperator::PartialMergeMulti must always produce a value.

However, consider that I have an operand list like so:

{
  operand[0]="Add(Item1)"
  operand[1]="Remove(Item1)"
}

My custom merge operator is able to realise that these two operations, actually reduce down to nothing i.e. no operation needs to be performed.

If I clear the output by calling new_value->clear(). The problem I see is that subsequent calls to MergeOperator::FullMergeV2 then have empty operands in the operand_list.
Whilst I could handle empty operands in my FullMergeV2, this seems like an unnecessary memory and processing overhead (especially if I have a large operand list, where all the operands are effectively empty).

I tried modifying db/merge_helper.cc (around line 341), so that if PartialMergeMulti results in an empty merge_result, then the result is ignored:

 if (merge_success) {
  // Merging of operands (associative merge) was successful.
  // Replace operands with the merge result
  merge_context_.Clear();
  if (!merge_result.empty()) {
    merge_context_.PushOperand(merge_result);
  }
  keys_.erase(keys_.begin(), keys_.end() - 1);
}

However I get assertion failures with regards to keys and values size. After further study it seems such a change is perhaps non-trivial to implement with the current design.

I do think that PartialMergeMulti being able to return nothing, is a valid state (as demonstrated by my custom MergeOperator above). Please advise as to the best way to move forward with this...

adamretter added a commit to adamretter/rocksdb that referenced this issue Feb 29, 2020
adamretter added a commit to adamretter/rocksdb that referenced this issue Jan 6, 2021
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

No branches or pull requests

1 participant