]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
cls/fifo: Retry on push to nonexistent part 49682/head
authorAdam C. Emerson <aemerson@redhat.com>
Wed, 30 Nov 2022 00:41:40 +0000 (19:41 -0500)
committerAdam C. Emerson <aemerson@redhat.com>
Tue, 10 Jan 2023 00:34:45 +0000 (19:34 -0500)
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 <aemerson@redhat.com>
(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 <aemerson@redhat.com>
src/rgw/cls_fifo_legacy.cc

index 6c1357b201b5b021facac3db7a028a742bbcbdd7..8e15091116c8efc2000a0ee6f18d0346ed677cfd 100644 (file)
@@ -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<cb::list>& 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<cb::list>& 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<Pusher> {
   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<Pusher> {
   }
 
   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<Pusher> {
        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<Pusher> {
        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<Pusher> {
     }
 
     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<cb::list>& 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);
   }
 }