diff --git a/db/builder.cc b/db/builder.cc index 1bcd6afc1..9f127e895 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -194,7 +194,7 @@ Status BuildTable( blob_file_builder.get(), ioptions.allow_data_in_errors, /*compaction=*/nullptr, compaction_filter.get(), /*shutting_down=*/nullptr, - /*preserve_deletes_seqnum=*/0, /*manual_compaction_paused=*/nullptr, + /*manual_compaction_paused=*/nullptr, /*manual_compaction_canceled=*/nullptr, db_options.info_log, full_history_ts_low); diff --git a/db/compaction/compaction_iterator.cc b/db/compaction/compaction_iterator.cc index b52325c7f..7ee1dc15c 100644 --- a/db/compaction/compaction_iterator.cc +++ b/db/compaction/compaction_iterator.cc @@ -30,7 +30,6 @@ CompactionIterator::CompactionIterator( BlobFileBuilder* blob_file_builder, bool allow_data_in_errors, const Compaction* compaction, const CompactionFilter* compaction_filter, const std::atomic* shutting_down, - const SequenceNumber preserve_deletes_seqnum, const std::atomic* manual_compaction_paused, const std::atomic* manual_compaction_canceled, const std::shared_ptr info_log, @@ -42,9 +41,8 @@ CompactionIterator::CompactionIterator( blob_file_builder, allow_data_in_errors, std::unique_ptr( compaction ? new RealCompaction(compaction) : nullptr), - compaction_filter, shutting_down, preserve_deletes_seqnum, - manual_compaction_paused, manual_compaction_canceled, info_log, - full_history_ts_low) {} + compaction_filter, shutting_down, manual_compaction_paused, + manual_compaction_canceled, info_log, full_history_ts_low) {} CompactionIterator::CompactionIterator( InternalIterator* input, const Comparator* cmp, MergeHelper* merge_helper, @@ -57,7 +55,6 @@ CompactionIterator::CompactionIterator( std::unique_ptr compaction, const CompactionFilter* compaction_filter, const std::atomic* shutting_down, - const SequenceNumber preserve_deletes_seqnum, const std::atomic* manual_compaction_paused, const std::atomic* manual_compaction_canceled, const std::shared_ptr info_log, @@ -80,7 +77,6 @@ CompactionIterator::CompactionIterator( shutting_down_(shutting_down), manual_compaction_paused_(manual_compaction_paused), manual_compaction_canceled_(manual_compaction_canceled), - preserve_deletes_seqnum_(preserve_deletes_seqnum), info_log_(info_log), allow_data_in_errors_(allow_data_in_errors), timestamp_size_(cmp_ ? cmp_->timestamp_size() : 0), @@ -758,7 +754,6 @@ void CompactionIterator::NextFromInput() { (ikey_.type == kTypeDeletionWithTimestamp && cmp_with_history_ts_low_ < 0)) && DefinitelyInSnapshot(ikey_.sequence, earliest_snapshot_) && - ikeyNotNeededForIncrementalSnapshot() && compaction_->KeyNotExistsBeyondOutputLevel(ikey_.user_key, &level_ptrs_)) { // TODO(noetzli): This is the only place where we use compaction_ @@ -792,7 +787,7 @@ void CompactionIterator::NextFromInput() { } else if ((ikey_.type == kTypeDeletion || (ikey_.type == kTypeDeletionWithTimestamp && cmp_with_history_ts_low_ < 0)) && - bottommost_level_ && ikeyNotNeededForIncrementalSnapshot()) { + bottommost_level_) { // Handle the case where we have a delete key at the bottom most level // We can skip outputting the key iff there are no subsequent puts for this // key @@ -1060,8 +1055,7 @@ void CompactionIterator::PrepareOutput() { // Can we do the same for levels above bottom level as long as // KeyNotExistsBeyondOutputLevel() return true? if (valid_ && compaction_ != nullptr && - !compaction_->allow_ingest_behind() && - ikeyNotNeededForIncrementalSnapshot() && bottommost_level_ && + !compaction_->allow_ingest_behind() && bottommost_level_ && DefinitelyInSnapshot(ikey_.sequence, earliest_snapshot_) && ikey_.type != kTypeMerge) { assert(ikey_.type != kTypeDeletion); @@ -1139,13 +1133,6 @@ inline SequenceNumber CompactionIterator::findEarliestVisibleSnapshot( return kMaxSequenceNumber; } -// used in 2 places - prevents deletion markers to be dropped if they may be -// needed and disables seqnum zero-out in PrepareOutput for recent keys. -inline bool CompactionIterator::ikeyNotNeededForIncrementalSnapshot() { - return (!compaction_->preserve_deletes()) || - (ikey_.sequence < preserve_deletes_seqnum_); -} - uint64_t CompactionIterator::ComputeBlobGarbageCollectionCutoffFileNumber( const CompactionProxy* compaction) { if (!compaction) { diff --git a/db/compaction/compaction_iterator.h b/db/compaction/compaction_iterator.h index e7023578c..c02a8d7ec 100644 --- a/db/compaction/compaction_iterator.h +++ b/db/compaction/compaction_iterator.h @@ -92,8 +92,6 @@ class CompactionIterator { virtual bool allow_ingest_behind() const = 0; - virtual bool preserve_deletes() const = 0; - virtual bool allow_mmap_reads() const = 0; virtual bool enable_blob_garbage_collection() const = 0; @@ -139,8 +137,6 @@ class CompactionIterator { return compaction_->immutable_options()->allow_ingest_behind; } - bool preserve_deletes() const override { return false; } - bool allow_mmap_reads() const override { return compaction_->immutable_options()->allow_mmap_reads; } @@ -183,7 +179,6 @@ class CompactionIterator { const Compaction* compaction = nullptr, const CompactionFilter* compaction_filter = nullptr, const std::atomic* shutting_down = nullptr, - const SequenceNumber preserve_deletes_seqnum = 0, const std::atomic* manual_compaction_paused = nullptr, const std::atomic* manual_compaction_canceled = nullptr, const std::shared_ptr info_log = nullptr, @@ -201,7 +196,6 @@ class CompactionIterator { std::unique_ptr compaction, const CompactionFilter* compaction_filter = nullptr, const std::atomic* shutting_down = nullptr, - const SequenceNumber preserve_deletes_seqnum = 0, const std::atomic* manual_compaction_paused = nullptr, const std::atomic* manual_compaction_canceled = nullptr, const std::shared_ptr info_log = nullptr, @@ -272,11 +266,6 @@ class CompactionIterator { inline SequenceNumber findEarliestVisibleSnapshot( SequenceNumber in, SequenceNumber* prev_snapshot); - // Checks whether the currently seen ikey_ is needed for - // incremental (differential) snapshot and hence can't be dropped - // or seqnum be zero-ed out even if all other conditions for it are met. - inline bool ikeyNotNeededForIncrementalSnapshot(); - inline bool KeyCommitted(SequenceNumber sequence) { return snapshot_checker_ == nullptr || snapshot_checker_->CheckInSnapshot(sequence, kMaxSequenceNumber) == @@ -332,7 +321,6 @@ class CompactionIterator { const std::atomic* shutting_down_; const std::atomic* manual_compaction_paused_; const std::atomic* manual_compaction_canceled_; - const SequenceNumber preserve_deletes_seqnum_; bool bottommost_level_; bool valid_ = false; bool visible_at_tip_; diff --git a/db/compaction/compaction_iterator_test.cc b/db/compaction/compaction_iterator_test.cc index 02e7d164b..285cad057 100644 --- a/db/compaction/compaction_iterator_test.cc +++ b/db/compaction/compaction_iterator_test.cc @@ -166,8 +166,6 @@ class FakeCompaction : public CompactionIterator::CompactionProxy { bool allow_ingest_behind() const override { return is_allow_ingest_behind; } - bool preserve_deletes() const override { return false; } - bool allow_mmap_reads() const override { return false; } bool enable_blob_garbage_collection() const override { return false; } @@ -281,7 +279,7 @@ class CompactionIteratorTest : public testing::TestWithParam { Env::Default(), false /* report_detailed_time */, false, range_del_agg_.get(), nullptr /* blob_file_builder */, true /*allow_data_in_errors*/, std::move(compaction), filter, - &shutting_down_, /*preserve_deletes_seqnum=*/0, + &shutting_down_, /*manual_compaction_paused=*/nullptr, /*manual_compaction_canceled=*/nullptr, /*info_log=*/nullptr, full_history_ts_low)); diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 35535db0b..ef797307e 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -417,10 +417,10 @@ CompactionJob::CompactionJob( int job_id, Compaction* compaction, const ImmutableDBOptions& db_options, const MutableDBOptions& mutable_db_options, const FileOptions& file_options, VersionSet* versions, const std::atomic* shutting_down, - const SequenceNumber preserve_deletes_seqnum, LogBuffer* log_buffer, - FSDirectory* db_directory, FSDirectory* output_directory, - FSDirectory* blob_output_directory, Statistics* stats, - InstrumentedMutex* db_mutex, ErrorHandler* db_error_handler, + LogBuffer* log_buffer, FSDirectory* db_directory, + FSDirectory* output_directory, FSDirectory* blob_output_directory, + Statistics* stats, InstrumentedMutex* db_mutex, + ErrorHandler* db_error_handler, std::vector existing_snapshots, SequenceNumber earliest_write_conflict_snapshot, const SnapshotChecker* snapshot_checker, std::shared_ptr table_cache, @@ -456,7 +456,6 @@ CompactionJob::CompactionJob( shutting_down_(shutting_down), manual_compaction_paused_(manual_compaction_paused), manual_compaction_canceled_(manual_compaction_canceled), - preserve_deletes_seqnum_(preserve_deletes_seqnum), db_directory_(db_directory), blob_output_directory_(blob_output_directory), db_mutex_(db_mutex), @@ -1476,8 +1475,8 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) { /*expect_valid_internal_key=*/true, &range_del_agg, blob_file_builder.get(), db_options_.allow_data_in_errors, sub_compact->compaction, compaction_filter, shutting_down_, - preserve_deletes_seqnum_, manual_compaction_paused_, - manual_compaction_canceled_, db_options_.info_log, full_history_ts_low)); + manual_compaction_paused_, manual_compaction_canceled_, + db_options_.info_log, full_history_ts_low)); auto c_iter = sub_compact->c_iter.get(); c_iter->SeekToFirst(); if (c_iter->Valid() && sub_compact->compaction->output_level() != 0) { @@ -2494,7 +2493,7 @@ CompactionServiceCompactionJob::CompactionServiceCompactionJob( CompactionServiceResult* compaction_service_result) : CompactionJob( job_id, compaction, db_options, mutable_db_options, file_options, - versions, shutting_down, 0, log_buffer, nullptr, output_directory, + versions, shutting_down, log_buffer, nullptr, output_directory, nullptr, stats, db_mutex, db_error_handler, existing_snapshots, kMaxSequenceNumber, nullptr, table_cache, event_logger, compaction->mutable_cf_options()->paranoid_file_checks, diff --git a/db/compaction/compaction_job.h b/db/compaction/compaction_job.h index 2b6b1c2bc..17b4feccd 100644 --- a/db/compaction/compaction_job.h +++ b/db/compaction/compaction_job.h @@ -67,8 +67,7 @@ class CompactionJob { int job_id, Compaction* compaction, const ImmutableDBOptions& db_options, const MutableDBOptions& mutable_db_options, const FileOptions& file_options, VersionSet* versions, - const std::atomic* shutting_down, - const SequenceNumber preserve_deletes_seqnum, LogBuffer* log_buffer, + const std::atomic* shutting_down, LogBuffer* log_buffer, FSDirectory* db_directory, FSDirectory* output_directory, FSDirectory* blob_output_directory, Statistics* stats, InstrumentedMutex* db_mutex, ErrorHandler* db_error_handler, @@ -196,7 +195,6 @@ class CompactionJob { const std::atomic* shutting_down_; const std::atomic* manual_compaction_paused_; const std::atomic* manual_compaction_canceled_; - const SequenceNumber preserve_deletes_seqnum_; FSDirectory* db_directory_; FSDirectory* blob_output_directory_; InstrumentedMutex* db_mutex_; diff --git a/db/compaction/compaction_job_test.cc b/db/compaction/compaction_job_test.cc index d7a98177e..f87b8197d 100644 --- a/db/compaction/compaction_job_test.cc +++ b/db/compaction/compaction_job_test.cc @@ -87,7 +87,6 @@ class CompactionJobTestBase : public testing::Test { /*block_cache_tracer=*/nullptr, /*io_tracer=*/nullptr, /*db_session_id*/ "")), shutting_down_(false), - preserve_deletes_seqnum_(0), mock_table_factory_(new mock::MockTableFactory()), error_handler_(nullptr, db_options_, &mutex_), encode_u64_ts_(std::move(encode_u64_ts)) { @@ -354,8 +353,8 @@ class CompactionJobTestBase : public testing::Test { ucmp_->timestamp_size() == full_history_ts_low_.size()); CompactionJob compaction_job( 0, &compaction, db_options_, mutable_db_options_, env_options_, - versions_.get(), &shutting_down_, preserve_deletes_seqnum_, &log_buffer, - nullptr, nullptr, nullptr, nullptr, &mutex_, &error_handler_, snapshots, + versions_.get(), &shutting_down_, &log_buffer, nullptr, nullptr, + nullptr, nullptr, &mutex_, &error_handler_, snapshots, earliest_write_conflict_snapshot, snapshot_checker, table_cache_, &event_logger, false, false, dbname_, &compaction_job_stats_, Env::Priority::USER, nullptr /* IOTracer */, @@ -409,7 +408,6 @@ class CompactionJobTestBase : public testing::Test { std::unique_ptr versions_; InstrumentedMutex mutex_; std::atomic shutting_down_; - SequenceNumber preserve_deletes_seqnum_; std::shared_ptr mock_table_factory_; CompactionJobStats compaction_job_stats_; ColumnFamilyData* cfd_; diff --git a/db/db_impl/db_impl.h b/db/db_impl/db_impl.h index 39e310a4f..97e3d1b8a 100644 --- a/db/db_impl/db_impl.h +++ b/db/db_impl/db_impl.h @@ -2369,11 +2369,6 @@ class DBImpl : public DB { // DB::Open() or passed to us bool own_sfm_; - // Default value is 0 which means ALL deletes are - // preserved. Note that this has no effect if preserve_deletes is false. - const std::atomic preserve_deletes_seqnum_{0}; - const bool preserve_deletes_ = false; - // Flag to check whether Close() has been called on this DB bool closed_; // save the closing status, for re-calling the close() diff --git a/db/db_impl/db_impl_compaction_flush.cc b/db/db_impl/db_impl_compaction_flush.cc index c026d4e5d..6d893a8b7 100644 --- a/db/db_impl/db_impl_compaction_flush.cc +++ b/db/db_impl/db_impl_compaction_flush.cc @@ -1363,7 +1363,7 @@ Status DBImpl::CompactFilesImpl( CompactionJob compaction_job( job_context->job_id, c.get(), immutable_db_options_, mutable_db_options_, file_options_for_compaction_, versions_.get(), &shutting_down_, - preserve_deletes_seqnum_.load(), log_buffer, directories_.GetDbDir(), + log_buffer, directories_.GetDbDir(), GetDataDir(c->column_family_data(), c->output_path_id()), GetDataDir(c->column_family_data(), 0), stats_, &mutex_, &error_handler_, snapshot_seqs, earliest_write_conflict_snapshot, snapshot_checker, @@ -3357,8 +3357,7 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, CompactionJob compaction_job( job_context->job_id, c.get(), immutable_db_options_, mutable_db_options_, file_options_for_compaction_, versions_.get(), - &shutting_down_, preserve_deletes_seqnum_.load(), log_buffer, - directories_.GetDbDir(), + &shutting_down_, log_buffer, directories_.GetDbDir(), GetDataDir(c->column_family_data(), c->output_path_id()), GetDataDir(c->column_family_data(), 0), stats_, &mutex_, &error_handler_, snapshot_seqs, earliest_write_conflict_snapshot, diff --git a/db/db_iter.cc b/db/db_iter.cc index 8a854606b..d9da9624a 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -78,7 +78,6 @@ DBIter::DBIter(Env* _env, const ReadOptions& read_options, range_del_agg_(&ioptions.internal_comparator, s), db_impl_(db_impl), cfd_(cfd), - start_seqnum_(0ULL), timestamp_ub_(read_options.timestamp), timestamp_lb_(read_options.iter_start_ts), timestamp_size_(timestamp_ub_ ? timestamp_ub_->size() : 0) { @@ -328,25 +327,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key, case kTypeSingleDeletion: // Arrange to skip all upcoming entries for this key since // they are hidden by this deletion. - // if iterartor specified start_seqnum we - // 1) return internal key, including the type - // 2) return ikey only if ikey.seqnum >= start_seqnum_ - // note that if deletion seqnum is < start_seqnum_ we - // just skip it like in normal iterator. - if (start_seqnum_ > 0) { - if (ikey_.sequence >= start_seqnum_) { - saved_key_.SetInternalKey(ikey_); - valid_ = true; - return true; - } else { - saved_key_.SetUserKey( - ikey_.user_key, - !pin_thru_lifetime_ || - !iter_.iter()->IsKeyPinned() /* copy */); - skipping_saved_key = true; - PERF_COUNTER_ADD(internal_delete_skipped_count, 1); - } - } else if (timestamp_lb_) { + if (timestamp_lb_) { saved_key_.SetInternalKey(ikey_); valid_ = true; return true; @@ -360,28 +341,7 @@ bool DBIter::FindNextUserEntryInternal(bool skipping_saved_key, break; case kTypeValue: case kTypeBlobIndex: - if (start_seqnum_ > 0) { - if (ikey_.sequence >= start_seqnum_) { - saved_key_.SetInternalKey(ikey_); - - if (ikey_.type == kTypeBlobIndex) { - if (!SetBlobValueIfNeeded(ikey_.user_key, iter_.value())) { - return false; - } - } - - valid_ = true; - return true; - } else { - // this key and all previous versions shouldn't be included, - // skipping_saved_key - saved_key_.SetUserKey( - ikey_.user_key, - !pin_thru_lifetime_ || - !iter_.iter()->IsKeyPinned() /* copy */); - skipping_saved_key = true; - } - } else if (timestamp_lb_) { + if (timestamp_lb_) { saved_key_.SetInternalKey(ikey_); if (ikey_.type == kTypeBlobIndex) { diff --git a/db/db_iter.h b/db/db_iter.h index 20a3982f5..1c549bcf7 100644 --- a/db/db_iter.h +++ b/db/db_iter.h @@ -151,7 +151,7 @@ class DBIter final : public Iterator { } Slice key() const override { assert(valid_); - if (start_seqnum_ > 0 || timestamp_lb_) { + if (timestamp_lb_) { return saved_key_.GetInternalKey(); } else { const Slice ukey_and_ts = saved_key_.GetUserKey(); @@ -371,9 +371,6 @@ class DBIter final : public Iterator { ROCKSDB_FIELD_UNUSED #endif ColumnFamilyData* cfd_; - // for diff snapshots we want the lower bound on the seqnum; - // if this value > 0 iterator will return internal keys - SequenceNumber start_seqnum_; const Slice* const timestamp_ub_; const Slice* const timestamp_lb_; const size_t timestamp_size_; diff --git a/db/flush_job.cc b/db/flush_job.cc index d81ddb03a..55aeed19c 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -464,7 +464,7 @@ Status FlushJob::MemPurge() { nullptr, ioptions->allow_data_in_errors, /*compaction=*/nullptr, compaction_filter.get(), /*shutting_down=*/nullptr, - /*preserve_deletes_seqnum=*/0, /*manual_compaction_paused=*/nullptr, + /*manual_compaction_paused=*/nullptr, /*manual_compaction_canceled=*/nullptr, ioptions->info_log, &(cfd_->GetFullHistoryTsLow()));