]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw: Wait until a generation has been empty for an hour to delete
authorAdam C. Emerson <aemerson@redhat.com>
Wed, 10 Feb 2021 21:18:09 +0000 (16:18 -0500)
committerAdam C. Emerson <aemerson@redhat.com>
Mon, 5 Apr 2021 17:48:19 +0000 (13:48 -0400)
This fixes a problem where, while the backing handle remains allocated
while a call completes, the objects it depends on may be deleted
behind it.

Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
(cherry picked from commit 7018c25d47edf7e12b581f7f28c2549fe73bde15)
Signed-off-by: Adam C. Emerson <aemerson@redhat.com>
src/rgw/rgw_datalog.cc
src/rgw/rgw_log_backing.cc
src/rgw/rgw_log_backing.h
src/test/rgw/test_log_backing.cc

index 0b68c45a13e8d2a475b03dee6c3cbf108697e735..184d0713fb2a99b8bc91b3b0ee7cd55c62911813 100644 (file)
@@ -390,7 +390,7 @@ bs::error_code DataLogBackends::handle_init(entries_t e) noexcept {
   std::unique_lock l(m);
 
   for (const auto& [gen_id, gen] : e) {
-    if (gen.empty) {
+    if (gen.pruned) {
       lderr(datalog.cct)
        << __PRETTY_FUNCTION__ << ":" << __LINE__
        << ": ERROR: given empty generation: gen_id=" << gen_id << dendl;
index 67fc9255869191f08ffb655628233e01efc4b9d3..8ce88aa21414fcf7b023c287fc88beef00cc7670 100644 (file)
@@ -583,7 +583,7 @@ bs::error_code logback_generations::empty_to(uint64_t gen_id,
       }
       for (auto i = es.begin(); i < ei; ++i) {
        newtail = i->first;
-       i->second.empty = true;
+       i->second.pruned = ceph::real_clock::now();
       }
       ec = write(std::move(es), std::move(l), y);
       ++tries;
@@ -626,31 +626,44 @@ bs::error_code logback_generations::remove_empty(optional_yield y) noexcept {
     entries_t new_entries;
     std::unique_lock l(m);
     ceph_assert(!entries_.empty());
-    auto i = lowest_nomempty(entries_);
-    if (i == entries_.begin()) {
-      return {};
+    {
+      auto i = lowest_nomempty(entries_);
+      if (i == entries_.begin()) {
+       return {};
+      }
     }
-    auto ln = i->first;
     entries_t es;
-    std::copy(entries_.cbegin(), i,
-             std::inserter(es, es.end()));
+    auto now = ceph::real_clock::now();
     l.unlock();
     do {
+      std::copy_if(entries_.cbegin(), entries_.cend(),
+                  std::inserter(es, es.end()),
+                  [now](const auto& e) {
+                    if (!e.second.pruned)
+                      return false;
+
+                    auto pruned = *e.second.pruned;
+                    return (now - pruned) >= 1h;
+                  });
+      auto es2 = entries_;
       for (const auto& [gen_id, e] : es) {
-       ceph_assert(e.empty);
+       ceph_assert(e.pruned);
        auto ec = log_remove(ioctx, shards,
                             [this, gen_id](int shard) {
                               return this->get_oid(gen_id, shard);
                             }, (gen_id == 0), y);
        if (ec) {
-         return ec;
+         lderr(cct) << __PRETTY_FUNCTION__ << ":" << __LINE__
+                    << ": Error pruning: gen_id=" << gen_id
+                    << " ec=" << ec.message() << dendl;
+       }
+       if (auto i = es2.find(gen_id); i != es2.end()) {
+         es2.erase(i);
        }
       }
       l.lock();
-      i = entries_.find(ln);
       es.clear();
-      std::copy(i, entries_.cend(), std::inserter(es, es.end()));
-      ec = write(std::move(es), std::move(l), y);
+      ec = write(std::move(es2), std::move(l), y);
       ++tries;
     } while (ec == bs::errc::operation_canceled &&
             tries < max_tries);
index e592bc29b2bcf18e69447f14a43429b8185ac967..d5996049e58739ccd46e5bf9cbe2e3e4f6f187c6 100644 (file)
@@ -95,13 +95,13 @@ bs::error_code log_remove(librados::IoCtx& ioctx,
 struct logback_generation {
   uint64_t gen_id = 0;
   log_type type;
-  bool empty = false;
+  std::optional<ceph::real_time> pruned;
 
   void encode(ceph::buffer::list& bl) const {
     ENCODE_START(1, 1, bl);
     encode(gen_id, bl);
     encode(type, bl);
-    encode(empty, bl);
+    encode(pruned, bl);
     ENCODE_FINISH(bl);
   }
 
@@ -109,7 +109,7 @@ struct logback_generation {
     DECODE_START(1, bl);
     decode(gen_id, bl);
     decode(type, bl);
-    decode(empty, bl);
+    decode(pruned, bl);
     DECODE_FINISH(bl);
   }
 };
@@ -157,7 +157,7 @@ private:
   auto lowest_nomempty(const entries_t& es) {
     return std::find_if(es.begin(), es.end(),
                        [](const auto& e) {
-                         return !e.second.empty;
+                         return !e.second.pruned;
                        });
   }
 
index 166de2dd8242c840a4166ec1345ef6315c200d2a..95f1e613936b01fb6e439ac9fe81c1acf0f5c3d2 100644 (file)
@@ -241,7 +241,7 @@ TEST_F(LogBacking, GenerationSingle)
 
   ASSERT_EQ(0, lg->got_entries[0].gen_id);
   ASSERT_EQ(log_type::fifo, lg->got_entries[0].type);
-  ASSERT_FALSE(lg->got_entries[0].empty);
+  ASSERT_FALSE(lg->got_entries[0].pruned);
 
   auto ec = lg->empty_to(0, null_yield);
   ASSERT_TRUE(ec);
@@ -258,7 +258,7 @@ TEST_F(LogBacking, GenerationSingle)
 
   ASSERT_EQ(0, lg->got_entries[0].gen_id);
   ASSERT_EQ(log_type::fifo, lg->got_entries[0].type);
-  ASSERT_FALSE(lg->got_entries[0].empty);
+  ASSERT_FALSE(lg->got_entries[0].pruned);
 
   lg->got_entries.clear();
 
@@ -268,7 +268,7 @@ TEST_F(LogBacking, GenerationSingle)
   ASSERT_EQ(1, lg->got_entries.size());
   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].empty);
+  ASSERT_FALSE(lg->got_entries[1].pruned);
 
   lg.reset();
 
@@ -280,11 +280,11 @@ TEST_F(LogBacking, GenerationSingle)
   ASSERT_EQ(2, lg->got_entries.size());
   ASSERT_EQ(0, lg->got_entries[0].gen_id);
   ASSERT_EQ(log_type::fifo, lg->got_entries[0].type);
-  ASSERT_FALSE(lg->got_entries[0].empty);
+  ASSERT_FALSE(lg->got_entries[0].pruned);
 
   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].empty);
+  ASSERT_FALSE(lg->got_entries[1].pruned);
 
   ec = lg->empty_to(0, null_yield);
   ASSERT_FALSE(ec);
@@ -301,7 +301,7 @@ TEST_F(LogBacking, GenerationSingle)
   ASSERT_EQ(1, lg->got_entries.size());
   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].empty);
+  ASSERT_FALSE(lg->got_entries[1].pruned);
 
   ec = lg->remove_empty(null_yield);
   ASSERT_FALSE(ec);
@@ -311,7 +311,7 @@ TEST_F(LogBacking, GenerationSingle)
 
   ASSERT_EQ(1, entries[1].gen_id);
   ASSERT_EQ(log_type::omap, entries[1].type);
-  ASSERT_FALSE(entries[1].empty);
+  ASSERT_FALSE(entries[1].pruned);
 
   lg.reset();
 }
@@ -329,7 +329,7 @@ TEST_F(LogBacking, GenerationWN)
   ASSERT_EQ(1, lg1->got_entries.size());
   ASSERT_EQ(1, lg1->got_entries[1].gen_id);
   ASSERT_EQ(log_type::omap, lg1->got_entries[1].type);
-  ASSERT_FALSE(lg1->got_entries[1].empty);
+  ASSERT_FALSE(lg1->got_entries[1].pruned);
 
   lg1->got_entries.clear();
 
@@ -342,11 +342,11 @@ TEST_F(LogBacking, GenerationWN)
 
   ASSERT_EQ(0, lg2->got_entries[0].gen_id);
   ASSERT_EQ(log_type::fifo, lg2->got_entries[0].type);
-  ASSERT_FALSE(lg2->got_entries[0].empty);
+  ASSERT_FALSE(lg2->got_entries[0].pruned);
 
   ASSERT_EQ(1, lg2->got_entries[1].gen_id);
   ASSERT_EQ(log_type::omap, lg2->got_entries[1].type);
-  ASSERT_FALSE(lg2->got_entries[1].empty);
+  ASSERT_FALSE(lg2->got_entries[1].pruned);
 
   lg2->got_entries.clear();
 
@@ -356,12 +356,12 @@ TEST_F(LogBacking, GenerationWN)
   ASSERT_EQ(1, lg1->got_entries.size());
   ASSERT_EQ(2, lg1->got_entries[2].gen_id);
   ASSERT_EQ(log_type::fifo, lg1->got_entries[2].type);
-  ASSERT_FALSE(lg1->got_entries[2].empty);
+  ASSERT_FALSE(lg1->got_entries[2].pruned);
 
   ASSERT_EQ(1, lg2->got_entries.size());
   ASSERT_EQ(2, lg2->got_entries[2].gen_id);
   ASSERT_EQ(log_type::fifo, lg2->got_entries[2].type);
-  ASSERT_FALSE(lg2->got_entries[2].empty);
+  ASSERT_FALSE(lg2->got_entries[2].pruned);
 
   lg1->got_entries.clear();
   lg2->got_entries.clear();