From baf22b4ee6eb27d7833dfbf5c59a32e15eef31a4 Mon Sep 17 00:00:00 2001 From: Merlin Mao Date: Fri, 20 Aug 2021 15:32:55 -0700 Subject: [PATCH] Add `IteratorTraceExecutionResult` for iterator related trace records. (#8687) Summary: - Allow to get `Valid()`, `status()`, `key()` and `value()` of an iterator from `IteratorTraceExecutionResult`. - Move lower bound and upper bound from `IteratorSeekQueryTraceRecord` to `IteratorQueryTraceRecord`. Added test in `DBTest2.TraceAndReplay`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8687 Reviewed By: zhichao-cao Differential Revision: D30457630 Pulled By: autopear fbshipit-source-id: be433099a25895b3aa6f0c00f95ad7b1d7489c1d --- db/db_test2.cc | 70 +++++++++++++++++------- include/rocksdb/trace_record.h | 29 ++++++---- include/rocksdb/trace_record_result.h | 79 +++++++++++++++------------ trace_replay/trace_record.cc | 40 +++++++++----- trace_replay/trace_record_handler.cc | 17 ++++-- trace_replay/trace_record_result.cc | 40 ++++++++++++++ 6 files changed, 188 insertions(+), 87 deletions(-) diff --git a/db/db_test2.cc b/db/db_test2.cc index b7e626df6..f3b8dfbfd 100644 --- a/db/db_test2.cc +++ b/db/db_test2.cc @@ -4256,13 +4256,6 @@ class TraceExecutionResultHandler : public TraceRecordResult::Handler { writes_++; break; } - case kTraceIteratorSeek: - case kTraceIteratorSeekForPrev: { - total_latency_ += result.GetLatency(); - cnt_++; - seeks_++; - break; - } default: return Status::Corruption("Type mismatch."); } @@ -4309,6 +4302,25 @@ class TraceExecutionResultHandler : public TraceRecordResult::Handler { return Status::OK(); } + virtual Status Handle(const IteratorTraceExecutionResult& result) override { + if (result.GetStartTimestamp() > result.GetEndTimestamp()) { + return Status::InvalidArgument("Invalid timestamps."); + } + result.GetStatus().PermitUncheckedError(); + switch (result.GetTraceType()) { + case kTraceIteratorSeek: + case kTraceIteratorSeekForPrev: { + total_latency_ += result.GetLatency(); + cnt_++; + seeks_++; + break; + } + default: + return Status::Corruption("Type mismatch."); + } + return Status::OK(); + } + void Reset() { total_latency_ = 0; cnt_ = 0; @@ -4644,23 +4656,39 @@ TEST_F(DBTest2, TraceAndManualReplay) { continue; } if (s.ok()) { - if (record->GetTraceType() == kTraceIteratorSeek || - record->GetTraceType() == kTraceIteratorSeekForPrev) { - IteratorSeekQueryTraceRecord* iter_r = - dynamic_cast(record.get()); - // Check if lower/upper bounds are correctly saved and decoded. - lower_bound = iter_r->GetLowerBound(); - if (!lower_bound.empty()) { - ASSERT_EQ(lower_bound.ToString(), "iter-1"); - } - upper_bound = iter_r->GetUpperBound(); - if (!upper_bound.empty()) { - ASSERT_EQ(upper_bound.ToString(), "iter-3"); - } - } ASSERT_OK(replayer->Execute(record, &result)); if (result != nullptr) { ASSERT_OK(result->Accept(&res_handler)); + if (record->GetTraceType() == kTraceIteratorSeek || + record->GetTraceType() == kTraceIteratorSeekForPrev) { + IteratorSeekQueryTraceRecord* iter_rec = + dynamic_cast(record.get()); + IteratorTraceExecutionResult* iter_res = + dynamic_cast(result.get()); + // Check if lower/upper bounds are correctly saved and decoded. + std::string lower_str = iter_rec->GetLowerBound().ToString(); + std::string upper_str = iter_rec->GetUpperBound().ToString(); + std::string iter_key = iter_res->GetKey().ToString(); + std::string iter_value = iter_res->GetValue().ToString(); + if (!lower_str.empty() && !upper_str.empty()) { + ASSERT_EQ(lower_str, "iter-1"); + ASSERT_EQ(upper_str, "iter-3"); + if (iter_res->GetValid()) { + // If iterator is valid, then lower_bound <= key < upper_bound. + ASSERT_GE(iter_key, lower_str); + ASSERT_LT(iter_key, upper_str); + } else { + // If iterator is invalid, then + // key < lower_bound or key >= upper_bound. + ASSERT_TRUE(iter_key < lower_str || iter_key >= upper_str); + } + } + // If iterator is invalid, the key and value should be empty. + if (!iter_res->GetValid()) { + ASSERT_TRUE(iter_key.empty()); + ASSERT_TRUE(iter_value.empty()); + } + } result.reset(); } } diff --git a/include/rocksdb/trace_record.h b/include/rocksdb/trace_record.h index defa89934..3d0c0acbf 100644 --- a/include/rocksdb/trace_record.h +++ b/include/rocksdb/trace_record.h @@ -65,19 +65,15 @@ class TraceRecord { public: virtual ~Handler() = default; - // Handle WriteQueryTraceRecord virtual Status Handle(const WriteQueryTraceRecord& record, std::unique_ptr* result) = 0; - // Handle GetQueryTraceRecord virtual Status Handle(const GetQueryTraceRecord& record, std::unique_ptr* result) = 0; - // Handle IteratorSeekQueryTraceRecord virtual Status Handle(const IteratorSeekQueryTraceRecord& record, std::unique_ptr* result) = 0; - // Handle MultiGetQueryTraceRecord virtual Status Handle(const MultiGetQueryTraceRecord& record, std::unique_ptr* result) = 0; }; @@ -152,6 +148,23 @@ class GetQueryTraceRecord : public QueryTraceRecord { class IteratorQueryTraceRecord : public QueryTraceRecord { public: explicit IteratorQueryTraceRecord(uint64_t timestamp); + + IteratorQueryTraceRecord(PinnableSlice&& lower_bound, + PinnableSlice&& upper_bound, uint64_t timestamp); + + IteratorQueryTraceRecord(const std::string& lower_bound, + const std::string& upper_bound, uint64_t timestamp); + + virtual ~IteratorQueryTraceRecord() override; + + // Get the iterator's lower/upper bound. They may be used in ReadOptions to + // create an Iterator instance. + virtual Slice GetLowerBound() const; + virtual Slice GetUpperBound() const; + + private: + PinnableSlice lower_; + PinnableSlice upper_; }; // Trace record for Iterator::Seek() and Iterator::SeekForPrev() operation. @@ -193,12 +206,6 @@ class IteratorSeekQueryTraceRecord : public IteratorQueryTraceRecord { // Key to seek to. virtual Slice GetKey() const; - // Iterate lower bound. - virtual Slice GetLowerBound() const; - - // Iterate upper bound. - virtual Slice GetUpperBound() const; - Status Accept(Handler* handler, std::unique_ptr* result) override; @@ -206,8 +213,6 @@ class IteratorSeekQueryTraceRecord : public IteratorQueryTraceRecord { SeekType type_; uint32_t cf_id_; PinnableSlice key_; - PinnableSlice lower_; - PinnableSlice upper_; }; // Trace record for DB::MultiGet() operation. diff --git a/include/rocksdb/trace_record_result.h b/include/rocksdb/trace_record_result.h index 7924c6de3..0cd0004a6 100644 --- a/include/rocksdb/trace_record_result.h +++ b/include/rocksdb/trace_record_result.h @@ -9,11 +9,13 @@ #include #include "rocksdb/rocksdb_namespace.h" +#include "rocksdb/slice.h" #include "rocksdb/status.h" #include "rocksdb/trace_record.h" namespace ROCKSDB_NAMESPACE { +class IteratorTraceExecutionResult; class MultiValuesTraceExecutionResult; class SingleValueTraceExecutionResult; class StatusOnlyTraceExecutionResult; @@ -34,42 +36,14 @@ class TraceRecordResult { public: virtual ~Handler() = default; - // Handle StatusOnlyTraceExecutionResult virtual Status Handle(const StatusOnlyTraceExecutionResult& result) = 0; - // Handle SingleValueTraceExecutionResult virtual Status Handle(const SingleValueTraceExecutionResult& result) = 0; - // Handle MultiValuesTraceExecutionResult virtual Status Handle(const MultiValuesTraceExecutionResult& result) = 0; - }; - /* - * Example handler to just print the trace record execution results. - * - * class ResultPrintHandler : public TraceRecordResult::Handler { - * public: - * ResultPrintHandler(); - * ~ResultPrintHandler() override {} - * - * Status Handle(const StatusOnlyTraceExecutionResult& result) override { - * std::cout << "Status: " << result.GetStatus().ToString() << std::endl; - * } - * - * Status Handle(const SingleValueTraceExecutionResult& result) override { - * std::cout << "Status: " << result.GetStatus().ToString() - * << ", value: " << result.GetValue() << std::endl; - * } - * - * Status Handle(const MultiValuesTraceExecutionResult& result) override { - * size_t size = result.GetMultiStatus().size(); - * for (size_t i = 0; i < size; i++) { - * std::cout << "Status: " << result.GetMultiStatus()[i].ToString() - * << ", value: " << result.GetValues()[i] << std::endl; - * } - * } - * }; - * */ + virtual Status Handle(const IteratorTraceExecutionResult& result) = 0; + }; // Accept the handler. virtual Status Accept(Handler* handler) = 0; @@ -106,8 +80,7 @@ class TraceExecutionResult : public TraceRecordResult { }; // Result for operations that only return a single Status. -// Example operations: DB::Write(), Iterator::Seek() and -// Iterator::SeekForPrev(). +// Example operation: DB::Write() class StatusOnlyTraceExecutionResult : public TraceExecutionResult { public: StatusOnlyTraceExecutionResult(Status status, uint64_t start_timestamp, @@ -138,7 +111,7 @@ class SingleValueTraceExecutionResult : public TraceExecutionResult { virtual ~SingleValueTraceExecutionResult() override; - // Return status of DB::Get(), etc. + // Return status of DB::Get(). virtual const Status& GetStatus() const; // Value for the searched key. @@ -151,7 +124,7 @@ class SingleValueTraceExecutionResult : public TraceExecutionResult { std::string value_; }; -// Result for operations that return multiple Status(es) and values. +// Result for operations that return multiple Status(es) and values as vectors. // Example operation: DB::MultiGet() class MultiValuesTraceExecutionResult : public TraceExecutionResult { public: @@ -162,7 +135,7 @@ class MultiValuesTraceExecutionResult : public TraceExecutionResult { virtual ~MultiValuesTraceExecutionResult() override; - // Returned Status(es) of DB::MultiGet(), etc. + // Returned Status(es) of DB::MultiGet(). virtual const std::vector& GetMultiStatus() const; // Returned values for the searched keys. @@ -175,4 +148,40 @@ class MultiValuesTraceExecutionResult : public TraceExecutionResult { std::vector values_; }; +// Result for Iterator operations. +// Example operations: Iterator::Seek(), Iterator::SeekForPrev() +class IteratorTraceExecutionResult : public TraceExecutionResult { + public: + IteratorTraceExecutionResult(bool valid, Status status, PinnableSlice&& key, + PinnableSlice&& value, uint64_t start_timestamp, + uint64_t end_timestamp, TraceType trace_type); + + IteratorTraceExecutionResult(bool valid, Status status, + const std::string& key, const std::string& value, + uint64_t start_timestamp, uint64_t end_timestamp, + TraceType trace_type); + + virtual ~IteratorTraceExecutionResult() override; + + // Return if the Iterator is valid. + virtual bool GetValid() const; + + // Return the status of the Iterator. + virtual const Status& GetStatus() const; + + // Key of the current iterating entry, empty if GetValid() is false. + virtual Slice GetKey() const; + + // Value of the current iterating entry, empty if GetValid() is false. + virtual Slice GetValue() const; + + virtual Status Accept(Handler* handler) override; + + private: + bool valid_; + Status status_; + PinnableSlice key_; + PinnableSlice value_; +}; + } // namespace ROCKSDB_NAMESPACE diff --git a/trace_replay/trace_record.cc b/trace_replay/trace_record.cc index b377a18d2..21df0275d 100644 --- a/trace_replay/trace_record.cc +++ b/trace_replay/trace_record.cc @@ -82,6 +82,27 @@ Status GetQueryTraceRecord::Accept(Handler* handler, IteratorQueryTraceRecord::IteratorQueryTraceRecord(uint64_t timestamp) : QueryTraceRecord(timestamp) {} +IteratorQueryTraceRecord::IteratorQueryTraceRecord(PinnableSlice&& lower_bound, + PinnableSlice&& upper_bound, + uint64_t timestamp) + : QueryTraceRecord(timestamp), + lower_(std::move(lower_bound)), + upper_(std::move(upper_bound)) {} + +IteratorQueryTraceRecord::IteratorQueryTraceRecord( + const std::string& lower_bound, const std::string& upper_bound, + uint64_t timestamp) + : QueryTraceRecord(timestamp) { + lower_.PinSelf(lower_bound); + upper_.PinSelf(upper_bound); +} + +IteratorQueryTraceRecord::~IteratorQueryTraceRecord() {} + +Slice IteratorQueryTraceRecord::GetLowerBound() const { return Slice(lower_); } + +Slice IteratorQueryTraceRecord::GetUpperBound() const { return Slice(upper_); } + // IteratorSeekQueryTraceRecord IteratorSeekQueryTraceRecord::IteratorSeekQueryTraceRecord( SeekType seek_type, uint32_t column_family_id, PinnableSlice&& key, @@ -104,23 +125,20 @@ IteratorSeekQueryTraceRecord::IteratorSeekQueryTraceRecord( SeekType seek_type, uint32_t column_family_id, PinnableSlice&& key, PinnableSlice&& lower_bound, PinnableSlice&& upper_bound, uint64_t timestamp) - : IteratorQueryTraceRecord(timestamp), + : IteratorQueryTraceRecord(std::move(lower_bound), std::move(upper_bound), + timestamp), type_(seek_type), cf_id_(column_family_id), - key_(std::move(key)), - lower_(std::move(lower_bound)), - upper_(std::move(upper_bound)) {} + key_(std::move(key)) {} IteratorSeekQueryTraceRecord::IteratorSeekQueryTraceRecord( SeekType seek_type, uint32_t column_family_id, const std::string& key, const std::string& lower_bound, const std::string& upper_bound, uint64_t timestamp) - : IteratorQueryTraceRecord(timestamp), + : IteratorQueryTraceRecord(lower_bound, upper_bound, timestamp), type_(seek_type), cf_id_(column_family_id) { key_.PinSelf(key); - lower_.PinSelf(lower_bound); - upper_.PinSelf(upper_bound); } IteratorSeekQueryTraceRecord::~IteratorSeekQueryTraceRecord() { key_.clear(); } @@ -140,14 +158,6 @@ uint32_t IteratorSeekQueryTraceRecord::GetColumnFamilyID() const { Slice IteratorSeekQueryTraceRecord::GetKey() const { return Slice(key_); } -Slice IteratorSeekQueryTraceRecord::GetLowerBound() const { - return Slice(lower_); -} - -Slice IteratorSeekQueryTraceRecord::GetUpperBound() const { - return Slice(upper_); -} - Status IteratorSeekQueryTraceRecord::Accept( Handler* handler, std::unique_ptr* result) { assert(handler != nullptr); diff --git a/trace_replay/trace_record_handler.cc b/trace_replay/trace_record_handler.cc index e0b5fb202..8f5371622 100644 --- a/trace_replay/trace_record_handler.cc +++ b/trace_replay/trace_record_handler.cc @@ -120,12 +120,21 @@ Status TraceExecutionHandler::Handle( uint64_t end = clock_->NowMicros(); Status s = single_iter->status(); - delete single_iter; - if (s.ok() && result != nullptr) { - result->reset(new StatusOnlyTraceExecutionResult(s, start, end, - record.GetTraceType())); + if (single_iter->Valid()) { + PinnableSlice ps_key; + ps_key.PinSelf(single_iter->key()); + PinnableSlice ps_value; + ps_value.PinSelf(single_iter->value()); + result->reset(new IteratorTraceExecutionResult( + true, s, std::move(ps_key), std::move(ps_value), start, end, + record.GetTraceType())); + } else { + result->reset(new IteratorTraceExecutionResult( + false, s, "", "", start, end, record.GetTraceType())); + } } + delete single_iter; return s; } diff --git a/trace_replay/trace_record_result.cc b/trace_replay/trace_record_result.cc index b22b57e43..9c0cb43ad 100644 --- a/trace_replay/trace_record_result.cc +++ b/trace_replay/trace_record_result.cc @@ -103,4 +103,44 @@ Status MultiValuesTraceExecutionResult::Accept(Handler* handler) { return handler->Handle(*this); } +// IteratorTraceExecutionResult +IteratorTraceExecutionResult::IteratorTraceExecutionResult( + bool valid, Status status, PinnableSlice&& key, PinnableSlice&& value, + uint64_t start_timestamp, uint64_t end_timestamp, TraceType trace_type) + : TraceExecutionResult(start_timestamp, end_timestamp, trace_type), + valid_(valid), + status_(std::move(status)), + key_(std::move(key)), + value_(std::move(value)) {} + +IteratorTraceExecutionResult::IteratorTraceExecutionResult( + bool valid, Status status, const std::string& key, const std::string& value, + uint64_t start_timestamp, uint64_t end_timestamp, TraceType trace_type) + : TraceExecutionResult(start_timestamp, end_timestamp, trace_type), + valid_(valid), + status_(std::move(status)) { + key_.PinSelf(key); + value_.PinSelf(value); +} + +IteratorTraceExecutionResult::~IteratorTraceExecutionResult() { + key_.clear(); + value_.clear(); +} + +bool IteratorTraceExecutionResult::GetValid() const { return valid_; } + +const Status& IteratorTraceExecutionResult::GetStatus() const { + return status_; +} + +Slice IteratorTraceExecutionResult::GetKey() const { return Slice(key_); } + +Slice IteratorTraceExecutionResult::GetValue() const { return Slice(value_); } + +Status IteratorTraceExecutionResult::Accept(Handler* handler) { + assert(handler != nullptr); + return handler->Handle(*this); +} + } // namespace ROCKSDB_NAMESPACE -- GitLab