Revert "Support marking snapshots for write-conflict checking - Take 2"

This reverts commit e5c5f23814a995797d64fa3c2bc65dd0a1d27aaa.

diff --git a/db/builder.cc b/db/builder.cc
index 68b9fe804c77f953770e8553560ed1d58b41e844..bd695f1ddf7ac333647b5951b6f90dedaaaf50a6 100644 (file)
--- a/db/builder.cc
+++ b/db/builder.cc
@@ -96,8 +96,7 @@ Status BuildTable(
                       snapshots.empty() ? 0 : snapshots.back());
 
     CompactionIterator c_iter(iter, internal_comparator.user_comparator(),
-                              &merge, kMaxSequenceNumber, &snapshots,
-                              kMaxSequenceNumber, env,
+                              &merge, kMaxSequenceNumber, &snapshots, env,
                               true /* internal key corruption is not ok */);
     c_iter.SeekToFirst();
     for (; c_iter.Valid(); c_iter.Next()) {

diff --git a/db/compaction_iterator.cc b/db/compaction_iterator.cc
index 9cdc4feb60a971066a9d9e618a2843c967b19cbe..8d0dc7f62b0fdee05659a7b9092631939289609e 100644 (file)
--- a/db/compaction_iterator.cc
+++ b/db/compaction_iterator.cc
@@ -13,14 +13,12 @@ namespace rocksdb {
 CompactionIterator::CompactionIterator(
     InternalIterator* input, const Comparator* cmp, MergeHelper* merge_helper,
     SequenceNumber last_sequence, std::vector<SequenceNumber>* snapshots,
-    SequenceNumber earliest_write_conflict_snapshot, Env* env,
-    bool expect_valid_internal_key, Compaction* compaction,
+    Env* env, bool expect_valid_internal_key, Compaction* compaction,
     const CompactionFilter* compaction_filter, LogBuffer* log_buffer)
     : input_(input),
       cmp_(cmp),
       merge_helper_(merge_helper),
       snapshots_(snapshots),
-      earliest_write_conflict_snapshot_(earliest_write_conflict_snapshot),
       env_(env),
       expect_valid_internal_key_(expect_valid_internal_key),
       compaction_(compaction),
@@ -202,11 +200,6 @@ void CompactionIterator::NextFromInput() {
       ParsedInternalKey next_ikey;
       input_->Next();
 
-      if (earliest_write_conflict_snapshot_) {
-        // TODO(agiardullo): to be used in D50295
-        // adding this if statement to keep CLANG happy in the meantime
-      }
-
       // Check whether the current key is valid, not corrupt and the same
       // as the single delete.
       if (input_->Valid() && ParseInternalKey(input_->key(), &next_ikey) &&

diff --git a/db/compaction_iterator.h b/db/compaction_iterator.h
index 3be9112bc3402e83fd3555a0d1ee08f321137ec1..fdb8a7bf56912d4f5a3315b0fadfa5c57f5ae800 100644 (file)
--- a/db/compaction_iterator.h
+++ b/db/compaction_iterator.h
@@ -39,8 +39,7 @@ class CompactionIterator {
  public:
   CompactionIterator(InternalIterator* input, const Comparator* cmp,
                      MergeHelper* merge_helper, SequenceNumber last_sequence,
-                     std::vector<SequenceNumber>* snapshots,
-                     SequenceNumber earliest_write_conflict_snapshot, Env* env,
+                     std::vector<SequenceNumber>* snapshots, Env* env,
                      bool expect_valid_internal_key,
                      Compaction* compaction = nullptr,
                      const CompactionFilter* compaction_filter = nullptr,
@@ -89,7 +88,6 @@ class CompactionIterator {
   const Comparator* cmp_;
   MergeHelper* merge_helper_;
   const std::vector<SequenceNumber>* snapshots_;
-  const SequenceNumber earliest_write_conflict_snapshot_;
   Env* env_;
   bool expect_valid_internal_key_;
   Compaction* compaction_;

diff --git a/db/compaction_iterator_test.cc b/db/compaction_iterator_test.cc
index a59f567717332785902d1d1572d73316c13bdb32..1148c2ac7ac3ec188d7bfbc2e9a633929c9c1a79 100644 (file)
--- a/db/compaction_iterator_test.cc
+++ b/db/compaction_iterator_test.cc
@@ -20,9 +20,9 @@ class CompactionIteratorTest : public testing::Test {
                                         nullptr, 0U, false, 0));
     iter_.reset(new test::VectorIterator(ks, vs));
     iter_->SeekToFirst();
-    c_iter_.reset(new CompactionIterator(
-        iter_.get(), cmp_, merge_helper_.get(), last_sequence, &snapshots_,
-        kMaxSequenceNumber, Env::Default(), false));
+    c_iter_.reset(new CompactionIterator(iter_.get(), cmp_, merge_helper_.get(),
+                                         last_sequence, &snapshots_,
+                                         Env::Default(), false));
   }
 
   const Comparator* cmp_;

diff --git a/db/compaction_job.cc b/db/compaction_job.cc
index 4806eff7e23d39c05d450b624c712d081ff032b7..ea052b84f3c3975fc47fa4fbc56e2d7190739efb 100644 (file)
--- a/db/compaction_job.cc
+++ b/db/compaction_job.cc
@@ -212,7 +212,6 @@ CompactionJob::CompactionJob(
     std::atomic<bool>* shutting_down, LogBuffer* log_buffer,
     Directory* db_directory, Directory* output_directory, Statistics* stats,
     std::vector<SequenceNumber> existing_snapshots,
-    SequenceNumber earliest_write_conflict_snapshot,
     std::shared_ptr<Cache> table_cache, EventLogger* event_logger,
     bool paranoid_file_checks, bool measure_io_stats, const std::string& dbname,
     CompactionJobStats* compaction_job_stats)
@@ -231,7 +230,6 @@ CompactionJob::CompactionJob(
       output_directory_(output_directory),
       stats_(stats),
       existing_snapshots_(std::move(existing_snapshots)),
-      earliest_write_conflict_snapshot_(earliest_write_conflict_snapshot),
       table_cache_(std::move(table_cache)),
       event_logger_(event_logger),
       paranoid_file_checks_(paranoid_file_checks),
@@ -640,8 +638,8 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
   Status status;
   sub_compact->c_iter.reset(new CompactionIterator(
       input.get(), cfd->user_comparator(), &merge, versions_->LastSequence(),
-      &existing_snapshots_, earliest_write_conflict_snapshot_, env_, false,
-      sub_compact->compaction, compaction_filter));
+      &existing_snapshots_, env_, false, sub_compact->compaction,
+      compaction_filter));
   auto c_iter = sub_compact->c_iter.get();
   c_iter->SeekToFirst();
   const auto& c_iter_stats = c_iter->iter_stats();

diff --git a/db/compaction_job.h b/db/compaction_job.h
index cfb57ce2ddb45779abff14ea240d8f604b006079..ab71519f43a226b43260a59f8c3531be1046ea33 100644 (file)
--- a/db/compaction_job.h
+++ b/db/compaction_job.h
@@ -58,7 +58,6 @@ class CompactionJob {
                 Directory* db_directory, Directory* output_directory,
                 Statistics* stats,
                 std::vector<SequenceNumber> existing_snapshots,
-                SequenceNumber earliest_write_conflict_snapshot,
                 std::shared_ptr<Cache> table_cache, EventLogger* event_logger,
                 bool paranoid_file_checks, bool measure_io_stats,
                 const std::string& dbname,
@@ -135,12 +134,6 @@ class CompactionJob {
   // entirely within s1 and s2, then the earlier version of k1 can be safely
   // deleted because that version is not visible in any snapshot.
   std::vector<SequenceNumber> existing_snapshots_;
-
-  // This is the earliest snapshot that could be used for write-conflict
-  // checking by a transaction.  For any user-key newer than this snapshot, we
-  // should make sure not to remove evidence that a write occured.
-  SequenceNumber earliest_write_conflict_snapshot_;
-
   std::shared_ptr<Cache> table_cache_;
 
   EventLogger* event_logger_;

diff --git a/db/compaction_job_test.cc b/db/compaction_job_test.cc
index 57c5a4aeca41af9ad9adb7da91373fe885d7befb..d4ae5f2f3d4fecea1d9534d0dda6c254e9286a5b 100644 (file)
--- a/db/compaction_job_test.cc
+++ b/db/compaction_job_test.cc
@@ -243,11 +243,11 @@ class CompactionJobTest : public testing::Test {
     LogBuffer log_buffer(InfoLogLevel::INFO_LEVEL, db_options_.info_log.get());
     mutex_.Lock();
     EventLogger event_logger(db_options_.info_log.get());
-    CompactionJob compaction_job(
-        0, &compaction, db_options_, env_options_, versions_.get(),
-        &shutting_down_, &log_buffer, nullptr, nullptr, nullptr, snapshots,
-        kMaxSequenceNumber, table_cache_, &event_logger, false, false, dbname_,
-        &compaction_job_stats_);
+    CompactionJob compaction_job(0, &compaction, db_options_, env_options_,
+                                 versions_.get(), &shutting_down_, &log_buffer,
+                                 nullptr, nullptr, nullptr, snapshots,
+                                 table_cache_, &event_logger, false, false,
+                                 dbname_, &compaction_job_stats_);
 
     VerifyInitializationOfCompactionJobStats(compaction_job_stats_);
 

diff --git a/db/db_impl.cc b/db/db_impl.cc
index e62e228329579a75dfd43aeb312a4a96b2331995..1e05bc8b397a316e799578758b1ca0579925d2ec 100644 (file)
--- a/db/db_impl.cc
+++ b/db/db_impl.cc
@@ -1779,16 +1779,12 @@ Status DBImpl::CompactFilesImpl(
   // deletion compaction currently not allowed in CompactFiles.
   assert(!c->deletion_compaction());
 
-  SequenceNumber earliest_write_conflict_snapshot;
-  std::vector<SequenceNumber> snapshot_seqs =
-      snapshots_.GetAll(&earliest_write_conflict_snapshot);
-
   assert(is_snapshot_supported_ || snapshots_.empty());
   CompactionJob compaction_job(
       job_context->job_id, c.get(), db_options_, env_options_, versions_.get(),
       &shutting_down_, log_buffer, directories_.GetDbDir(),
-      directories_.GetDataDir(c->output_path_id()), stats_, snapshot_seqs,
-      earliest_write_conflict_snapshot, table_cache_, &event_logger_,
+      directories_.GetDataDir(c->output_path_id()), stats_, snapshots_.GetAll(),
+      table_cache_, &event_logger_,
       c->mutable_cf_options()->paranoid_file_checks,
       c->mutable_cf_options()->compaction_measure_io_stats, dbname_,
       nullptr);  // Here we pass a nullptr for CompactionJobStats because
@@ -2879,17 +2875,12 @@ Status DBImpl::BackgroundCompaction(bool* made_progress,
     int output_level  __attribute__((unused)) = c->output_level();
     TEST_SYNC_POINT_CALLBACK("DBImpl::BackgroundCompaction:NonTrivial",
                              &output_level);
-
-    SequenceNumber earliest_write_conflict_snapshot;
-    std::vector<SequenceNumber> snapshot_seqs =
-        snapshots_.GetAll(&earliest_write_conflict_snapshot);
-
     assert(is_snapshot_supported_ || snapshots_.empty());
     CompactionJob compaction_job(
         job_context->job_id, c.get(), db_options_, env_options_,
         versions_.get(), &shutting_down_, log_buffer, directories_.GetDbDir(),
-        directories_.GetDataDir(c->output_path_id()), stats_, snapshot_seqs,
-        earliest_write_conflict_snapshot, table_cache_, &event_logger_,
+        directories_.GetDataDir(c->output_path_id()), stats_,
+        snapshots_.GetAll(), table_cache_, &event_logger_,
         c->mutable_cf_options()->paranoid_file_checks,
         c->mutable_cf_options()->compaction_measure_io_stats, dbname_,
         &compaction_job_stats);
@@ -3811,13 +3802,7 @@ Status DBImpl::NewIterators(
   return Status::OK();
 }
 
-const Snapshot* DBImpl::GetSnapshot() { return GetSnapshotImpl(false); }
-
-const Snapshot* DBImpl::GetSnapshotForWriteConflictBoundary() {
-  return GetSnapshotImpl(true);
-}
-
-const Snapshot* DBImpl::GetSnapshotImpl(bool is_write_conflict_boundary) {
+const Snapshot* DBImpl::GetSnapshot() {
   int64_t unix_time = 0;
   env_->GetCurrentTime(&unix_time);  // Ignore error
   SnapshotImpl* s = new SnapshotImpl;
@@ -3828,8 +3813,7 @@ const Snapshot* DBImpl::GetSnapshotImpl(bool is_write_conflict_boundary) {
     delete s;
     return nullptr;
   }
-  return snapshots_.New(s, versions_->LastSequence(), unix_time,
-                        is_write_conflict_boundary);
+  return snapshots_.New(s, versions_->LastSequence(), unix_time);
 }
 
 void DBImpl::ReleaseSnapshot(const Snapshot* s) {

diff --git a/db/db_impl.h b/db/db_impl.h
index a02f47bfbebeed7fca9d3eb22193a0bf02d7d996..d016af7b4f0ca02a4ab6fc1d512866ccc5bdbf54 100644 (file)
--- a/db/db_impl.h
+++ b/db/db_impl.h
@@ -243,12 +243,6 @@ class DBImpl : public DB {
 
 #endif  // ROCKSDB_LITE
 
-  // Similar to GetSnapshot(), but also lets the db know that this snapshot
-  // will be used for transaction write-conflict checking.  The DB can then
-  // make sure not to compact any keys that would prevent a write-conflict from
-  // being detected.
-  const Snapshot* GetSnapshotForWriteConflictBoundary();
-
   // checks if all live files exist on file system and that their file sizes
   // match to our in-memory records
   virtual Status CheckConsistency();
@@ -570,8 +564,6 @@ class DBImpl : public DB {
   // helper function to call after some of the logs_ were synced
   void MarkLogsSynced(uint64_t up_to, bool synced_dir, const Status& status);
 
-  const Snapshot* GetSnapshotImpl(bool is_write_conflict_boundary);
-
   // table_cache_ provides its own synchronization
   std::shared_ptr<Cache> table_cache_;
 

diff --git a/db/snapshot_impl.cc b/db/snapshot_impl.cc
index d901b61d27817f827b388c3d5eef648d6e4317d9..1546d68f69ea2accaef49935670cc87b965e7fe9 100644 (file)
--- a/db/snapshot_impl.cc
+++ b/db/snapshot_impl.cc
@@ -12,9 +12,6 @@ namespace rocksdb {
 ManagedSnapshot::ManagedSnapshot(DB* db) : db_(db),
                                            snapshot_(db->GetSnapshot()) {}
 
-ManagedSnapshot::ManagedSnapshot(DB* db, const Snapshot* _snapshot)
-    : db_(db), snapshot_(_snapshot) {}
-
 ManagedSnapshot::~ManagedSnapshot() {
   if (snapshot_) {
     db_->ReleaseSnapshot(snapshot_);

diff --git a/db/snapshot_impl.h b/db/snapshot_impl.h
index 4fa0bb9d59814afeb895f78a2538eb84fce66826..b4d58fdf01ba4e4939cd872ccee909ef6501d842 100644 (file)
--- a/db/snapshot_impl.h
+++ b/db/snapshot_impl.h
@@ -34,9 +34,6 @@ class SnapshotImpl : public Snapshot {
   SnapshotList* list_;                 // just for sanity checks
 
   int64_t unix_time_;
-
-  // Will this snapshot be used by a Transaction to do write-conflict checking?
-  bool is_write_conflict_boundary_;
 };
 
 class SnapshotList {
@@ -53,10 +50,9 @@ class SnapshotList {
   SnapshotImpl* newest() const { assert(!empty()); return list_.prev_; }
 
   const SnapshotImpl* New(SnapshotImpl* s, SequenceNumber seq,
-                          uint64_t unix_time, bool is_write_conflict_boundary) {
+                          uint64_t unix_time) {
     s->number_ = seq;
     s->unix_time_ = unix_time;
-    s->is_write_conflict_boundary_ = is_write_conflict_boundary;
     s->list_ = this;
     s->next_ = &list_;
     s->prev_ = list_.prev_;
@@ -75,29 +71,14 @@ class SnapshotList {
   }
 
   // retrieve all snapshot numbers. They are sorted in ascending order.
-  std::vector<SequenceNumber> GetAll(
-      SequenceNumber* oldest_write_conflict_snapshot = nullptr) {
+  std::vector<SequenceNumber> GetAll() {
     std::vector<SequenceNumber> ret;
-
-    if (oldest_write_conflict_snapshot != nullptr) {
-      *oldest_write_conflict_snapshot = kMaxSequenceNumber;
-    }
-
     if (empty()) {
       return ret;
     }
     SnapshotImpl* s = &list_;
     while (s->next_ != &list_) {
       ret.push_back(s->next_->number_);
-
-      if (oldest_write_conflict_snapshot != nullptr &&
-          *oldest_write_conflict_snapshot != kMaxSequenceNumber &&
-          s->next_->is_write_conflict_boundary_) {
-        // If this is the first write-conflict boundary snapshot in the list,
-        // it is the oldest
-        *oldest_write_conflict_snapshot = s->next_->number_;
-      }
-
       s = s->next_;
     }
     return ret;

diff --git a/include/rocksdb/snapshot.h b/include/rocksdb/snapshot.h
index 95822d297bdbaa252a7d2d217dd5ba86ab60da94..aad675b4b2f5a9ce75ddd1741004dfa7491d817f 100644 (file)
--- a/include/rocksdb/snapshot.h
+++ b/include/rocksdb/snapshot.h
@@ -33,9 +33,6 @@ class ManagedSnapshot {
  public:
   explicit ManagedSnapshot(DB* db);
 
-  // Instead of creating a snapshot, take ownership of the input snapshot.
-  ManagedSnapshot(DB* db, const Snapshot* _snapshot);
-
   ~ManagedSnapshot();
 
   const Snapshot* snapshot();

diff --git a/utilities/transactions/transaction_base.cc b/utilities/transactions/transaction_base.cc
index 5f3e97e9bd2ea1acadfd1b25ddc141e3f4d87f38..d31ac6a26ed0369cd8d8030ce156782d290e49e9 100644 (file)
--- a/utilities/transactions/transaction_base.cc
+++ b/utilities/transactions/transaction_base.cc
@@ -7,7 +7,6 @@
 
 #include "utilities/transactions/transaction_base.h"
 
-#include "db/db_impl.h"
 #include "db/column_family.h"
 #include "rocksdb/comparator.h"
 #include "rocksdb/db.h"
@@ -36,11 +35,7 @@ void TransactionBaseImpl::Clear() {
 }
 
 void TransactionBaseImpl::SetSnapshot() {
-  assert(dynamic_cast<DBImpl*>(db_) != nullptr);
-  auto db_impl = reinterpret_cast<DBImpl*>(db_);
-
-  const Snapshot* snapshot = db_impl->GetSnapshotForWriteConflictBoundary();
-  snapshot_.reset(new ManagedSnapshot(db_, snapshot));
+  snapshot_.reset(new ManagedSnapshot(db_));
   snapshot_needed_ = false;
   snapshot_notifier_ = nullptr;
 }