From c365cca4f36a01e6942844879366a0072a9ebb13 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Wed, 4 Dec 2013 15:42:21 -0800 Subject: [PATCH] osd/HitSet: track BloomHitSet::Params fpp in micros, not as a double ...and store it as a 32-bit value, so that it actually works! Signed-off-by: Sage Weil --- src/mon/OSDMonitor.cc | 6 +++--- src/osd/HitSet.h | 29 +++++++++++++++++------------ src/osd/ReplicatedPG.cc | 9 ++++----- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index bbd8cf0f80596..7d2ca25b9dbc0 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -2843,7 +2843,7 @@ int OSDMonitor::prepare_command_pool_set(map &cmdmap, p.hit_set_params = HitSet::Params(); else if (val == "bloom") { BloomHitSet::Params *bsp = new BloomHitSet::Params; - bsp->false_positive = .01; + bsp->set_fpp(.01); p.hit_set_params = HitSet::Params(bsp); } else if (val == "explicit_hash") p.hit_set_params = HitSet::Params(new ExplicitHashHitSet::Params); @@ -2878,8 +2878,8 @@ int OSDMonitor::prepare_command_pool_set(map &cmdmap, return -EINVAL; } BloomHitSet::Params *bloomp = static_cast(p.hit_set_params.impl.get()); - bloomp->false_positive = f; - ss << "set hit_set_fpp to " << bloomp->false_positive; + bloomp->set_fpp(f); + ss << "set hit_set_fpp to " << bloomp->get_fpp(); } else { ss << "unrecognized variable '" << var << "'"; return -EINVAL; diff --git a/src/osd/HitSet.h b/src/osd/HitSet.h index ace4d052a1b71..6d11554b95c49 100644 --- a/src/osd/HitSet.h +++ b/src/osd/HitSet.h @@ -345,22 +345,29 @@ public: return new BloomHitSet; } - double false_positive; ///< false positive probability + uint32_t fpp_micro; ///< false positive probability / 1M uint64_t target_size; ///< number of unique insertions we expect to this HitSet uint64_t seed; ///< seed to use when initializing the bloom filter Params() - : false_positive(0), target_size(0), seed(0) {} + : fpp_micro(0), target_size(0), seed(0) {} Params(double fpp, uint64_t t, uint64_t s) - : false_positive(fpp), target_size(t), seed(s) {} + : fpp_micro(fpp * 1000000.0), target_size(t), seed(s) {} Params(const Params &o) - : false_positive(o.false_positive), - target_size(o.target_size), seed(o.seed) {} + : fpp_micro(o.fpp_micro), + target_size(o.target_size), + seed(o.seed) {} ~Params() {} + double get_fpp() const { + return (double)fpp_micro / 1000000.0; + } + void set_fpp(double f) { + fpp_micro = (unsigned)(f * 1000000.0); + } + void encode(bufferlist& bl) const { ENCODE_START(1, 1, bl); - uint16_t fpp_micro = static_cast(false_positive * 1000000.0); ::encode(fpp_micro, bl); ::encode(target_size, bl); ::encode(seed, bl); @@ -368,27 +375,25 @@ public: } void decode(bufferlist::iterator& bl) { DECODE_START(1, bl); - uint16_t fpp_micro; ::decode(fpp_micro, bl); - false_positive = fpp_micro * 1000000.0; ::decode(target_size, bl); ::decode(seed, bl); DECODE_FINISH(bl); } void dump(Formatter *f) const { - f->dump_int("false_positive_probability", false_positive); + f->dump_int("false_positive_probability", get_fpp()); f->dump_int("target_size", target_size); f->dump_int("seed", seed); } void dump_stream(ostream& o) const { o << "false_positive_probability: " - << false_positive << ", target_size: " << target_size + << get_fpp() << ", target_size: " << target_size << ", seed: " << seed; } static void generate_test_instances(list& o) { o.push_back(new Params); o.push_back(new Params); - (*o.rbegin())->false_positive = .11; + (*o.rbegin())->fpp_micro = 123456; (*o.rbegin())->target_size = 300; (*o.rbegin())->seed = 99; } @@ -399,7 +404,7 @@ public: : bloom(inserts, fpp, seed) {} BloomHitSet(const BloomHitSet::Params *p) : bloom(p->target_size, - p->false_positive, + p->get_fpp(), p->seed) {} diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index 7f5a12faa175e..05dec0b229da1 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -8510,11 +8510,10 @@ void ReplicatedPG::hit_set_create() static_cast(params.impl.get()); dout(20) << __func__ << " " << params << " " << p << dendl; - if (p->false_positive <= 0.0) - p->false_positive = .01; // fpp cannot be zero! - // convert false positive rate so it holds up across the full period - p->false_positive = p->false_positive / pool.info.hit_set_count; + p->set_fpp(p->get_fpp() / pool.info.hit_set_count); + if (p->get_fpp() <= 0.0) + p->set_fpp(.01); // fpp cannot be zero! // if we don't have specified size, estimate target size based on the // previous bin! @@ -8532,7 +8531,7 @@ void ReplicatedPG::hit_set_create() p->seed = now.sec(); dout(10) << __func__ << " target_size " << p->target_size - << " fpp " << p->false_positive << dendl; + << " fpp " << p->get_fpp() << dendl; } hit_set.reset(new HitSet(params)); hit_set_start_stats.reset(new pg_stat_t(info.stats)); -- 2.39.5