Skip to content

Commit

Permalink
storage/engine: fix DeleteRange+ApplyBatchRepr
Browse files Browse the repository at this point in the history
Added support for DeleteRange to DBBatchInserter. This missing support
was causing test failures and various data corruption when enabling
COCKROACH_ENABLE_FAST_CLEAR_RANGE after cockroachdb#14138 started using
ApplyBatchRepr on batches containing DeleteRange operations.

See cockroachdb#14391
  • Loading branch information
petermattis committed Mar 28, 2017
1 parent 0f0677b commit 6b62283
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
12 changes: 10 additions & 2 deletions pkg/storage/engine/db.cc
Original file line number Diff line number Diff line change
Expand Up @@ -509,8 +509,16 @@ class DBBatchInserter : public rocksdb::WriteBatch::Handler {
virtual void Merge(const rocksdb::Slice& key, const rocksdb::Slice& value) {
batch_->Merge(key, value);
}
// NB: we don't support DeleteRangeCF yet which causes us to pick up
// the default implementation that returns an error.
virtual rocksdb::Status DeleteRangeCF(
uint32_t column_family_id,
const rocksdb::Slice& begin_key,
const rocksdb::Slice& end_key) {
if (column_family_id == 0) {
batch_->DeleteRange(begin_key, end_key);
return rocksdb::Status::OK();
}
return rocksdb::Status::InvalidArgument("DeleteRangeCF not implemented");
}

private:
rocksdb::WriteBatchBase* const batch_;
Expand Down
17 changes: 17 additions & 0 deletions pkg/storage/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,23 @@ func TestEngineDeleteRange(t *testing.T) {
})
}

func TestEngineDeleteRangeBatch(t *testing.T) {
defer leaktest.AfterTest(t)()
testEngineDeleteRange(t, func(engine Engine, start, end MVCCKey) error {
batch := engine.NewWriteOnlyBatch()
defer batch.Close()
if err := batch.ClearRange(start, end); err != nil {
return err
}
batch2 := engine.NewWriteOnlyBatch()
defer batch2.Close()
if err := batch2.ApplyBatchRepr(batch.Repr(), false); err != nil {
return err
}
return batch2.Commit(false)
})
}

func TestEngineDeleteIterRange(t *testing.T) {
defer leaktest.AfterTest(t)()
testEngineDeleteRange(t, func(engine Engine, start, end MVCCKey) error {
Expand Down

0 comments on commit 6b62283

Please sign in to comment.