Skip to content

Commit ebaeb03

Browse files
pdillingerfacebook-github-bot
authored andcommitted
Write failure can be permanently fatal and break WriteBatch atomicity (#13428)
Summary: This adds a test that attempts DeleteRange() with PlainTable (not supported) and shows that it not only puts the DB in failed write mode, it (a) breaks WriteBatch atomicity for readers, because they can see just part of a failed WriteBatch, and (b) makes the DB not recoverable (without manual intervention) if using WAL. Note: WriteBatch atomicity is not clearly documented but indicated at the top of write_batch.h and the wiki page for Transactions, even without Transactions. Pull Request resolved: #13428 Test Plan: this is the test Reviewed By: anand1976 Differential Revision: D70332226 Pulled By: pdillinger fbshipit-source-id: 67bc4de68833a80578e48baa9d3a4f23f1600f3c
1 parent d1f383b commit ebaeb03

File tree

1 file changed

+44
-0
lines changed

1 file changed

+44
-0
lines changed

db/plain_table_db_test.cc

+44
Original file line numberDiff line numberDiff line change
@@ -1338,6 +1338,50 @@ TEST_P(PlainTableDBTest, AdaptiveTable) {
13381338

13391339
INSTANTIATE_TEST_CASE_P(PlainTableDBTest, PlainTableDBTest, ::testing::Bool());
13401340

1341+
TEST_P(PlainTableDBTest, DeleteRangeNotSupported) {
1342+
// XXX: After attempting DeleteRange with PlainTable, Writes will permanently
1343+
// fail. Even if re-opening the DB, if WAL is used, the WAL is not recoverable
1344+
// (without manual intervention). Furthermore, a partial write batch can
1345+
// be exposed to readers, breaking WriteBatch atomicity.
1346+
for (bool use_write_batch : {/*false, */ true}) {
1347+
DestroyAndReopen();
1348+
1349+
ASSERT_OK(Put("a0001111", "1"));
1350+
ASSERT_OK(Put("b0001111", "2"));
1351+
ASSERT_OK(Put("c0001111", "3"));
1352+
if (use_write_batch) {
1353+
WriteBatch wb;
1354+
ASSERT_OK(wb.Put("d0001111", "4"));
1355+
ASSERT_OK(wb.DeleteRange("a", "b"));
1356+
ASSERT_OK(wb.Put("e0001111", "5"));
1357+
ASSERT_EQ(dbfull()->Write({}, &wb).code(), Status::Code::kNotSupported);
1358+
} else {
1359+
ASSERT_EQ(dbfull()->DeleteRange({}, "az", "bz").code(),
1360+
Status::Code::kNotSupported);
1361+
}
1362+
ASSERT_EQ(Get("a0001111"), "1");
1363+
ASSERT_EQ(Get("b0001111"), "2");
1364+
ASSERT_EQ(Get("c0001111"), "3");
1365+
if (use_write_batch) {
1366+
// XXX: broken WriteBatch atomicity
1367+
ASSERT_EQ(Get("d0001111"), "4");
1368+
} else {
1369+
ASSERT_EQ(Get("d0001111"), "NOT_FOUND");
1370+
}
1371+
ASSERT_EQ(Get("e0001111"), "NOT_FOUND");
1372+
1373+
ASSERT_EQ(Put("e0001111", "5").code(), Status::Code::kNotSupported);
1374+
ASSERT_EQ(Get("e0001111"), "NOT_FOUND");
1375+
1376+
// Even trying to flush
1377+
ASSERT_EQ(dbfull()->TEST_FlushMemTable().code(),
1378+
Status::Code::kNotSupported);
1379+
1380+
// XXX: WAL is not recoverable
1381+
ASSERT_EQ(TryReopen().code(), Status::Code::kNotSupported);
1382+
}
1383+
}
1384+
13411385
} // namespace ROCKSDB_NAMESPACE
13421386

13431387
int main(int argc, char** argv) {

0 commit comments

Comments
 (0)