dolysis
/
rocksdb
mirror of https://github.com/facebook/rocksdb.git
3
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fail DeleteRange() early when row_cache is configured (#12710) 

Summary:
https://github.com/facebook/rocksdb/issues/12512 added the sanity check for this incompatible combination. However, it does the check during memtable insertion which can turn the DB into read-only mode. This PR moves the check earlier so that this write failure will not turn the DB into read-only mode and affect other DB operations.

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

Test Plan: * updated unit test `DBRangeDelTest.RowCache` to write to DB after a failed DeleteRange(). The test fails before this PR.

Reviewed By: ajkr

Differential Revision: D57925188

Pulled By: cbi42

fbshipit-source-id: 8bf001bd3fcf05635411ba28bc4a037321942879
This commit is contained in:
Changyu Bi 2024-05-29 15:03:15 -07:00 committed by Facebook GitHub Bot
parent d9316260e4
commit af3be5255a
3 changed files with 8 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
db/db_impl/db_impl_write.cc
View File

@ -268,6 +268,10 @@ Status DBImpl::WriteImpl(const WriteOptions& write_options,
return Status::NotSupported( return Status::NotSupported(
"seq_per_batch currently does not honor post_memtable_callback"); "seq_per_batch currently does not honor post_memtable_callback");
} }
if (my_batch->HasDeleteRange() && immutable_db_options_.row_cache) {
return Status::NotSupported(
"DeleteRange is not compatible with row cache.");
}
// Otherwise IsLatestPersistentState optimization does not make sense // Otherwise IsLatestPersistentState optimization does not make sense
assert(!WriteBatchInternal::IsLatestPersistentState(my_batch) || assert(!WriteBatchInternal::IsLatestPersistentState(my_batch) ||
disable_memtable); disable_memtable);

4
db/db_range_del_test.cc
View File

@ -3820,6 +3820,10 @@ TEST_F(DBRangeDelTest, RowCache) {
ASSERT_OK(wb.DeleteRange(Key(1), Key(5))); ASSERT_OK(wb.DeleteRange(Key(1), Key(5)));
ASSERT_TRUE(db_->Write(WriteOptions(), &wb).IsNotSupported()); ASSERT_TRUE(db_->Write(WriteOptions(), &wb).IsNotSupported());
ASSERT_EQ(Get(Key(3)), "val"); ASSERT_EQ(Get(Key(3)), "val");
// By default, memtable insertion failure will turn the DB to read-only mode.
// The check for delete range should happen before that to fail early
// and should not turn db into read-only mdoe.
ASSERT_OK(Put(Key(5), "foo"));
} }
} // namespace ROCKSDB_NAMESPACE } // namespace ROCKSDB_NAMESPACE

5
db/write_batch.cc
View File

@ -2550,11 +2550,6 @@ class MemTableInserter : public WriteBatch::Handler {
assert(ret_status.ok()); assert(ret_status.ok());
if (db_ != nullptr) { if (db_ != nullptr) {
if (db_->immutable_db_options().row_cache) {
ret_status.PermitUncheckedError();
return Status::NotSupported(
"DeleteRange is not compatible with row cache.");
}
auto cf_handle = cf_mems_->GetColumnFamilyHandle(); auto cf_handle = cf_mems_->GetColumnFamilyHandle();
if (cf_handle == nullptr) { if (cf_handle == nullptr) {
cf_handle = db_->DefaultColumnFamily(); cf_handle = db_->DefaultColumnFamily();