From 07cf0ee5891582152a1fd26a107555173f294bdb Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 21 Nov 2018 18:29:08 -0800 Subject: [PATCH] Fix ticker stat for number files closed (#4703) Summary: We haven't been populating `NO_FILE_CLOSES` since v1.5.8 even though it was never marked as deprecated. Start populating it again. Conveniently `DeleteTableReader` has an unused `void*` argument that we can use... Blame: 63f216ee0a2a6e28f9dfe24913d134d3a7fa3aca Closes #4700. Pull Request resolved: https://github.com/facebook/rocksdb/pull/4703 Differential Revision: D13146769 Pulled By: ajkr fbshipit-source-id: ad8d6fb0493e701f60a165a3bca1787d255be008 --- HISTORY.md | 1 + db/db_iterator_test.cc | 4 ++++ db/table_cache.cc | 7 +++++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index b844c2902..99cb3dfb2 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,6 +6,7 @@ * `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. ### Bug Fixes * Fixed Get correctness bug in the presence of range tombstones where merge operands covered by a range tombstone always result in NotFound. +* Start populating `NO_FILE_CLOSES` ticker statistic, which was always zero previously. ## 5.18.0 (11/12/2018) ### New Features diff --git a/db/db_iterator_test.cc b/db/db_iterator_test.cc index 994a838dd..0e8c78155 100644 --- a/db/db_iterator_test.cc +++ b/db/db_iterator_test.cc @@ -1951,6 +1951,7 @@ TEST_P(DBIteratorTest, ReadAhead) { size_t bytes_read = env_->random_read_bytes_counter_; delete iter; + int64_t num_file_closes = TestGetTickerCount(options, NO_FILE_CLOSES); env_->random_read_bytes_counter_ = 0; options.statistics->setTickerCount(NO_FILE_OPENS, 0); read_options.readahead_size = 1024 * 10; @@ -1959,7 +1960,10 @@ TEST_P(DBIteratorTest, ReadAhead) { int64_t num_file_opens_readahead = TestGetTickerCount(options, NO_FILE_OPENS); size_t bytes_read_readahead = env_->random_read_bytes_counter_; delete iter; + int64_t num_file_closes_readahead = + TestGetTickerCount(options, NO_FILE_CLOSES); ASSERT_EQ(num_file_opens + 3, num_file_opens_readahead); + ASSERT_EQ(num_file_closes + 3, num_file_closes_readahead); ASSERT_GT(bytes_read_readahead, bytes_read); ASSERT_GT(bytes_read_readahead, read_options.readahead_size * 3); diff --git a/db/table_cache.cc b/db/table_cache.cc index b491a162b..62a02ef33 100644 --- a/db/table_cache.cc +++ b/db/table_cache.cc @@ -42,8 +42,10 @@ static void UnrefEntry(void* arg1, void* arg2) { cache->Release(h); } -static void DeleteTableReader(void* arg1, void* /*arg2*/) { +static void DeleteTableReader(void* arg1, void* arg2) { TableReader* table_reader = reinterpret_cast(arg1); + Statistics* stats = reinterpret_cast(arg2); + RecordTick(stats, NO_FILE_CLOSES); delete table_reader; } @@ -249,7 +251,8 @@ InternalIterator* TableCache::NewIterator( } if (create_new_table_reader) { assert(handle == nullptr); - result->RegisterCleanup(&DeleteTableReader, table_reader, nullptr); + result->RegisterCleanup(&DeleteTableReader, table_reader, + ioptions_.statistics); } else if (handle != nullptr) { result->RegisterCleanup(&UnrefEntry, cache_, handle); handle = nullptr; // prevent from releasing below -- GitLab