Replace macros in compaction_iterator.cc with inline functions (#8592)

Summary:
Internal task T96186510.

Created new inline member functions in `CompactionIterator`,
`DefinitelyInSnapshot`, `DefinitelyNotInSnapshot`, and
`InEarliestSnapshot` to replace the macros at the top of
`compaction_iterator.cc`.

Placed the definitions in `compaction_iterator.h` in accordance with
Google's style guide for inline functions. Separated the declarations
and definitions, and only placed the `inline` keyword on the
definitions, in line with ISO CPP recommendations.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8592

Test Plan: Ran `make check`.  Successful build and all tests appeared to pass.

Reviewed By: riversand963

Differential Revision: D29966782

Pulled By: jimmycFB

fbshipit-source-id: 3584290bbbabf862e9ab58852281f46d37f58be6
main
jimmycleary 3 years ago committed by Facebook GitHub Bot
parent d6006f9c9b
commit e0ff365a76
  1. 30
      db/compaction/compaction_iterator.cc
  2. 28
      db/compaction/compaction_iterator.h

@ -16,22 +16,6 @@
#include "table/internal_iterator.h" #include "table/internal_iterator.h"
#include "test_util/sync_point.h" #include "test_util/sync_point.h"
#define DEFINITELY_IN_SNAPSHOT(seq, snapshot) \
((seq) <= (snapshot) && \
(snapshot_checker_ == nullptr || \
LIKELY(snapshot_checker_->CheckInSnapshot((seq), (snapshot)) == \
SnapshotCheckerResult::kInSnapshot)))
#define DEFINITELY_NOT_IN_SNAPSHOT(seq, snapshot) \
((seq) > (snapshot) || \
(snapshot_checker_ != nullptr && \
UNLIKELY(snapshot_checker_->CheckInSnapshot((seq), (snapshot)) == \
SnapshotCheckerResult::kNotInSnapshot)))
#define IN_EARLIEST_SNAPSHOT(seq) \
((seq) <= earliest_snapshot_ && \
(snapshot_checker_ == nullptr || LIKELY(IsInEarliestSnapshot(seq))))
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
CompactionIterator::CompactionIterator( CompactionIterator::CompactionIterator(
InternalIterator* input, const Comparator* cmp, MergeHelper* merge_helper, InternalIterator* input, const Comparator* cmp, MergeHelper* merge_helper,
@ -574,7 +558,7 @@ void CompactionIterator::NextFromInput() {
// Check whether the next key belongs to the same snapshot as the // Check whether the next key belongs to the same snapshot as the
// SingleDelete. // SingleDelete.
if (prev_snapshot == 0 || if (prev_snapshot == 0 ||
DEFINITELY_NOT_IN_SNAPSHOT(next_ikey.sequence, prev_snapshot)) { DefinitelyNotInSnapshot(next_ikey.sequence, prev_snapshot)) {
if (next_ikey.type == kTypeSingleDeletion) { if (next_ikey.type == kTypeSingleDeletion) {
// We encountered two SingleDeletes in a row. This could be due to // We encountered two SingleDeletes in a row. This could be due to
// unexpected user input. // unexpected user input.
@ -586,8 +570,8 @@ void CompactionIterator::NextFromInput() {
++iter_stats_.num_record_drop_obsolete; ++iter_stats_.num_record_drop_obsolete;
++iter_stats_.num_single_del_mismatch; ++iter_stats_.num_single_del_mismatch;
} else if (has_outputted_key_ || } else if (has_outputted_key_ ||
DEFINITELY_IN_SNAPSHOT( DefinitelyInSnapshot(ikey_.sequence,
ikey_.sequence, earliest_write_conflict_snapshot_)) { earliest_write_conflict_snapshot_)) {
// Found a matching value, we can drop the single delete and the // Found a matching value, we can drop the single delete and the
// value. It is safe to drop both records since we've already // value. It is safe to drop both records since we've already
// outputted a key in this snapshot, or there is no earlier // outputted a key in this snapshot, or there is no earlier
@ -635,7 +619,7 @@ void CompactionIterator::NextFromInput() {
// iteration. If the next key is corrupt, we return before the // iteration. If the next key is corrupt, we return before the
// comparison, so the value of has_current_user_key does not matter. // comparison, so the value of has_current_user_key does not matter.
has_current_user_key_ = false; has_current_user_key_ = false;
if (compaction_ != nullptr && IN_EARLIEST_SNAPSHOT(ikey_.sequence) && if (compaction_ != nullptr && InEarliestSnapshot(ikey_.sequence) &&
compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key,
&level_ptrs_)) { &level_ptrs_)) {
// Key doesn't exist outside of this range. // Key doesn't exist outside of this range.
@ -679,7 +663,7 @@ void CompactionIterator::NextFromInput() {
(ikey_.type == kTypeDeletion || (ikey_.type == kTypeDeletion ||
(ikey_.type == kTypeDeletionWithTimestamp && (ikey_.type == kTypeDeletionWithTimestamp &&
cmp_with_history_ts_low_ < 0)) && cmp_with_history_ts_low_ < 0)) &&
IN_EARLIEST_SNAPSHOT(ikey_.sequence) && InEarliestSnapshot(ikey_.sequence) &&
ikeyNotNeededForIncrementalSnapshot() && ikeyNotNeededForIncrementalSnapshot() &&
compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key,
&level_ptrs_)) { &level_ptrs_)) {
@ -734,7 +718,7 @@ void CompactionIterator::NextFromInput() {
.ok()) && .ok()) &&
cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key) && cmp_->EqualWithoutTimestamp(ikey_.user_key, next_ikey.user_key) &&
(prev_snapshot == 0 || (prev_snapshot == 0 ||
DEFINITELY_NOT_IN_SNAPSHOT(next_ikey.sequence, prev_snapshot))) { DefinitelyNotInSnapshot(next_ikey.sequence, prev_snapshot))) {
AdvanceInputIter(); AdvanceInputIter();
} }
// If you find you still need to output a row with this key, we need to output the // If you find you still need to output a row with this key, we need to output the
@ -977,7 +961,7 @@ void CompactionIterator::PrepareOutput() {
if (valid_ && compaction_ != nullptr && if (valid_ && compaction_ != nullptr &&
!compaction_->allow_ingest_behind() && !compaction_->allow_ingest_behind() &&
ikeyNotNeededForIncrementalSnapshot() && bottommost_level_ && ikeyNotNeededForIncrementalSnapshot() && bottommost_level_ &&
IN_EARLIEST_SNAPSHOT(ikey_.sequence) && ikey_.type != kTypeMerge) { InEarliestSnapshot(ikey_.sequence) && ikey_.type != kTypeMerge) {
assert(ikey_.type != kTypeDeletion && ikey_.type != kTypeSingleDeletion); assert(ikey_.type != kTypeDeletion && ikey_.type != kTypeSingleDeletion);
if (ikey_.type == kTypeDeletion || ikey_.type == kTypeSingleDeletion) { if (ikey_.type == kTypeDeletion || ikey_.type == kTypeSingleDeletion) {
ROCKS_LOG_FATAL(info_log_, ROCKS_LOG_FATAL(info_log_,

@ -269,6 +269,12 @@ class CompactionIterator {
bool IsInEarliestSnapshot(SequenceNumber sequence); bool IsInEarliestSnapshot(SequenceNumber sequence);
bool DefinitelyInSnapshot(SequenceNumber seq, SequenceNumber snapshot);
bool DefinitelyNotInSnapshot(SequenceNumber seq, SequenceNumber snapshot);
bool InEarliestSnapshot(SequenceNumber seq);
// Extract user-defined timestamp from user key if possible and compare it // Extract user-defined timestamp from user key if possible and compare it
// with *full_history_ts_low_ if applicable. // with *full_history_ts_low_ if applicable.
inline void UpdateTimestampAndCompareWithFullHistoryLow() { inline void UpdateTimestampAndCompareWithFullHistoryLow() {
@ -412,4 +418,26 @@ class CompactionIterator {
manual_compaction_canceled_->load(std::memory_order_relaxed)); manual_compaction_canceled_->load(std::memory_order_relaxed));
} }
}; };
inline bool CompactionIterator::DefinitelyInSnapshot(SequenceNumber seq,
SequenceNumber snapshot) {
return ((seq) <= (snapshot) &&
(snapshot_checker_ == nullptr ||
LIKELY(snapshot_checker_->CheckInSnapshot((seq), (snapshot)) ==
SnapshotCheckerResult::kInSnapshot)));
}
inline bool CompactionIterator::DefinitelyNotInSnapshot(
SequenceNumber seq, SequenceNumber snapshot) {
return ((seq) > (snapshot) ||
(snapshot_checker_ != nullptr &&
UNLIKELY(snapshot_checker_->CheckInSnapshot((seq), (snapshot)) ==
SnapshotCheckerResult::kNotInSnapshot)));
}
inline bool CompactionIterator::InEarliestSnapshot(SequenceNumber seq) {
return ((seq) <= earliest_snapshot_ &&
(snapshot_checker_ == nullptr || LIKELY(IsInEarliestSnapshot(seq))));
}
} // namespace ROCKSDB_NAMESPACE } // namespace ROCKSDB_NAMESPACE

Loading…
Cancel
Save