diff --git a/HISTORY.md b/HISTORY.md index b32d7bdce..abfd1a15c 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,6 +1,7 @@ # Rocksdb Change Log ## Unreleased ### Public API Change +* The merge operands are passed to `MergeOperator::ShouldMerge` in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2) for performance reasons ### New Features * Changes the format of index blocks by delta encoding the index values, which are the block handles. This saves the encoding of BlockHandle::offset of the non-head index entries in each restart interval. The feature is backward compatible but not forward compatible. It is disabled by default unless format_version 4 or above is used. * Add a new tool: trace_analyzer. Trace_analyzer analyzes the trace file generated by using trace_replay API. It can convert the binary format trace file to a human readable txt file, output the statistics of the analyzed query types such as access statistics and size statistics, combining the dumped whole key space file to analyze, support query correlation analyzing, and etc. Current supported query types are: Get, Put, Delete, SingleDelete, DeleteRange, Merge, Iterator (Seek, SeekForPrev only). diff --git a/db/memtable.cc b/db/memtable.cc index 6be872881..70e6d9da5 100644 --- a/db/memtable.cc +++ b/db/memtable.cc @@ -697,7 +697,7 @@ static bool SaveValue(void* arg, const char* entry) { *(s->merge_in_progress) = true; merge_context->PushOperand( v, s->inplace_update_support == false /* operand_pinned */); - if (merge_operator->ShouldMerge(merge_context->GetOperands())) { + if (merge_operator->ShouldMerge(merge_context->GetOperandsDirectionBackward())) { *(s->status) = MergeHelper::TimedFullMerge( merge_operator, s->key->user_key(), nullptr, merge_context->GetOperands(), s->value, s->logger, s->statistics, diff --git a/db/merge_context.h b/db/merge_context.h index 5e75e0997..c226f64e5 100644 --- a/db/merge_context.h +++ b/db/merge_context.h @@ -74,8 +74,13 @@ class MergeContext { return (*operand_list_)[index]; } - // Return all the operands. + // Same as GetOperandsDirectionForward const std::vector& GetOperands() { + return GetOperandsDirectionForward(); + } + + // Return all the operands in the order as they were merged (passed to FullMerge or FullMergeV2) + const std::vector& GetOperandsDirectionForward() { if (!operand_list_) { return empty_operand_list; } @@ -84,6 +89,16 @@ class MergeContext { return *operand_list_; } + // Return all the operands in the reversed order relative to how they were merged (passed to FullMerge or FullMergeV2) + const std::vector& GetOperandsDirectionBackward() { + if (!operand_list_) { + return empty_operand_list; + } + + SetDirectionBackward(); + return *operand_list_; + } + private: void Initialize() { if (!operand_list_) { diff --git a/include/rocksdb/merge_operator.h b/include/rocksdb/merge_operator.h index cd7563cff..8406d4a74 100644 --- a/include/rocksdb/merge_operator.h +++ b/include/rocksdb/merge_operator.h @@ -195,6 +195,11 @@ class MergeOperator { // during a point lookup, thereby helping in limiting the number of levels to // read from. // Doesn't help with iterators. + // + // Note: the merge operands are passed to this function in the reversed order + // relative to how they were merged (passed to FullMerge or FullMergeV2) + // for performance reasons, see also: + // https://github.com/facebook/rocksdb/issues/3865 virtual bool ShouldMerge(const std::vector& /*operands*/) const { return false; }