Skip to content

Commit

Permalink
Skip trace write after write failure
Browse files Browse the repository at this point in the history
  • Loading branch information
hx235 committed Oct 21, 2023
1 parent 543191f commit 491a4ad
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
40 changes: 40 additions & 0 deletions tools/trace_analyzer_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ int main() {
#include "test_util/testutil.h"
#include "tools/trace_analyzer_tool.h"
#include "trace_replay/trace_replay.h"
#include "utilities/fault_injection_env.h"
#include "utilities/trace/file_trace_reader_writer.h"

namespace ROCKSDB_NAMESPACE {

Expand Down Expand Up @@ -785,6 +787,44 @@ TEST_F(TraceAnalyzerTest, Iterator) {
*/
}

TEST_F(TraceAnalyzerTest, ExistsPreviousTraceWriteError) {
const std::string key(1000000, 'k');
const std::string value(1000000, 'v');

DB* db_ = nullptr;
Options options;
options.create_if_missing = true;
std::unique_ptr<FaultInjectionTestEnv> fault_env(
new FaultInjectionTestEnv(env_));
const std::string trace_path =
test_path_ + "/previous_trace_write_error_trace";
std::unique_ptr<TraceWriter> trace_writer;
ASSERT_OK(NewFileTraceWriter(fault_env.get(), env_options_, trace_path,
&trace_writer));

ASSERT_OK(DB::Open(options, dbname_, &db_));
ASSERT_OK(db_->StartTrace(TraceOptions(), std::move(trace_writer)));

// Inject write error
fault_env->SetFilesystemActive(false);
ASSERT_OK(db_->Put(WriteOptions(), key, value));
fault_env->SetFilesystemActive(true);

// Without proper handling of previous trace write error,
// this trace write will continue and trigger assertion failure
// on writing to the trace file that has seen error.
ASSERT_OK(db_->Put(WriteOptions(), key, value));

// Verify `EndTrace()` returns the previous write trace error if any
Status s = db_->EndTrace();
ASSERT_TRUE(s.IsIOError());
ASSERT_TRUE(s.ToString().find("Skip this trace writing") !=
std::string::npos);

delete db_;
ASSERT_OK(DestroyDB(dbname_, options));
}

// Test analyzing of multiget
TEST_F(TraceAnalyzerTest, MultiGet) {
std::string trace_path = test_path_ + "/trace";
Expand Down
1 change: 1 addition & 0 deletions unreleased_history/behavior_changes/skip_trace_write.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
If an error is hit when writing to a trace file after `DB::StartTrace()`, the subsequent trace writes are skipped to avoid writing to a file that has previously seen error `DB::EndTrace()` will then signal users tht the tracing has failed.
4 changes: 4 additions & 0 deletions utilities/trace/file_trace_reader_writer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ Status FileTraceWriter::Close() {
}

Status FileTraceWriter::Write(const Slice& data) {
if (file_writer_->seen_error()) {
return IOStatus::IOError(
"Error has happened to a previous write. Skip this trace writing.");
}
return file_writer_->Append(data);
}

Expand Down

0 comments on commit 491a4ad

Please sign in to comment.