From d795a730be77c16e3f5e81d3e5fd4e7b75945cdc Mon Sep 17 00:00:00 2001 From: anand76 Date: Tue, 22 Feb 2022 10:34:57 -0800 Subject: [PATCH] Combine data members of IOStatus with Status (#9549) Summary: Combine the data members retryable_, data_loss_ and scope_ of IOStatus with Status, as protected members. IOStatus is now defined as a derived class of Status with no new data, but additional methods. This will allow us to eventually track the result of FileSystem calls in RocksDB with one variable instead of two. Benchmark commands and results are below. The performance after changes seems slightly better. ```./db_bench -db=/data/mysql/rocksdb/prefix_scan -benchmarks="fillseq" -key_size=32 -value_size=512 -num=5000000 -use_direct_io_for_flush_and_compaction=true -target_file_size_base=16777216``` ```./db_bench -use_existing_db=true --db=/data/mysql/rocksdb/prefix_scan -benchmarks="readseq,seekrandom,readseq" -key_size=32 -value_size=512 -num=5000000 -seek_nexts=10000 -use_direct_reads=true -duration=60 -ops_between_duration_checks=1 -readonly=true -adaptive_readahead=false -threads=1 -cache_size=10485760000``` Before - seekrandom : 3715.432 micros/op 269 ops/sec; 1394.9 MB/s (16149 of 16149 found) seekrandom : 3687.177 micros/op 271 ops/sec; 1405.6 MB/s (16273 of 16273 found) seekrandom : 3709.646 micros/op 269 ops/sec; 1397.1 MB/s (16175 of 16175 found) readseq : 0.369 micros/op 2711321 ops/sec; 1406.6 MB/s readseq : 0.363 micros/op 2754092 ops/sec; 1428.8 MB/s readseq : 0.372 micros/op 2688046 ops/sec; 1394.6 MB/s After - seekrandom : 3606.830 micros/op 277 ops/sec; 1436.9 MB/s (16636 of 16636 found) seekrandom : 3594.467 micros/op 278 ops/sec; 1441.9 MB/s (16693 of 16693 found) seekrandom : 3597.919 micros/op 277 ops/sec; 1440.5 MB/s (16677 of 16677 found) readseq : 0.354 micros/op 2822809 ops/sec; 1464.5 MB/s readseq : 0.358 micros/op 2795080 ops/sec; 1450.1 MB/s readseq : 0.354 micros/op 2822889 ops/sec; 1464.5 MB/s Pull Request resolved: https://github.com/facebook/rocksdb/pull/9549 Reviewed By: pdillinger Differential Revision: D34310362 Pulled By: anand1976 fbshipit-source-id: 54b27756edf9c9ecfe730a2dce542a7a46743096 --- include/rocksdb/io_status.h | 37 ++++++++------------------- include/rocksdb/status.h | 51 ++++++++++++++++++++++++++++++++++--- util/status.cc | 7 ++++- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/include/rocksdb/io_status.h b/include/rocksdb/io_status.h index e7290e438..5e8cf6c7a 100644 --- a/include/rocksdb/io_status.h +++ b/include/rocksdb/io_status.h @@ -28,7 +28,7 @@ class IOStatus : public Status { using Code = Status::Code; using SubCode = Status::SubCode; - enum IOErrorScope { + enum IOErrorScope : unsigned char { kIOErrorScopeFileSystem, kIOErrorScopeFile, kIOErrorScopeRange, @@ -57,11 +57,13 @@ class IOStatus : public Status { void SetRetryable(bool retryable) { retryable_ = retryable; } void SetDataLoss(bool data_loss) { data_loss_ = data_loss; } - void SetScope(IOErrorScope scope) { scope_ = scope; } + void SetScope(IOErrorScope scope) { + scope_ = static_cast(scope); + } bool GetRetryable() const { return retryable_; } bool GetDataLoss() const { return data_loss_; } - IOErrorScope GetScope() const { return scope_; } + IOErrorScope GetScope() const { return static_cast(scope_); } // Return a success status. static IOStatus OK() { return IOStatus(); } @@ -137,15 +139,9 @@ class IOStatus : public Status { private: friend IOStatus status_to_io_status(Status&&); - bool retryable_; - bool data_loss_; - IOErrorScope scope_; explicit IOStatus(Code _code, SubCode _subcode = kNone) - : Status(_code, _subcode), - retryable_(false), - data_loss_(false), - scope_(kIOErrorScopeFileSystem) {} + : Status(_code, _subcode, false, false, kIOErrorScopeFileSystem) {} IOStatus(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2); IOStatus(Code _code, const Slice& msg, const Slice& msg2) @@ -154,10 +150,7 @@ class IOStatus : public Status { inline IOStatus::IOStatus(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2) - : Status(_code, _subcode), - retryable_(false), - data_loss_(false), - scope_(kIOErrorScopeFileSystem) { + : Status(_code, _subcode, false, false, kIOErrorScopeFileSystem) { assert(code_ != kOk); assert(subcode_ != kMaxSubCode); const size_t len1 = msg.size(); @@ -249,18 +242,10 @@ inline bool IOStatus::operator!=(const IOStatus& rhs) const { } inline IOStatus status_to_io_status(Status&& status) { - if (status.ok()) { - // Fast path - return IOStatus::OK(); - } else { - const char* state = status.getState(); - if (state) { - return IOStatus(status.code(), status.subcode(), - Slice(state, strlen(status.getState()) + 1), Slice()); - } else { - return IOStatus(status.code(), status.subcode()); - } - } + IOStatus io_s; + Status& s = io_s; + s = std::move(status); + return io_s; } } // namespace ROCKSDB_NAMESPACE diff --git a/include/rocksdb/status.h b/include/rocksdb/status.h index 5550fa828..b512b2da9 100644 --- a/include/rocksdb/status.h +++ b/include/rocksdb/status.h @@ -35,7 +35,14 @@ namespace ROCKSDB_NAMESPACE { class Status { public: // Create a success status. - Status() : code_(kOk), subcode_(kNone), sev_(kNoError), state_(nullptr) {} + Status() + : code_(kOk), + subcode_(kNone), + sev_(kNoError), + retryable_(false), + data_loss_(false), + scope_(0), + state_(nullptr) {} ~Status() { #ifdef ROCKSDB_ASSERT_STATUS_CHECKED if (!checked_) { @@ -454,6 +461,9 @@ class Status { Code code_; SubCode subcode_; Severity sev_; + bool retryable_; + bool data_loss_; + unsigned char scope_; // A nullptr state_ (which is at least the case for OK) means the extra // message is empty. std::unique_ptr state_; @@ -462,7 +472,21 @@ class Status { #endif // ROCKSDB_ASSERT_STATUS_CHECKED explicit Status(Code _code, SubCode _subcode = kNone) - : code_(_code), subcode_(_subcode), sev_(kNoError) {} + : code_(_code), + subcode_(_subcode), + sev_(kNoError), + retryable_(false), + data_loss_(false), + scope_(0) {} + + explicit Status(Code _code, SubCode _subcode, bool retryable, bool data_loss, + unsigned char scope) + : code_(_code), + subcode_(_subcode), + sev_(kNoError), + retryable_(retryable), + data_loss_(data_loss), + scope_(scope) {} Status(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2, Severity sev = kNoError); @@ -479,12 +503,22 @@ class Status { }; inline Status::Status(const Status& s) - : code_(s.code_), subcode_(s.subcode_), sev_(s.sev_) { + : code_(s.code_), + subcode_(s.subcode_), + sev_(s.sev_), + retryable_(s.retryable_), + data_loss_(s.data_loss_), + scope_(s.scope_) { s.MarkChecked(); state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_.get()); } inline Status::Status(const Status& s, Severity sev) - : code_(s.code_), subcode_(s.subcode_), sev_(sev) { + : code_(s.code_), + subcode_(s.subcode_), + sev_(sev), + retryable_(s.retryable_), + data_loss_(s.data_loss_), + scope_(s.scope_) { s.MarkChecked(); state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_.get()); } @@ -495,6 +529,9 @@ inline Status& Status::operator=(const Status& s) { code_ = s.code_; subcode_ = s.subcode_; sev_ = s.sev_; + retryable_ = s.retryable_; + data_loss_ = s.data_loss_; + scope_ = s.scope_; state_ = (s.state_ == nullptr) ? nullptr : CopyState(s.state_.get()); } return *this; @@ -523,6 +560,12 @@ inline Status& Status::operator=(Status&& s) s.subcode_ = kNone; sev_ = std::move(s.sev_); s.sev_ = kNoError; + retryable_ = std::move(s.retryable_); + s.retryable_ = false; + data_loss_ = std::move(s.data_loss_); + s.data_loss_ = false; + scope_ = std::move(s.scope_); + s.scope_ = 0; state_ = std::move(s.state_); } return *this; diff --git a/util/status.cc b/util/status.cc index 025c47d31..2c9aa5015 100644 --- a/util/status.cc +++ b/util/status.cc @@ -46,7 +46,12 @@ static const char* msgs[static_cast(Status::kMaxSubCode)] = { Status::Status(Code _code, SubCode _subcode, const Slice& msg, const Slice& msg2, Severity sev) - : code_(_code), subcode_(_subcode), sev_(sev) { + : code_(_code), + subcode_(_subcode), + sev_(sev), + retryable_(false), + data_loss_(false), + scope_(0) { assert(subcode_ != kMaxSubCode); const size_t len1 = msg.size(); const size_t len2 = msg2.size();