Skip to content

Commit 53ad5c8

Browse files
committed
fix
1 parent b67149a commit 53ad5c8

File tree

4 files changed

+26
-20
lines changed

4 files changed

+26
-20
lines changed

db/db_wal_test.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3121,22 +3121,24 @@ TEST_F(DBWALTest, RecoveryFlushSwitchWALOnEmptyMemtable) {
31213121
Destroy(options);
31223122
}
31233123

3124-
TEST_F(DBWALTest, WALWriteErrorNoRecovery) {
3124+
TEST_F(DBWALTest, WALWriteErrorNoAutoRecovery) {
31253125
Options options = CurrentOptions();
31263126
auto fault_fs = std::make_shared<FaultInjectionTestFS>(FileSystem::Default());
31273127
std::unique_ptr<Env> fault_fs_env(NewCompositeEnv(fault_fs));
31283128
options.env = fault_fs_env.get();
3129-
options.manual_wal_flush = true;
3129+
options.atomic_flush = false;
31303130
DestroyAndReopen(options);
3131+
CreateAndReopenWithCF({"pikachu"}, options);
3132+
31313133
fault_fs->SetThreadLocalErrorContext(
31323134
FaultInjectionIOType::kWrite, 7 /* seed*/, 1 /* one_in */,
31333135
true /* retryable */, false /* has_data_loss*/);
31343136
fault_fs->EnableThreadLocalErrorInjection(FaultInjectionIOType::kWrite);
3135-
3136-
ASSERT_OK(Put("k", "v"));
3137-
Status s;
3138-
s = db_->FlushWAL(false);
3137+
Status s = Put("k", "v");
31393138
ASSERT_TRUE(s.IsIOError());
3139+
ASSERT_TRUE(
3140+
s.ToString().find("injected write error failed to write to WAL") !=
3141+
std::string::npos);
31403142
s = dbfull()->TEST_GetBGError();
31413143
ASSERT_EQ(s.severity(), Status::Severity::kFatalError);
31423144
ASSERT_FALSE(dbfull()->TEST_IsRecoveryInProgress());

db/error_handler.cc

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -423,14 +423,17 @@ void ErrorHandler::SetBGError(const Status& bg_status,
423423
reason == BackgroundErrorReason::kMemTable ||
424424
reason == BackgroundErrorReason::kFlush);
425425
}
426-
if (db_options_.manual_wal_flush && wal_related && bg_io_err.IsIOError()) {
427-
// With manual_wal_flush, a WAL write failure can drop buffered WAL writes.
428-
// Memtables and WAL then become inconsistent. A successful memtable flush
429-
// on one CF can cause CFs to be inconsistent upon restart. Before we fix
430-
// the bug in auto recovery from WAL write failures that can flush one CF
431-
// at a time, we set the error severity to fatal to disallow auto recovery.
432-
// TODO: remove parameter `wal_related` once we can automatically recover
433-
// from WAL write failures.
426+
427+
// When `atomic_flush = false` with multiple column families, when
428+
// encountering WAL related IO error, individual CF flushing during auto
429+
// recovery can create data inconsistencies where some column families advance
430+
// past the corruption point while others remain behind, preventing successful
431+
// database restart. Therefore we disable auto recovery by setting a higher
432+
// severity `Status::Severity::kFatalError`.
433+
bool has_multiple_cfs =
434+
db_->versions_->GetColumnFamilySet()->NumberOfColumnFamilies() > 1;
435+
if (!db_options_.atomic_flush && has_multiple_cfs && wal_related &&
436+
bg_io_err.IsIOError()) {
434437
bool auto_recovery = false;
435438
Status bg_err(new_bg_io_err, Status::Severity::kFatalError);
436439
CheckAndSetRecoveryAndBGError(bg_err);

db_stress_tool/db_stress_test_base.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,8 @@ class StressTest {
354354
assert(!error_s.ok());
355355
return error_s.getState() &&
356356
FaultInjectionTestFS::IsInjectedError(error_s) &&
357-
!status_to_io_status(Status(error_s)).GetDataLoss();
357+
!status_to_io_status(Status(error_s)).GetDataLoss() &&
358+
error_s.severity() <= Status::Severity::kHardError;
358359
}
359360

360361
void ProcessStatus(SharedState* shared, std::string msg, const Status& s,

tools/db_crashtest.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -996,7 +996,7 @@ def finalize_and_sanitize(src_params):
996996
if (
997997
dest_params.get("reopen", 0) > 0
998998
or (
999-
dest_params.get("manual_wal_flush_one_in")
999+
dest_params.get("atomic_flush") != 1
10001000
and dest_params.get("column_families") != 1
10011001
)
10021002
or (
@@ -1010,10 +1010,10 @@ def finalize_and_sanitize(src_params):
10101010
# To simplify, we disable any WAL write error injection.
10111011
# TODO(hx235): support WAL write error injection with reopen
10121012
#
1013-
# 2. WAL write failure can drop buffered WAL data. This can cause
1014-
# inconsistency when one CF has a successful flush during auto
1015-
# recovery. Disable the fault injection in this path for now until
1016-
# we have a fix that allows auto recovery.
1013+
# 2. When `atomic_flush = false` with multiple column families, when encountering WAL related IO error,
1014+
# individual CF flushing during auto recovery can create data inconsistencies where some column families advance
1015+
# past the corruption point while others remain behind, preventing successful database restart.
1016+
# Therefore we disable auto recovery and testing for this case in crash test
10171017
#
10181018
# 3. Pessimistic transactions use 2PC, which can't auto-recover from WAL write errors.
10191019
# This is because RocksDB cannot easily discard the corrupted WAL without risking the

0 commit comments

Comments
 (0)