From c34271a5a58e09f5a8d93ad961db9b53aa553636 Mon Sep 17 00:00:00 2001 From: Mayank Agarwal Date: Sun, 1 Sep 2013 14:58:53 -0700 Subject: [PATCH] Fix bug in Counters and record Sequencenumber using only TickerCount Summary: The way counters/statistics are implemented in rocksdb demands that enum Tickers and TickerNameMap follow the same order, otherwise statistics exposed from fbcode/rocks get out-of-sync. 2 counters for prefix had violated this order and when I built counters for fbcode/mcrocksdb, statistics for sequence number were appearing out-of-sync. The other change is to record sequence-number using setTickerCount only and not recordTick. This is because of difference in statistics as understood by rocks/utils which uses ServiceData::statistics function and rocksdb statistics. In rocksdb there is just 1 counter for a countername. But in ServiceData there are 4 independent buckets for every countername-Count, Sum, Average and Rate. SetTickerCount and RecordTick update the same variable in rocksdb but different buckets in ServiceData. Therefore, I had to choose one consistent function from RecordTick or SetTickerCount for sequence number in rocksdb. I chose SetTickerCount because the statistics object in options passed during rocksdb-open is user-dependent and SetTickerCount makes sense there. There will be a corresponding diff to mcorcksdb in fbcode shortly. Test Plan: make all check; check ticker value using fprintfs Reviewers: dhruba Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D12669 --- db/db_impl.cc | 2 +- include/rocksdb/statistics.h | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index 4c7488fa9..6ed88da43 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -2468,7 +2468,7 @@ Status DBImpl::Write(const WriteOptions& options, WriteBatch* my_batch) { // have succeeded in memtable but Status reports error for all writes. throw std::runtime_error("In memory WriteBatch corruption!"); } - RecordTick(options_.statistics, SEQUENCE_NUMBER, my_batch_count); + SetTickerCount(options_.statistics, SEQUENCE_NUMBER, last_sequence); versions_->SetLastSequence(last_sequence); last_flushed_sequence_ = current_sequence; } diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h index 6b0f9d286..e665278b0 100644 --- a/include/rocksdb/statistics.h +++ b/include/rocksdb/statistics.h @@ -75,8 +75,6 @@ const std::vector> TickersNameMap = { { BLOCK_CACHE_MISS, "rocksdb.block.cache.miss" }, { BLOCK_CACHE_HIT, "rocksdb.block.cache.hit" }, { BLOOM_FILTER_USEFUL, "rocksdb.bloom.filter.useful" }, - { BLOOM_FILTER_PREFIX_CHECKED, "rocksdb.bloom.filter.prefix.checked" }, - { BLOOM_FILTER_PREFIX_USEFUL, "rocksdb.bloom.filter.prefix.useful" }, { COMPACTION_KEY_DROP_NEWER_ENTRY, "rocksdb.compaction.key.drop.new" }, { COMPACTION_KEY_DROP_OBSOLETE, "rocksdb.compaction.key.drop.obsolete" }, { COMPACTION_KEY_DROP_USER, "rocksdb.compaction.key.drop.user" }, @@ -97,7 +95,9 @@ const std::vector> TickersNameMap = { { NUMBER_MULTIGET_BYTES_READ, "rocksdb.number.multiget.bytes.read" }, { NUMBER_FILTERED_DELETES, "rocksdb.number.deletes.filtered" }, { NUMBER_MERGE_FAILURES, "rocksdb.number.merge.failures" }, - { SEQUENCE_NUMBER, "rocksdb.sequence.number" } + { SEQUENCE_NUMBER, "rocksdb.sequence.number" }, + { BLOOM_FILTER_PREFIX_CHECKED, "rocksdb.bloom.filter.prefix.checked" }, + { BLOOM_FILTER_PREFIX_USEFUL, "rocksdb.bloom.filter.prefix.useful" } }; /**