From 74a652a45ff63350640245ba7994bebc748bb0d4 Mon Sep 17 00:00:00 2001 From: Merlin Mao Date: Thu, 12 Aug 2021 09:21:40 -0700 Subject: [PATCH] Code cleanup for trace replayer (#8652) Summary: - Remove extra `;` in trace_record.h - Remove some unnecessary `assert` in trace_record_handler.cc - Initialize `env_` after` exec_handler_` in `ReplayerImpl` to let db be asserted in creating the handler before getting `db->GetEnv()`. - Update history to include the new `TraceReader::Reset()` Pull Request resolved: https://github.com/facebook/rocksdb/pull/8652 Reviewed By: ajkr Differential Revision: D30276872 Pulled By: autopear fbshipit-source-id: 476ee162e0f241490c6209307448343a5b326b37 --- HISTORY.md | 2 +- include/rocksdb/trace_record.h | 6 +++--- trace_replay/trace_record_handler.cc | 3 --- utilities/trace/replayer_impl.cc | 5 +++-- utilities/trace/replayer_impl.h | 10 +++++----- 5 files changed, 12 insertions(+), 14 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index ea98e1a94..e68d6dc1f 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -18,7 +18,7 @@ * Fast forward option in Trace replay changed to double type to allow replaying at a lower speed, by settings the value between 0 and 1. This option can be set via `ReplayOptions` in `Replayer::Replay()`, or via `--trace_replay_fast_forward` in db_bench. ## Public API change -* Added APIs to decode and replay trace file via Replayer class. Added `DB::NewDefaultReplayer()` to create a default Replayer instance. Created trace_record.h and utilities/replayer.h files to access decoded Trace records and replay them. +* Added APIs to decode and replay trace file via Replayer class. Added `DB::NewDefaultReplayer()` to create a default Replayer instance. Added `TraceReader::Reset()` to restart reading a trace file. Created trace_record.h and utilities/replayer.h files to access decoded Trace records and replay them. ### Performance Improvements * Try to avoid updating DBOptions if `SetDBOptions()` does not change any option value. diff --git a/include/rocksdb/trace_record.h b/include/rocksdb/trace_record.h index add235eaa..f715a4396 100644 --- a/include/rocksdb/trace_record.h +++ b/include/rocksdb/trace_record.h @@ -95,7 +95,7 @@ class WriteQueryTraceRecord : public QueryTraceRecord { virtual ~WriteQueryTraceRecord() override; - TraceType GetTraceType() const override { return kTraceWrite; }; + TraceType GetTraceType() const override { return kTraceWrite; } virtual Slice GetWriteBatchRep() const; @@ -116,7 +116,7 @@ class GetQueryTraceRecord : public QueryTraceRecord { virtual ~GetQueryTraceRecord() override; - TraceType GetTraceType() const override { return kTraceGet; }; + TraceType GetTraceType() const override { return kTraceGet; } virtual uint32_t GetColumnFamilyID() const; @@ -187,7 +187,7 @@ class MultiGetQueryTraceRecord : public QueryTraceRecord { virtual ~MultiGetQueryTraceRecord() override; - TraceType GetTraceType() const override { return kTraceMultiGet; }; + TraceType GetTraceType() const override { return kTraceMultiGet; } virtual std::vector GetColumnFamilyIDs() const; diff --git a/trace_replay/trace_record_handler.cc b/trace_replay/trace_record_handler.cc index 4e8a40b94..3651d0fe2 100644 --- a/trace_replay/trace_record_handler.cc +++ b/trace_replay/trace_record_handler.cc @@ -38,7 +38,6 @@ Status TraceExecutionHandler::Handle(const GetQueryTraceRecord& record) { if (it == cf_map_.end()) { return Status::Corruption("Invalid Column Family ID."); } - assert(it->second != nullptr); std::string value; Status s = db_->Get(read_opts_, it->second, record.GetKey(), &value); @@ -53,7 +52,6 @@ Status TraceExecutionHandler::Handle( if (it == cf_map_.end()) { return Status::Corruption("Invalid Column Family ID."); } - assert(it->second != nullptr); Iterator* single_iter = db_->NewIterator(read_opts_, it->second); @@ -80,7 +78,6 @@ Status TraceExecutionHandler::Handle(const MultiGetQueryTraceRecord& record) { if (it == cf_map_.end()) { return Status::Corruption("Invalid Column Family ID."); } - assert(it->second != nullptr); handles.push_back(it->second); } diff --git a/utilities/trace/replayer_impl.cc b/utilities/trace/replayer_impl.cc index 2789de5cd..2462db468 100644 --- a/utilities/trace/replayer_impl.cc +++ b/utilities/trace/replayer_impl.cc @@ -25,12 +25,13 @@ ReplayerImpl::ReplayerImpl(DB* db, const std::vector& handles, std::unique_ptr&& reader) : Replayer(), - env_(db->GetEnv()), trace_reader_(std::move(reader)), prepared_(false), trace_end_(false), header_ts_(0), - exec_handler_(TraceRecord::NewExecutionHandler(db, handles)) {} + exec_handler_(TraceRecord::NewExecutionHandler(db, handles)), + env_(db->GetEnv()), + trace_file_version_(-1) {} ReplayerImpl::~ReplayerImpl() { exec_handler_.reset(); diff --git a/utilities/trace/replayer_impl.h b/utilities/trace/replayer_impl.h index b796d2226..6cf4455e9 100644 --- a/utilities/trace/replayer_impl.h +++ b/utilities/trace/replayer_impl.h @@ -62,17 +62,17 @@ class ReplayerImpl : public Replayer { // Generic function to execute a Trace in a thread pool. static void BackgroundWork(void* arg); - Env* env_; std::unique_ptr trace_reader_; - // When reading the trace header, the trace file version can be parsed. - // Replayer will use different decode method to get the trace content based - // on different trace file version. - int trace_file_version_; std::mutex mutex_; std::atomic prepared_; std::atomic trace_end_; uint64_t header_ts_; std::unique_ptr exec_handler_; + Env* env_; + // When reading the trace header, the trace file version can be parsed. + // Replayer will use different decode method to get the trace content based + // on different trace file version. + int trace_file_version_; }; // The passin arg of MultiThreadRepkay for each trace record.