From 266522a031f4716903c00f8e598203efe543d4b0 Mon Sep 17 00:00:00 2001 From: "Adam C. Emerson" Date: Tue, 29 Nov 2022 19:41:40 -0500 Subject: [PATCH] cls/fifo: Retry on push to nonexistent part A racing client may delete the part we're trying to push to. Use `assert_exists()` to check, and re-read metadata if we receive -ENOENT. Fixes: https://tracker.ceph.com/issues/57562 Signed-off-by: Adam C. Emerson (cherry picked from commit 3493e044cbb04130dcaa50eb70ce0a986270f1b0) Conflicts: src/rgw/cls_fifo_legacy.cc - Upstream dpp Fixes: https://tracker.ceph.com/issues/58403 Signed-off-by: Adam C. Emerson --- src/rgw/cls_fifo_legacy.cc | 76 ++++++++++++++++++++++++++++++++------ 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/src/rgw/cls_fifo_legacy.cc b/src/rgw/cls_fifo_legacy.cc index 6c1357b201b5b..8e15091116c8e 100644 --- a/src/rgw/cls_fifo_legacy.cc +++ b/src/rgw/cls_fifo_legacy.cc @@ -143,6 +143,8 @@ int push_part(const DoutPrefixProvider *dpp, lr::IoCtx& ioctx, const std::string lr::ObjectWriteOperation op; fifo::op::push_part pp; + op.assert_exists(); + pp.data_bufs = data_bufs; pp.total_len = 0; @@ -1388,7 +1390,7 @@ int FIFO::push(const DoutPrefixProvider *dpp, const std::vector& data_ canceled = true; ++retries; ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ - << " need new head tid=" << tid << dendl; + << " need new head tid=" << tid << dendl; r = _prepare_new_head(dpp, head_part_num + 1, tid, y); if (r < 0) { ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ @@ -1399,6 +1401,22 @@ int FIFO::push(const DoutPrefixProvider *dpp, const std::vector& data_ r = 0; continue; } + if (r == -ENOENT) { + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " racing client trimmed part, rereading metadata " + << "tid=" << tid << dendl; + canceled = true; + ++retries; + r = read_meta(dpp, y); + if (r < 0) { + ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " read_meta failed: r=" << r + << " tid=" << tid << dendl; + return r; + } + r = 0; + continue; + } if (r < 0) { ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ << " push_entries failed: r=" << r @@ -1433,9 +1451,9 @@ struct Pusher : public Completion { int i = 0; std::int64_t head_part_num; std::uint64_t tid; - bool new_heading = false; + enum { pushing, new_heading, meta_reading } state = pushing; - void prep_then_push(Ptr&& p, const unsigned successes) { + void prep_then_push(const DoutPrefixProvider *dpp, Ptr&& p, const unsigned successes) { std::unique_lock l(f->m); auto max_part_size = f->info.params.max_part_size; auto part_entry_overhead = f->part_entry_overhead; @@ -1488,18 +1506,37 @@ struct Pusher : public Completion { } void new_head(const DoutPrefixProvider *dpp, Ptr&& p) { - new_heading = true; + state = new_heading; f->_prepare_new_head(dpp, head_part_num + 1, tid, call(std::move(p))); } + void read_meta(const DoutPrefixProvider *dpp, Ptr&& p) { + ++i; + state = meta_reading; + f->read_meta(dpp, tid, call(std::move(p))); + } + void handle(const DoutPrefixProvider *dpp, Ptr&& p, int r) { - if (!new_heading) { + switch (state) { + case pushing: if (r == -ERANGE) { ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ << " need new head tid=" << tid << dendl; new_head(dpp, std::move(p)); return; } + if (r == -ENOENT) { + if (i > MAX_RACE_RETRIES) { + ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " racing client deleted part, but we're out" + << " of retries: tid=" << tid << dendl; + complete(std::move(p), r); + } + ldpp_dout(dpp, 20) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " racing client deleted part: tid=" << tid << dendl; + read_meta(dpp, std::move(p)); + return; + } if (r < 0) { ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ << " push_entries failed: r=" << r @@ -1508,8 +1545,10 @@ struct Pusher : public Completion { return; } i = 0; // We've made forward progress, so reset the race counter! - prep_then_push(std::move(p), r); - } else { + prep_then_push(dpp, std::move(p), r); + break; + + case new_heading: if (r < 0) { ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ << " prepare_new_head failed: r=" << r @@ -1517,12 +1556,25 @@ struct Pusher : public Completion { complete(std::move(p), r); return; } - new_heading = false; - handle_new_head(std::move(p), r); + state = pushing; + handle_new_head(dpp, std::move(p), r); + break; + + case meta_reading: + if (r < 0) { + ldpp_dout(dpp, -1) << __PRETTY_FUNCTION__ << ":" << __LINE__ + << " read_meta failed: r=" << r + << " tid=" << tid << dendl; + complete(std::move(p), r); + return; + } + state = pushing; + prep_then_push(dpp, std::move(p), r); + break; } } - void handle_new_head(Ptr&& p, int r) { + void handle_new_head(const DoutPrefixProvider *dpp, Ptr&& p, int r) { if (r == -ECANCELED) { if (p->i == MAX_RACE_RETRIES) { lderr(f->cct) << __PRETTY_FUNCTION__ << ":" << __LINE__ @@ -1537,7 +1589,7 @@ struct Pusher : public Completion { } if (p->batch.empty()) { - prep_then_push(std::move(p), 0); + prep_then_push(dpp, std::move(p), 0); return; } else { push(std::move(p)); @@ -1587,7 +1639,7 @@ void FIFO::push(const DoutPrefixProvider *dpp, const std::vector& data << " need new head tid=" << tid << dendl; p->new_head(dpp, std::move(p)); } else { - p->prep_then_push(std::move(p), 0); + p->prep_then_push(dpp, std::move(p), 0); } } -- 2.39.5