facebook / facebook/rocksdb

PartialMergeMulti does not support a Noop output

Open
#3,655 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

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:

```C++
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...

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.