From: Adam C. Emerson Date: Thu, 20 May 2021 23:19:55 +0000 (-0400) Subject: rgw: Simplify log shard probing and err on the side of omap X-Git-Tag: v16.2.5~86^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2d9cbc61a06c36fc0f71e3310821fddca3ac55c0;p=ceph.git rgw: Simplify log shard probing and err on the side of omap In the multigeneration version we no longer care whether entries exist, since we never delete and recreate empty logs. Remove logic that marked entirely empty shards as DNE under the assumption that they would be deleted if so. Fixes: https://tracker.ceph.com/issues/50169 Signed-off-by: Adam C. Emerson (cherry picked from commit 71ae2dd27ae07e6a5d2fda12834158fd2720bd7b) Signed-off-by: Adam C. Emerson Conflicts: src/rgw/rgw_log_backing.cc - Lambda capture, changed to be strictly conforming. --- diff --git a/src/cls/fifo/cls_fifo.cc b/src/cls/fifo/cls_fifo.cc index fc89a20e6b2b..14313a735165 100644 --- a/src/cls/fifo/cls_fifo.cc +++ b/src/cls/fifo/cls_fifo.cc @@ -217,7 +217,8 @@ int create_meta(cls_method_context_t hctx, auto iter = in->cbegin(); decode(op, iter); } catch (const ceph::buffer::error& err) { - CLS_ERR("ERROR: %s: failed to decode request", __PRETTY_FUNCTION__); + CLS_ERR("ERROR: %s: failed to decode request: %s", __PRETTY_FUNCTION__, + err.what()); return -EINVAL; } @@ -237,19 +238,24 @@ int create_meta(cls_method_context_t hctx, int r = cls_cxx_stat2(hctx, &size, nullptr); if (r < 0 && r != -ENOENT) { - CLS_ERR("ERROR: %s: cls_cxx_stat2() on obj returned %d", __PRETTY_FUNCTION__, r); + CLS_ERR("ERROR: %s: cls_cxx_stat2() on obj returned %d", + __PRETTY_FUNCTION__, r); return r; } if (op.exclusive && r == 0) { - CLS_ERR("%s: exclusive create but queue already exists", __PRETTY_FUNCTION__); + CLS_ERR("%s: exclusive create but queue already exists", + __PRETTY_FUNCTION__); return -EEXIST; } if (r == 0) { + CLS_LOG(5, "%s: FIFO already exists, reading from disk and comparing.", + __PRETTY_FUNCTION__); ceph::buffer::list bl; r = cls_cxx_read2(hctx, 0, size, &bl, CEPH_OSD_OP_FLAG_FADVISE_WILLNEED); if (r < 0) { - CLS_ERR("ERROR: %s: cls_cxx_read2() on obj returned %d", __PRETTY_FUNCTION__, r); + CLS_ERR("ERROR: %s: cls_cxx_read2() on obj returned %d", + __PRETTY_FUNCTION__, r); return r; } @@ -258,7 +264,8 @@ int create_meta(cls_method_context_t hctx, auto iter = bl.cbegin(); decode(header, iter); } catch (const ceph::buffer::error& err) { - CLS_ERR("ERROR: %s: failed decoding header", __PRETTY_FUNCTION__); + CLS_ERR("ERROR: %s: failed decoding header: %s", + __PRETTY_FUNCTION__, err.what()); return -EIO; } diff --git a/src/rgw/rgw_log_backing.cc b/src/rgw/rgw_log_backing.cc index baa1ea73836e..415ad1363d52 100644 --- a/src/rgw/rgw_log_backing.cc +++ b/src/rgw/rgw_log_backing.cc @@ -30,81 +30,54 @@ inline std::ostream& operator <<(std::ostream& m, const shard_check& t) { namespace { /// Return the shard type, and a bool to see whether it has entries. -std::pair +shard_check probe_shard(const DoutPrefixProvider *dpp, librados::IoCtx& ioctx, const std::string& oid, bool& fifo_unsupported, optional_yield y) { - bool omap = false; - { - librados::ObjectReadOperation op; - cls_log_header header; - cls_log_info(op, &header); - auto r = rgw_rados_operate(dpp, ioctx, oid, &op, nullptr, y); - if (r == -ENOENT) { - return { shard_check::dne, {} }; - } - - if (r < 0) { - ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << " error probing for omap: r=" << r - << ", oid=" << oid << dendl; - return { shard_check::corrupt, {} }; - } - if (header != cls_log_header{}) - omap = true; - } + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " probing oid=" << oid + << dendl; if (!fifo_unsupported) { std::unique_ptr fifo; auto r = rgw::cls::fifo::FIFO::open(dpp, ioctx, oid, &fifo, y, std::nullopt, true); - if (r < 0 && !(r == -ENOENT || r == -ENODATA || r == -EPERM)) { - ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << " error probing for fifo: r=" << r - << ", oid=" << oid << dendl; - return { shard_check::corrupt, {} }; - } - if (fifo && omap) { - ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << " fifo and omap found: oid=" << oid << dendl; - return { shard_check::corrupt, {} }; - } - if (fifo) { - bool more = false; - std::vector entries; - r = fifo->list(dpp, 1, nullopt, &entries, &more, y); - if (r < 0) { - ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << ": unable to list entries: r=" << r - << ", oid=" << oid << dendl; - return { shard_check::corrupt, {} }; - } - return { shard_check::fifo, !entries.empty() }; - } - if (r == -EPERM) { - // Returned by OSD id CLS module not loaded. + switch (r) { + case 0: + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << ": oid=" << oid << " is FIFO" + << dendl; + return shard_check::fifo; + + case -ENODATA: + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << ": oid=" << oid << " is empty and therefore OMAP" + << dendl; + return shard_check::omap; + + case -ENOENT: + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << ": oid=" << oid << " does not exist" + << dendl; + return shard_check::dne; + + case -EPERM: + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << ": FIFO is unsupported, marking." + << dendl; fifo_unsupported = true; - } - } - if (omap) { - std::list entries; - std::string out_marker; - bool truncated = false; - librados::ObjectReadOperation op; - cls_log_list(op, {}, {}, {}, 1, entries, - &out_marker, &truncated); - auto r = rgw_rados_operate(dpp, ioctx, oid, &op, nullptr, y); - if (r < 0) { + return shard_check::omap; + + default: ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << ": failed to list: r=" << r << ", oid=" << oid << dendl; - return { shard_check::corrupt, {} }; + << ": error probing: r=" << r + << ", oid=" << oid << dendl; + return shard_check::corrupt; } - return { shard_check::omap, !entries.empty() }; + } else { + // Since FIFO is unsupported, OMAP is the only alternative + return shard_check::omap; } - - // An object exists, but has never had FIFO or cls_log entries written - // to it. Likely just the marker Omap. - return { shard_check::dne, {} }; } tl::expected @@ -147,7 +120,7 @@ log_backing_type(const DoutPrefixProvider *dpp, auto check = shard_check::dne; bool fifo_unsupported = false; for (int i = 0; i < shards; ++i) { - auto [c, e] = probe_shard(dpp, ioctx, get_oid(i), fifo_unsupported, y); + auto c = probe_shard(dpp, ioctx, get_oid(i), fifo_unsupported, y); if (c == shard_check::corrupt) return tl::unexpected(bs::error_code(EIO, bs::system_category())); if (c == shard_check::dne) continue; @@ -660,13 +633,13 @@ bs::error_code logback_generations::remove_empty(const DoutPrefixProvider *dpp, for (const auto& [gen_id, e] : es) { ceph_assert(e.pruned); auto ec = log_remove(dpp, ioctx, shards, - [this, gen_id](int shard) { + [this, gen_id = gen_id](int shard) { return this->get_oid(gen_id, shard); }, (gen_id == 0), y); if (ec) { ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << ": Error pruning: gen_id=" << gen_id - << " ec=" << ec.message() << dendl; + << ": Error pruning: gen_id=" << gen_id + << " ec=" << ec.message() << dendl; } if (auto i = es2.find(gen_id); i != es2.end()) { es2.erase(i); diff --git a/src/rgw/rgw_log_backing.h b/src/rgw/rgw_log_backing.h index 5b9f1bfd21cd..29b315b21642 100644 --- a/src/rgw/rgw_log_backing.h +++ b/src/rgw/rgw_log_backing.h @@ -115,6 +115,10 @@ struct logback_generation { } }; WRITE_CLASS_ENCODER(logback_generation) +inline std::ostream& operator <<(std::ostream& m, const logback_generation& g) { + return m << "[" << g.gen_id << "," << g.type << "," + << (g.pruned ? "PRUNED" : "NOT PRUNED") << "]"; +} class logback_generations : public librados::WatchCtx2 { public: diff --git a/src/test/rgw/test_log_backing.cc b/src/test/rgw/test_log_backing.cc index f1bc30c762ab..c67debb9f59e 100644 --- a/src/test/rgw/test_log_backing.cc +++ b/src/test/rgw/test_log_backing.cc @@ -249,7 +249,6 @@ TEST_F(LogBacking, GenerationSingle) auto ec = lg->empty_to(&dp, 0, null_yield); ASSERT_TRUE(ec); - lg.reset(); lg = *logback_generations::init( @@ -305,18 +304,6 @@ TEST_F(LogBacking, GenerationSingle) ASSERT_EQ(1, lg->got_entries[1].gen_id); ASSERT_EQ(log_type::omap, lg->got_entries[1].type); ASSERT_FALSE(lg->got_entries[1].pruned); - - ec = lg->remove_empty(&dp, null_yield); - ASSERT_FALSE(ec); - - auto entries = lg->entries(); - ASSERT_EQ(1, entries.size()); - - ASSERT_EQ(1, entries[1].gen_id); - ASSERT_EQ(log_type::omap, entries[1].type); - ASSERT_FALSE(entries[1].pruned); - - lg.reset(); } TEST_F(LogBacking, GenerationWN)