diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc index e46a2cf2f..4bd1d41f3 100644 --- a/db/db_compaction_test.cc +++ b/db/db_compaction_test.cc @@ -12,7 +12,9 @@ #include "port/port.h" #include "rocksdb/experimental.h" #include "rocksdb/utilities/convenience.h" +#include "util/fault_injection_test_env.h" #include "util/sync_point.h" + namespace rocksdb { // SYNC_POINT is not supported in released Windows mode. @@ -4029,6 +4031,46 @@ TEST_F(DBCompactionTest, PartialManualCompaction) { dbfull()->CompactRange(cro, nullptr, nullptr); } +TEST_F(DBCompactionTest, ManualCompactionFailsInReadOnlyMode) { + // Regression test for bug where manual compaction hangs forever when the DB + // is in read-only mode. Verify it now at least returns, despite failing. + const int kNumL0Files = 4; + std::unique_ptr mock_env( + new FaultInjectionTestEnv(Env::Default())); + Options opts = CurrentOptions(); + opts.disable_auto_compactions = true; + opts.env = mock_env.get(); + DestroyAndReopen(opts); + + Random rnd(301); + for (int i = 0; i < kNumL0Files; ++i) { + // Make sure files are overlapping in key-range to prevent trivial move. + Put("key1", RandomString(&rnd, 1024)); + Put("key2", RandomString(&rnd, 1024)); + Flush(); + } + ASSERT_EQ(kNumL0Files, NumTableFilesAtLevel(0)); + + // Enter read-only mode by failing a write. + mock_env->SetFilesystemActive(false); + // Make sure this is outside `CompactRange`'s range so that it doesn't fail + // early trying to flush memtable. + ASSERT_NOK(Put("key3", RandomString(&rnd, 1024))); + + // In the bug scenario, the first manual compaction would fail and forget to + // unregister itself, causing the second one to hang forever due to conflict + // with a non-running compaction. + CompactRangeOptions cro; + cro.exclusive_manual_compaction = false; + Slice begin_key("key1"); + Slice end_key("key2"); + ASSERT_NOK(dbfull()->CompactRange(cro, &begin_key, &end_key)); + ASSERT_NOK(dbfull()->CompactRange(cro, &begin_key, &end_key)); + + // Close before mock_env destruct. + Close(); +} + #endif // !defined(ROCKSDB_LITE) } // namespace rocksdb diff --git a/db/db_impl_compaction_flush.cc b/db/db_impl_compaction_flush.cc index 49a23b48e..92b8c19e3 100644 --- a/db/db_impl_compaction_flush.cc +++ b/db/db_impl_compaction_flush.cc @@ -2170,6 +2170,10 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, manual_compaction->in_progress = false; manual_compaction = nullptr; } + if (c) { + c->ReleaseCompactionFiles(status); + c.reset(); + } return status; }