facebook / facebook/rocksdb

PartialMergeMulti does not support a Noop output

Aperta
#3,655 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
C++
Stelle
32.1k
Fork
6.9k
Merge medio
32m
PR unite (30g)
1

Descrizione

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...

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.