From 25fd743d75017c2d92e8b0e84c45a4f054208ecf Mon Sep 17 00:00:00 2001 From: agiardullo Date: Fri, 25 Sep 2015 12:23:07 -0700 Subject: [PATCH] Fix SingleDelete support in WriteBatchWithIndex Summary: Fixed some bugs in using SingleDelete on a WriteBatchWithIndex and added some tests. Test Plan: new tests Reviewers: sdong, yhchiang, rven, kradhakrishnan, IslamAbdelRahman, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D47529 --- .../utilities/write_batch_with_index.h | 21 +- .../write_batch_with_index.cc | 8 +- .../write_batch_with_index_internal.cc | 3 +- .../write_batch_with_index_test.cc | 217 +++++++++++++++--- 4 files changed, 213 insertions(+), 36 deletions(-) diff --git a/include/rocksdb/utilities/write_batch_with_index.h b/include/rocksdb/utilities/write_batch_with_index.h index 6d75c4b91..ea096efa1 100644 --- a/include/rocksdb/utilities/write_batch_with_index.h +++ b/include/rocksdb/utilities/write_batch_with_index.h @@ -37,8 +37,8 @@ enum WriteType { kLogDataRecord }; -// an entry for Put, Merge or Delete entry for write batches. Used in -// WBWIIterator. +// an entry for Put, Merge, Delete, or SingleDelete entry for write batches. +// Used in WBWIIterator. struct WriteEntry { WriteType type; Slice key; @@ -71,8 +71,8 @@ class WBWIIterator { // A WriteBatchWithIndex with a binary searchable index built for all the keys // inserted. -// In Put(), Merge() or Delete(), the same function of the wrapped will be -// called. At the same time, indexes will be built. +// In Put(), Merge() Delete(), or SingleDelete(), the same function of the +// wrapped will be called. At the same time, indexes will be built. // By calling GetWriteBatch(), a user will get the WriteBatch for the data // they inserted, which can be used for DB::Write(). // A user can call NewIterator() to create an iterator. @@ -126,12 +126,18 @@ class WriteBatchWithIndex : public WriteBatchBase { // order given by index_comparator. For multiple updates on the same key, // each update will be returned as a separate entry, in the order of update // time. + // + // The returned iterator should be deleted by the caller. WBWIIterator* NewIterator(ColumnFamilyHandle* column_family); // Create an iterator of the default column family. WBWIIterator* NewIterator(); // Will create a new Iterator that will use WBWIIterator as a delta and - // base_iterator as base + // base_iterator as base. + // + // The returned iterator should be deleted by the caller. + // The base_iterator is now 'owned' by the returned iterator. Deleting the + // returned iterator will also delete the base_iterator. Iterator* NewIteratorWithBase(ColumnFamilyHandle* column_family, Iterator* base_iterator); // default column family @@ -172,8 +178,9 @@ class WriteBatchWithIndex : public WriteBatchBase { // May be called multiple times to set multiple save points. void SetSavePoint() override; - // Remove all entries in this batch (Put, Merge, Delete, PutLogData) since the - // most recent call to SetSavePoint() and removes the most recent save point. + // Remove all entries in this batch (Put, Merge, Delete, SingleDelete, + // PutLogData) since the most recent call to SetSavePoint() and removes the + // most recent save point. // If there is no previous call to SetSavePoint(), behaves the same as // Clear(). // diff --git a/utilities/write_batch_with_index/write_batch_with_index.cc b/utilities/write_batch_with_index/write_batch_with_index.cc index 189e557ed..4b531c3a2 100644 --- a/utilities/write_batch_with_index/write_batch_with_index.cc +++ b/utilities/write_batch_with_index/write_batch_with_index.cc @@ -238,7 +238,8 @@ class BaseDeltaIterator : public Iterator { // Finished return; } - if (delta_entry.type == kDeleteRecord) { + if (delta_entry.type == kDeleteRecord || + delta_entry.type == kSingleDeleteRecord) { AdvanceDelta(); } else { current_at_base_ = false; @@ -256,7 +257,8 @@ class BaseDeltaIterator : public Iterator { if (compare == 0) { equal_keys_ = true; } - if (delta_entry.type != kDeleteRecord) { + if (delta_entry.type != kDeleteRecord && + delta_entry.type != kSingleDeleteRecord) { current_at_base_ = false; return; } @@ -507,6 +509,8 @@ void WriteBatchWithIndex::Rep::AddNewEntry(uint32_t column_family_id) { case kTypeValue: case kTypeColumnFamilyDeletion: case kTypeDeletion: + case kTypeColumnFamilySingleDeletion: + case kTypeSingleDeletion: case kTypeColumnFamilyMerge: case kTypeMerge: found++; diff --git a/utilities/write_batch_with_index/write_batch_with_index_internal.cc b/utilities/write_batch_with_index/write_batch_with_index_internal.cc index eda20150c..d336dddea 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_internal.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_internal.cc @@ -189,7 +189,8 @@ WriteBatchWithIndexInternal::Result WriteBatchWithIndexInternal::GetFromBatch( merge_context->PushOperand(entry.value); break; } - case kDeleteRecord: { + case kDeleteRecord: + case kSingleDeleteRecord: { result = WriteBatchWithIndexInternal::Result::kDeleted; break; } diff --git a/utilities/write_batch_with_index/write_batch_with_index_test.cc b/utilities/write_batch_with_index/write_batch_with_index_test.cc index 3e509ca93..96f1c4b47 100644 --- a/utilities/write_batch_with_index/write_batch_with_index_test.cc +++ b/utilities/write_batch_with_index/write_batch_with_index_test.cc @@ -1390,6 +1390,10 @@ static std::string PrintContents(WriteBatchWithIndex* batch, result.append(e.key.ToString()); result.append("):"); result.append(e.value.ToString()); + } else if (e.type == kSingleDeleteRecord) { + result.append("SINGLE-DEL("); + result.append(e.key.ToString()); + result.append(")"); } else { assert(e.type == kDeleteRecord); result.append("DEL("); @@ -1405,6 +1409,36 @@ static std::string PrintContents(WriteBatchWithIndex* batch, return result; } +static std::string PrintContents(WriteBatchWithIndex* batch, KVMap* base_map, + ColumnFamilyHandle* column_family) { + std::string result; + + Iterator* iter; + if (column_family == nullptr) { + iter = batch->NewIteratorWithBase(new KVIter(base_map)); + } else { + iter = batch->NewIteratorWithBase(column_family, new KVIter(base_map)); + } + + iter->SeekToFirst(); + while (iter->Valid()) { + assert(iter->status().ok()); + + Slice key = iter->key(); + Slice value = iter->value(); + + result.append(key.ToString()); + result.append(":"); + result.append(value.ToString()); + result.append(","); + + iter->Next(); + } + + delete iter; + return result; +} + TEST_F(WriteBatchWithIndexTest, SavePointTest) { WriteBatchWithIndex batch; ColumnFamilyHandleImplDummy cf1(1, BytewiseComparator()); @@ -1416,96 +1450,227 @@ TEST_F(WriteBatchWithIndexTest, SavePointTest) { batch.Put(&cf1, "A", "a1"); batch.Delete(&cf1, "B"); batch.Put(&cf1, "C", "c1"); + batch.Put(&cf1, "E", "e1"); - batch.SetSavePoint(); + batch.SetSavePoint(); // 1 batch.Put("C", "cc"); batch.Put("B", "bb"); batch.Delete("A"); batch.Put(&cf1, "B", "b1"); batch.Delete(&cf1, "A"); - batch.SetSavePoint(); + batch.SingleDelete(&cf1, "E"); + batch.SetSavePoint(); // 2 batch.Put("A", "aaa"); batch.Put("A", "xxx"); batch.Delete("B"); batch.Put(&cf1, "B", "b2"); batch.Delete(&cf1, "C"); - batch.SetSavePoint(); - batch.SetSavePoint(); - batch.Delete("D"); + batch.SetSavePoint(); // 3 + batch.SetSavePoint(); // 4 + batch.SingleDelete("D"); batch.Delete(&cf1, "D"); + batch.Delete(&cf1, "E"); ASSERT_EQ( "PUT(A):a,PUT(A):aa,DEL(A),PUT(A):aaa,PUT(A):xxx,PUT(B):b,PUT(B):bb,DEL(" "B)" - ",PUT(C):cc,DEL(D),", + ",PUT(C):cc,SINGLE-DEL(D),", PrintContents(&batch, nullptr)); ASSERT_EQ( "PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(B):b2,PUT(C):c1,DEL(C)," - "DEL(D),", + "DEL(D),PUT(E):e1,SINGLE-DEL(E),DEL(E),", PrintContents(&batch, &cf1)); - ASSERT_OK(batch.RollbackToSavePoint()); + ASSERT_OK(batch.RollbackToSavePoint()); // rollback to 4 ASSERT_EQ( "PUT(A):a,PUT(A):aa,DEL(A),PUT(A):aaa,PUT(A):xxx,PUT(B):b,PUT(B):bb,DEL(" "B)" ",PUT(C):cc,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(B):b2,PUT(C):c1,DEL(C),", - PrintContents(&batch, &cf1)); + ASSERT_EQ( + "PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(B):b2,PUT(C):c1,DEL(C)," + "PUT(E):e1,SINGLE-DEL(E),", + PrintContents(&batch, &cf1)); - ASSERT_OK(batch.RollbackToSavePoint()); + ASSERT_OK(batch.RollbackToSavePoint()); // rollback to 3 ASSERT_EQ( "PUT(A):a,PUT(A):aa,DEL(A),PUT(A):aaa,PUT(A):xxx,PUT(B):b,PUT(B):bb,DEL(" "B)" ",PUT(C):cc,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(B):b2,PUT(C):c1,DEL(C),", - PrintContents(&batch, &cf1)); + ASSERT_EQ( + "PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(B):b2,PUT(C):c1,DEL(C)," + "PUT(E):e1,SINGLE-DEL(E),", + PrintContents(&batch, &cf1)); - ASSERT_OK(batch.RollbackToSavePoint()); + ASSERT_OK(batch.RollbackToSavePoint()); // rollback to 2 ASSERT_EQ("PUT(A):a,PUT(A):aa,DEL(A),PUT(B):b,PUT(B):bb,PUT(C):cc,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(C):c1,", - PrintContents(&batch, &cf1)); + ASSERT_EQ( + "PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(C):c1," + "PUT(E):e1,SINGLE-DEL(E),", + PrintContents(&batch, &cf1)); - batch.SetSavePoint(); + batch.SetSavePoint(); // 5 batch.Put("X", "x"); ASSERT_EQ("PUT(A):a,PUT(A):aa,DEL(A),PUT(B):b,PUT(B):bb,PUT(C):cc,PUT(X):x,", PrintContents(&batch, nullptr)); - ASSERT_OK(batch.RollbackToSavePoint()); + ASSERT_OK(batch.RollbackToSavePoint()); // rollback to 5 ASSERT_EQ("PUT(A):a,PUT(A):aa,DEL(A),PUT(B):b,PUT(B):bb,PUT(C):cc,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(C):c1,", - PrintContents(&batch, &cf1)); + ASSERT_EQ( + "PUT(A):a1,DEL(A),DEL(B),PUT(B):b1,PUT(C):c1," + "PUT(E):e1,SINGLE-DEL(E),", + PrintContents(&batch, &cf1)); - ASSERT_OK(batch.RollbackToSavePoint()); + ASSERT_OK(batch.RollbackToSavePoint()); // rollback to 1 ASSERT_EQ("PUT(A):a,PUT(A):aa,PUT(B):b,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(B),PUT(C):c1,", PrintContents(&batch, &cf1)); + ASSERT_EQ("PUT(A):a1,DEL(B),PUT(C):c1,PUT(E):e1,", + PrintContents(&batch, &cf1)); - s = batch.RollbackToSavePoint(); + s = batch.RollbackToSavePoint(); // no savepoint found ASSERT_TRUE(s.IsNotFound()); ASSERT_EQ("PUT(A):a,PUT(A):aa,PUT(B):b,", PrintContents(&batch, nullptr)); - ASSERT_EQ("PUT(A):a1,DEL(B),PUT(C):c1,", PrintContents(&batch, &cf1)); + ASSERT_EQ("PUT(A):a1,DEL(B),PUT(C):c1,PUT(E):e1,", + PrintContents(&batch, &cf1)); - batch.SetSavePoint(); + batch.SetSavePoint(); // 6 batch.Clear(); ASSERT_EQ("", PrintContents(&batch, nullptr)); ASSERT_EQ("", PrintContents(&batch, &cf1)); - s = batch.RollbackToSavePoint(); + s = batch.RollbackToSavePoint(); // rollback to 6 + ASSERT_TRUE(s.IsNotFound()); +} + +TEST_F(WriteBatchWithIndexTest, SingleDeleteTest) { + WriteBatchWithIndex batch; + Status s; + std::string value; + DBOptions db_options; + + batch.SingleDelete("A"); + + s = batch.GetFromBatch(db_options, "A", &value); + ASSERT_TRUE(s.IsNotFound()); + s = batch.GetFromBatch(db_options, "B", &value); + ASSERT_TRUE(s.IsNotFound()); + value = PrintContents(&batch, nullptr); + ASSERT_EQ("SINGLE-DEL(A),", value); + + batch.Clear(); + batch.Put("A", "a"); + batch.Put("A", "a2"); + batch.Put("B", "b"); + batch.SingleDelete("A"); + + s = batch.GetFromBatch(db_options, "A", &value); ASSERT_TRUE(s.IsNotFound()); + s = batch.GetFromBatch(db_options, "B", &value); + ASSERT_OK(s); + ASSERT_EQ("b", value); + + value = PrintContents(&batch, nullptr); + ASSERT_EQ("PUT(A):a,PUT(A):a2,SINGLE-DEL(A),PUT(B):b,", value); + + batch.Put("C", "c"); + batch.Put("A", "a3"); + batch.Delete("B"); + batch.SingleDelete("B"); + batch.SingleDelete("C"); + + s = batch.GetFromBatch(db_options, "A", &value); + ASSERT_OK(s); + ASSERT_EQ("a3", value); + s = batch.GetFromBatch(db_options, "B", &value); + ASSERT_TRUE(s.IsNotFound()); + s = batch.GetFromBatch(db_options, "C", &value); + ASSERT_TRUE(s.IsNotFound()); + s = batch.GetFromBatch(db_options, "D", &value); + ASSERT_TRUE(s.IsNotFound()); + + value = PrintContents(&batch, nullptr); + ASSERT_EQ( + "PUT(A):a,PUT(A):a2,SINGLE-DEL(A),PUT(A):a3,PUT(B):b,DEL(B),SINGLE-DEL(B)" + ",PUT(C):c,SINGLE-DEL(C),", + value); + + batch.Put("B", "b4"); + batch.Put("C", "c4"); + batch.Put("D", "d4"); + batch.SingleDelete("D"); + batch.SingleDelete("D"); + batch.Delete("A"); + + s = batch.GetFromBatch(db_options, "A", &value); + ASSERT_TRUE(s.IsNotFound()); + s = batch.GetFromBatch(db_options, "B", &value); + ASSERT_OK(s); + ASSERT_EQ("b4", value); + s = batch.GetFromBatch(db_options, "C", &value); + ASSERT_OK(s); + ASSERT_EQ("c4", value); + s = batch.GetFromBatch(db_options, "D", &value); + ASSERT_TRUE(s.IsNotFound()); + + value = PrintContents(&batch, nullptr); + ASSERT_EQ( + "PUT(A):a,PUT(A):a2,SINGLE-DEL(A),PUT(A):a3,DEL(A),PUT(B):b,DEL(B)," + "SINGLE-DEL(B),PUT(B):b4,PUT(C):c,SINGLE-DEL(C),PUT(C):c4,PUT(D):d4," + "SINGLE-DEL(D),SINGLE-DEL(D),", + value); +} + +TEST_F(WriteBatchWithIndexTest, SingleDeleteDeltaIterTest) { + Status s; + std::string value; + DBOptions db_options; + WriteBatchWithIndex batch(BytewiseComparator(), 20, true /* overwrite_key */); + batch.Put("A", "a"); + batch.Put("A", "a2"); + batch.Put("B", "b"); + batch.SingleDelete("A"); + batch.Delete("B"); + + KVMap map; + value = PrintContents(&batch, &map, nullptr); + ASSERT_EQ("", value); + + map["A"] = "aa"; + map["C"] = "cc"; + map["D"] = "dd"; + + batch.SingleDelete("B"); + batch.SingleDelete("C"); + batch.SingleDelete("Z"); + + value = PrintContents(&batch, &map, nullptr); + ASSERT_EQ("D:dd,", value); + + batch.Put("A", "a3"); + batch.Put("B", "b3"); + batch.SingleDelete("A"); + batch.SingleDelete("A"); + batch.SingleDelete("D"); + batch.SingleDelete("D"); + batch.Delete("D"); + + map["E"] = "ee"; + + value = PrintContents(&batch, &map, nullptr); + ASSERT_EQ("B:b3,E:ee,", value); } } // namespace