From: Samuel Just Date: Thu, 28 May 2020 20:31:27 +0000 (-0700) Subject: osd_types: replace build_intersection_set with calc_refs_to_drop_on_removal X-Git-Tag: v16.1.0~1738^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=408a6dffd7ba01fc73819f30ac70fb19199134f9;p=ceph.git osd_types: replace build_intersection_set with calc_refs_to_drop_on_removal calc_refs_to_to_drop_on_removal correctly handles cases where adjacent clones share a ref not shared by the clone to be removed as well as multiple references to the same backing chunk. This patch also adds tests specifically for calc_refs_to_drop_on_removal. Signed-off-by: Samuel Just --- diff --git a/src/osd/osd_types.cc b/src/osd/osd_types.cc index 969bd566e8c3..55e6e154c93b 100644 --- a/src/osd/osd_types.cc +++ b/src/osd/osd_types.cc @@ -5843,26 +5843,81 @@ ostream& operator<<(ostream& out, const chunk_info_t& ci) // -- object_manifest_t -- -/** - * build_intersection_set - * - * Returns the set offsets to references common (offset, length, and target match) to - * both map and this->chunk_map. - * - * @param map [in] map of object against which to compare - * @param map [in,out] set of target ids common to both maps - * @return void - */ +std::ostream& operator<<(std::ostream& out, const object_ref_delta_t & ci) +{ + return out << ci.ref_delta << std::endl; +} -void object_manifest_t::build_intersection_set(const std::map& map, - std::set& intersection) +void object_manifest_t::calc_refs_to_drop_on_removal( + const object_manifest_t* _g, + const object_manifest_t* _l, + object_ref_delta_t &refs) const { - for (const auto &p : chunk_map) { - auto c = map.find(p.first); - if (c != map.cend()) { - if (p == *c) { - intersection.insert(p.first); - } + /* At a high level, the rule is that consecutive clones with the same reference + * at the same offset share a reference. As such, removing *this may result + * in removing references in two cases: + * 1) *this has a reference which it shares with neither _g nor _l + * 2) _g and _l have a reference which they share with each other but not + * *this. + * + * For a particular offset, both 1 and 2 can happen. + * + * Notably, this means that to evaluate the reference change from removing + * the object with *this, we only need to look at the two adjacent clones. + */ + + // Paper over possibly missing _g or _l -- nullopt is semantically the same + // as an empty chunk_map + static const object_manifest_t empty; + const object_manifest_t &g = _g ? *_g : empty; + const object_manifest_t &l = _l ? *_l : empty; + + auto giter = g.chunk_map.begin(); + auto iter = chunk_map.begin(); + auto liter = l.chunk_map.begin(); + + // Translate iter, map pair to the current offset, end() -> max + auto get_offset = [](decltype(iter) &i, const object_manifest_t &manifest) + -> uint64_t { + return i == manifest.chunk_map.end() ? + std::numeric_limits::max() : i->first; + }; + + // Translate current, iter, map to a chunk_info_t, nullopt_t if iter + // is ahead of current or is at end() + auto get_chunk = []( + uint64_t current, decltype(iter) &i, const object_manifest_t &manifest) + -> const chunk_info_t * { + if (i == manifest.chunk_map.end() || current != i->first) { + return nullptr; + } else { + // We advance the iterator iff we consider the chunk_map on this iteration + return &(i++)->second; + } + }; + + while (giter != g.chunk_map.end() || + iter != chunk_map.end() || + liter != l.chunk_map.end()) { + auto current = std::min( + std::min(get_offset(giter, g), get_offset(iter, *this)), + get_offset(liter, l)); + + auto gchunk = get_chunk(current, giter, g); + auto chunk = get_chunk(current, iter, *this); + auto lchunk = get_chunk(current, liter, l); + + if (gchunk && lchunk && *gchunk == *lchunk && + (!chunk || *gchunk != *chunk)) { + // case 1 from above: l and g match, chunk does not + refs.dec_ref(gchunk->oid); + } + + if (chunk && + (!gchunk || chunk->oid != gchunk->oid) && + (!lchunk || chunk->oid != lchunk->oid)) { + // case 2 from above: *this matches neither + refs.dec_ref(chunk->oid); } } } diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h index a97c50ac2b9d..516b3bdf2b5a 100644 --- a/src/osd/osd_types.h +++ b/src/osd/osd_types.h @@ -5422,6 +5422,47 @@ static inline std::ostream& operator<<(std::ostream& out, const notify_info_t& n << " " << n.timeout << "s)"; } +class object_ref_delta_t { + std::map ref_delta; + +public: + object_ref_delta_t() = default; + object_ref_delta_t(const object_ref_delta_t &) = default; + object_ref_delta_t(object_ref_delta_t &&) = default; + + object_ref_delta_t(decltype(ref_delta) &&ref_delta) + : ref_delta(std::move(ref_delta)) {} + object_ref_delta_t(const decltype(ref_delta) &ref_delta) + : ref_delta(ref_delta) {} + + object_ref_delta_t &operator=(const object_ref_delta_t &) = default; + object_ref_delta_t &operator=(object_ref_delta_t &&) = default; + + void dec_ref(const hobject_t &hoid, unsigned num=1) { + mut_ref(hoid, -num); + } + void inc_ref(const hobject_t &hoid, unsigned num=1) { + mut_ref(hoid, num); + } + void mut_ref(const hobject_t &hoid, int num) { + auto [iter, _] = ref_delta.try_emplace(hoid, 0); + iter->second += num; + if (iter->second == 0) + ref_delta.erase(iter); + } + + auto begin() const { return ref_delta.begin(); } + auto end() const { return ref_delta.end(); } + + bool operator==(const object_ref_delta_t &rhs) const { + return ref_delta == rhs.ref_delta; + } + bool operator!=(const object_ref_delta_t &rhs) const { + return !(*this == rhs); + } + friend std::ostream& operator<<(std::ostream& out, const object_ref_delta_t & ci); +}; + struct chunk_info_t { typedef enum { FLAG_DIRTY = 1, @@ -5486,6 +5527,9 @@ struct chunk_info_t { void dump(ceph::Formatter *f) const; friend std::ostream& operator<<(std::ostream& out, const chunk_info_t& ci); bool operator==(const chunk_info_t& cit) const; + bool operator!=(const chunk_info_t& cit) const { + return !(cit == *this); + } }; WRITE_CLASS_ENCODER(chunk_info_t) std::ostream& operator<<(std::ostream& out, const chunk_info_t& ci); @@ -5530,8 +5574,22 @@ struct object_manifest_t { redirect_target = hobject_t(); chunk_map.clear(); } - void build_intersection_set(const std::map& map, - std::set& intersection); + + /** + * calc_refs_to_drop_on_removal + * + * Takes the two adjacent manifests and returns the set of refs to + * drop upon removal of the clone containing *this. + * + * g should be nullptr if *this is on HEAD, l should be nullptr if + * *this is on the oldest clone (or head if there are no clones). + */ + void calc_refs_to_drop_on_removal( + const object_manifest_t* g, ///< [in] manifest for clone > *this + const object_manifest_t* l, ///< [in] manifest for clone < *this + object_ref_delta_t &delta ///< [out] set of refs to drop + ) const; + static void generate_test_instances(std::list& o); void encode(ceph::buffer::list &bl) const; void decode(ceph::buffer::list::const_iterator &bl); diff --git a/src/test/osd/types.cc b/src/test/osd/types.cc index d9a821ef99c7..bee52c47b6a0 100644 --- a/src/test/osd/types.cc +++ b/src/test/osd/types.cc @@ -1887,6 +1887,146 @@ TEST_F(PITest, past_intervals_ec_lost) { /* pg_down */ false); } +void ci_ref_test( + object_manifest_t l, + object_manifest_t to_remove, + object_manifest_t g, + object_ref_delta_t expected_delta) +{ + { + object_ref_delta_t delta; + to_remove.calc_refs_to_drop_on_removal( + &l, + &g, + delta); + ASSERT_EQ( + expected_delta, + delta); + } + + // calc_refs_to_drop specifically handles nullptr identically to empty + // chunk_map + if (l.chunk_map.empty() || g.chunk_map.empty()) { + object_ref_delta_t delta; + to_remove.calc_refs_to_drop_on_removal( + l.chunk_map.empty() ? nullptr : &l, + g.chunk_map.empty() ? nullptr : &g, + delta); + ASSERT_EQ( + expected_delta, + delta); + } +} + +hobject_t mk_hobject(string name) +{ + return hobject_t( + std::move(name), + string(), + CEPH_NOSNAP, + 0x42, + 1, + string()); +} + +object_manifest_t mk_manifest( + std::map> m) +{ + object_manifest_t ret; + ret.type = object_manifest_t::TYPE_CHUNKED; + for (auto &[offset, tgt] : m) { + auto &[tgt_off, length, name] = tgt; + auto &ci = ret.chunk_map[offset]; + ci.offset = tgt_off; + ci.length = length; + ci.oid = mk_hobject(name); + } + return ret; +} + +object_ref_delta_t mk_delta(std::map _m) { + std::map m; + for (auto &[name, delta] : _m) { + m.insert( + std::make_pair( + mk_hobject(name), + delta)); + } + return object_ref_delta_t(std::move(m)); +} + +TEST(chunk_info_test, calc_refs_to_drop) { + ci_ref_test( + mk_manifest({}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({}), + mk_delta({{"foo", -1}})); + +} + + +TEST(chunk_info_test, calc_refs_to_drop_match) { + ci_ref_test( + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_delta({})); + +} + +TEST(chunk_info_test, calc_refs_to_drop_head_match) { + ci_ref_test( + mk_manifest({}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_delta({})); + +} + +TEST(chunk_info_test, calc_refs_to_drop_tail_match) { + ci_ref_test( + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({}), + mk_delta({})); + +} + +TEST(chunk_info_test, calc_refs_to_drop_second_reference) { + ci_ref_test( + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}, {4<<10, {0, 1<<10, "foo"}}}), + mk_manifest({}), + mk_delta({{"foo", -1}})); + +} + +TEST(chunk_info_test, calc_refs_offsets_dont_match) { + ci_ref_test( + mk_manifest({{0, {0, 1024, "foo"}}}), + mk_manifest({{512, {0, 1024, "foo"}}, {(4<<10) + 512, {0, 1<<10, "foo"}}}), + mk_manifest({}), + mk_delta({{"foo", -2}})); + +} + +TEST(chunk_info_test, calc_refs_g_l_match) { + ci_ref_test( + mk_manifest({{4096, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "foo"}}, {4096, {0, 1024, "bar"}}}), + mk_manifest({{4096, {0, 1024, "foo"}}}), + mk_delta({{"foo", -2}, {"bar", -1}})); + +} + +TEST(chunk_info_test, calc_refs_g_l_match_no_this) { + ci_ref_test( + mk_manifest({{4096, {0, 1024, "foo"}}}), + mk_manifest({{0, {0, 1024, "bar"}}}), + mk_manifest({{4096, {0, 1024, "foo"}}}), + mk_delta({{"foo", -1}, {"bar", -1}})); + +} /* * Local Variables: