From 5945e16dfc6e99522c607841cfd387eebed256fc Mon Sep 17 00:00:00 2001 From: Soli Como Date: Tue, 13 Nov 2018 11:44:25 -0800 Subject: [PATCH] Divide `NO_ITERATORS` into two counters `NO_ITERATOR_CREATED` and `NO_ITERATOR_DELETE` (#4498) Summary: Currently, `Statistics` can record tick by `recordTick()` whose second parameter is an `uint64_t`. That means tick can only increase. If we want to reduce tick, we have to work around like `RecordTick(statistics_, NO_ITERATORS, uint64_t(-1));`. That's kind of a hack. So, this PR divide `NO_ITERATORS` into two counters `NO_ITERATOR_CREATED` and `NO_ITERATOR_DELETE`, making the counters increase only. Fixes #3013 . Pull Request resolved: https://github.com/facebook/rocksdb/pull/4498 Differential Revision: D10395010 Pulled By: sagar0 fbshipit-source-id: cfb523b22a37411c794b4e9da090f1ae30293db2 --- HISTORY.md | 2 ++ db/db_iter.cc | 6 ++---- include/rocksdb/statistics.h | 7 +++++-- java/rocksjni/portal.h | 10 +++++++--- java/src/main/java/org/rocksdb/TickerType.java | 11 ++++++++--- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index 9cc5b7f58..850d83a35 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,7 @@ # Rocksdb Change Log ## Unreleased +### Public API Change +* `NO_ITERATORS` is divided into two counters `NO_ITERATOR_CREATED` and `NO_ITERATOR_DELETE`. Both of them are only increasing now, just as other counters. ## 5.18.0 (11/12/2018) ### New Features diff --git a/db/db_iter.cc b/db/db_iter.cc index 923cce462..6bca92982 100644 --- a/db/db_iter.cc +++ b/db/db_iter.cc @@ -142,7 +142,7 @@ class DBIter final: public Iterator { allow_blob_(allow_blob), is_blob_(false), start_seqnum_(read_options.iter_start_seqnum) { - RecordTick(statistics_, NO_ITERATORS); + RecordTick(statistics_, NO_ITERATOR_CREATED); prefix_extractor_ = mutable_cf_options.prefix_extractor.get(); max_skip_ = max_sequential_skip_in_iterations; max_skippable_internal_keys_ = read_options.max_skippable_internal_keys; @@ -158,9 +158,7 @@ class DBIter final: public Iterator { if (pinned_iters_mgr_.PinningEnabled()) { pinned_iters_mgr_.ReleasePinnedData(); } - // Compiler warning issue filed: - // https://github.com/facebook/rocksdb/issues/3013 - RecordTick(statistics_, NO_ITERATORS, uint64_t(-1)); + RecordTick(statistics_, NO_ITERATOR_DELETED); ResetInternalKeysSkippedCounter(); local_stats_.BumpGlobalStatistics(statistics_); if (!arena_mode_) { diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h index c493a1824..d9f413c11 100644 --- a/include/rocksdb/statistics.h +++ b/include/rocksdb/statistics.h @@ -155,7 +155,7 @@ enum Tickers : uint32_t { // Disabled by default. To enable it set stats level to kAll DB_MUTEX_WAIT_MICROS, RATE_LIMIT_DELAY_MILLIS, - NO_ITERATORS, // number of iterators currently open + NO_ITERATOR_CREATED, // number of iterators created // Number of MultiGet calls, keys read, and bytes read NUMBER_MULTIGET_CALLS, @@ -322,6 +322,8 @@ enum Tickers : uint32_t { // Number of keys actually found in MultiGet calls (vs number requested by caller) // NUMBER_MULTIGET_KEYS_READ gives the number requested by caller NUMBER_MULTIGET_KEYS_FOUND, + + NO_ITERATOR_DELETED, // number of iterators deleted TICKER_ENUM_MAX }; @@ -392,7 +394,7 @@ const std::vector> TickersNameMap = { {STALL_MICROS, "rocksdb.stall.micros"}, {DB_MUTEX_WAIT_MICROS, "rocksdb.db.mutex.wait.micros"}, {RATE_LIMIT_DELAY_MILLIS, "rocksdb.rate.limit.delay.millis"}, - {NO_ITERATORS, "rocksdb.num.iterators"}, + {NO_ITERATOR_CREATED, "rocksdb.num.iterator.created"}, {NUMBER_MULTIGET_CALLS, "rocksdb.number.multiget.get"}, {NUMBER_MULTIGET_KEYS_READ, "rocksdb.number.multiget.keys.read"}, {NUMBER_MULTIGET_BYTES_READ, "rocksdb.number.multiget.bytes.read"}, @@ -474,6 +476,7 @@ const std::vector> TickersNameMap = { {TXN_DUPLICATE_KEY_OVERHEAD, "rocksdb.txn.overhead.duplicate.key"}, {TXN_SNAPSHOT_MUTEX_OVERHEAD, "rocksdb.txn.overhead.mutex.snapshot"}, {NUMBER_MULTIGET_KEYS_FOUND, "rocksdb.number.multiget.keys.found"}, + {NO_ITERATOR_DELETED, "rocksdb.num.iterator.deleted"}, }; /** diff --git a/java/rocksjni/portal.h b/java/rocksjni/portal.h index 7a8d1f157..58f242986 100644 --- a/java/rocksjni/portal.h +++ b/java/rocksjni/portal.h @@ -3301,7 +3301,7 @@ class TickerTypeJni { return 0x36; case rocksdb::Tickers::RATE_LIMIT_DELAY_MILLIS: return 0x37; - case rocksdb::Tickers::NO_ITERATORS: + case rocksdb::Tickers::NO_ITERATOR_CREATED: return 0x38; case rocksdb::Tickers::NUMBER_MULTIGET_CALLS: return 0x39; @@ -3379,8 +3379,10 @@ class TickerTypeJni { return 0x5D; case rocksdb::Tickers::NUMBER_MULTIGET_KEYS_FOUND: return 0x5E; - case rocksdb::Tickers::TICKER_ENUM_MAX: + case rocksdb::Tickers::NO_ITERATOR_DELETED: return 0x5F; + case rocksdb::Tickers::TICKER_ENUM_MAX: + return 0x60; default: // undefined/default @@ -3505,7 +3507,7 @@ class TickerTypeJni { case 0x37: return rocksdb::Tickers::RATE_LIMIT_DELAY_MILLIS; case 0x38: - return rocksdb::Tickers::NO_ITERATORS; + return rocksdb::Tickers::NO_ITERATOR_CREATED; case 0x39: return rocksdb::Tickers::NUMBER_MULTIGET_CALLS; case 0x3A: @@ -3583,6 +3585,8 @@ class TickerTypeJni { case 0x5E: return rocksdb::Tickers::NUMBER_MULTIGET_KEYS_FOUND; case 0x5F: + return rocksdb::Tickers::NO_ITERATOR_DELETED; + case 0x60: return rocksdb::Tickers::TICKER_ENUM_MAX; default: diff --git a/java/src/main/java/org/rocksdb/TickerType.java b/java/src/main/java/org/rocksdb/TickerType.java index fdcf62ff8..08ed18fb3 100644 --- a/java/src/main/java/org/rocksdb/TickerType.java +++ b/java/src/main/java/org/rocksdb/TickerType.java @@ -304,9 +304,9 @@ public enum TickerType { RATE_LIMIT_DELAY_MILLIS((byte) 0x37), /** - * Number of iterators currently open. + * Number of iterators created. */ - NO_ITERATORS((byte) 0x38), + NO_ITERATOR_CREATED((byte) 0x38), /** * Number of MultiGet calls. @@ -475,7 +475,12 @@ public enum TickerType { */ NUMBER_MULTIGET_KEYS_FOUND((byte) 0x5E), - TICKER_ENUM_MAX((byte) 0x5F); + /** + * Number of iterators deleted. + */ + NO_ITERATOR_DELETED((byte) 0x5F), + + TICKER_ENUM_MAX((byte) 0x60); private final byte value;