rgwlc: implement NewerNoncurrentVersions

Per AWS doc, this value controls "how many noncurrent versions
Amazon S3 will retain." [1]  We understand this to mean, retain
NewerNoncurrentVersions of any object, regardless of expiration.

Removed unused RGWLifecycleConfiguration::has_same_action() (cleanup)

[1] https://docs.aws.amazon.com/AmazonS3/latest/API/API_NoncurrentVersionExpiration.html.

Signed-off-by: Matt Benjamin <mbenjamin@redhat.com>

diff --git a/src/rgw/rgw_lc.cc b/src/rgw/rgw_lc.cc
index c4888c630c36a40060e60bcce52448f0bd7bdee7..feee38d1f75c7d719c4687ff55c61125c4cb83b7 100644 (file)
--- a/src/rgw/rgw_lc.cc
+++ b/src/rgw/rgw_lc.cc
@@ -122,7 +122,9 @@ bool RGWLifecycleConfiguration::_add_rule(const LCRule& rule)
     op.expiration_date = ceph::from_iso_8601(rule.get_expiration().get_date());
   }
   if (rule.get_noncur_expiration().has_days()) {
-    op.noncur_expiration = rule.get_noncur_expiration().get_days();
+    const auto& exp = rule.get_noncur_expiration();
+    op.noncur_expiration = exp.get_days();
+    op.newer_noncurrent = exp.get_newer();
   }
   if (rule.get_mp_expiration().has_days()) {
     op.mp_expiration = rule.get_mp_expiration().get_days();
@@ -182,33 +184,6 @@ int RGWLifecycleConfiguration::check_and_add_rule(const LCRule& rule)
   return 0;
 }
 
-bool RGWLifecycleConfiguration::has_same_action(const lc_op& first,
-                                               const lc_op& second) {
-  if ((first.expiration > 0 || first.expiration_date != boost::none) && 
-    (second.expiration > 0 || second.expiration_date != boost::none)) {
-    return true;
-  } else if (first.noncur_expiration > 0 && second.noncur_expiration > 0) {
-    return true;
-  } else if (first.mp_expiration > 0 && second.mp_expiration > 0) {
-    return true;
-  } else if (!first.transitions.empty() && !second.transitions.empty()) {
-    for (auto &elem : first.transitions) {
-      if (second.transitions.find(elem.first) != second.transitions.end()) {
-        return true;
-      }
-    }
-  } else if (!first.noncur_transitions.empty() &&
-            !second.noncur_transitions.empty()) {
-    for (auto &elem : first.noncur_transitions) {
-      if (second.noncur_transitions.find(elem.first) !=
-         second.noncur_transitions.end()) {
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
 /* Formerly, this method checked for duplicate rules using an invalid
  * method (prefix uniqueness). */
 bool RGWLifecycleConfiguration::valid() 
@@ -373,13 +348,14 @@ class LCObjsLister {
   string prefix;
   vector<rgw_bucket_dir_entry>::iterator obj_iter;
   rgw_bucket_dir_entry pre_obj;
+  uint64_t num_noncurrent{0};
   int64_t delay_ms;
 
 public:
   LCObjsLister(rgw::sal::Driver* _driver, rgw::sal::Bucket* _bucket) :
       driver(_driver), bucket(_bucket) {
     list_params.list_versions = bucket->versioned();
-    list_params.allow_unordered = true;
+    list_params.allow_unordered = true; // XXX can be unconditionally true, so long as all versions of one object are assured to be on one shard and always ordered on that shard (true today in RADOS)
     delay_ms = driver->ctx()->_conf.get_val<int64_t>("rgw_lc_thread_delay");
   }
 
@@ -426,6 +402,13 @@ public:
       }
       delay();
     }
+
+    if (obj_iter->key.name == pre_obj.key.name) {
+      ++num_noncurrent;
+    } else {
+      num_noncurrent = 0; // XXX the first element must be current or a delete marker (?)
+    }
+
     /* returning address of entry in objs */
     *obj = &(*obj_iter);
     return obj_iter != list_results.objs.end();
@@ -451,6 +434,7 @@ public:
     return ((obj_iter + 1)->key.name);
   }
 
+  uint64_t get_num_noncurrent() { return num_noncurrent; }
 }; /* LCObjsLister */
 
 struct op_env {
@@ -477,6 +461,7 @@ struct lc_op_ctx {
   op_env env;
   rgw_bucket_dir_entry o;
   boost::optional<std::string> next_key_name;
+  uint64_t num_noncurrent;
   ceph::real_time effective_mtime;
 
   rgw::sal::Driver* driver;
@@ -493,10 +478,11 @@ struct lc_op_ctx {
 
   lc_op_ctx(op_env& env, rgw_bucket_dir_entry& o,
            boost::optional<std::string> next_key_name,
+           uint64_t num_noncurrent,
            ceph::real_time effective_mtime,
            const DoutPrefixProvider *dpp, WorkQ* wq)
     : cct(env.driver->ctx()), env(env), o(o), next_key_name(next_key_name),
-      effective_mtime(effective_mtime),
+      num_noncurrent(num_noncurrent), effective_mtime(effective_mtime),
       driver(env.driver), bucket(env.bucket), op(env.op), ol(env.ol),
       octx(env.driver), dpp(dpp), wq(wq)
     {
@@ -654,6 +640,7 @@ class LCOpRule {
 
   op_env env;
   boost::optional<std::string> next_key_name;
+  uint64_t num_noncurrent;
   ceph::real_time effective_mtime;
 
   std::vector<shared_ptr<LCOpFilter> > filters; // n.b., sharing ovhd
@@ -1198,10 +1185,12 @@ public:
                                      exp_time);
 
     ldpp_dout(dpp, 20) << __func__ << "(): key=" << o.key << ": is_expired="
-                     << is_expired << " "
-                     << oc.wq->thr_name() << dendl;
+                      << is_expired << " " << ": num_noncurrent="
+                      << oc.num_noncurrent
+                      << oc.wq->thr_name() << dendl;
 
     return is_expired &&
+      (oc.num_noncurrent > oc.op.newer_noncurrent) &&
       pass_object_lock_check(oc.driver, oc.obj.get(), dpp);
   }
 
@@ -1211,9 +1200,9 @@ public:
                               rgw::notify::ObjectExpirationNoncurrent);
     if (r < 0) {
       ldpp_dout(oc.dpp, 0) << "ERROR: remove_expired_obj (non-current expiration) " 
-                      << oc.bucket << ":" << o.key
-                      << " " << cpp_strerror(r)
-                      << " " << oc.wq->thr_name() << dendl;
+                          << oc.bucket << ":" << o.key
+                          << " " << cpp_strerror(r)
+                          << " " << oc.wq->thr_name() << dendl;
       return r;
     }
     if (perfcounter) {
@@ -1567,6 +1556,7 @@ void LCOpRule::build()
 void LCOpRule::update()
 {
   next_key_name = env.ol.next_key_name();
+  num_noncurrent = env.ol.get_num_noncurrent();
   effective_mtime = env.ol.get_prev_obj().meta.mtime;
 }
 
@@ -1574,7 +1564,7 @@ int LCOpRule::process(rgw_bucket_dir_entry& o,
                      const DoutPrefixProvider *dpp,
                      WorkQ* wq)
 {
-  lc_op_ctx ctx(env, o, next_key_name, effective_mtime, dpp, wq);
+  lc_op_ctx ctx(env, o, next_key_name, num_noncurrent, effective_mtime, dpp, wq);
   shared_ptr<LCOpAction> *selected = nullptr; // n.b., req'd by sharing
   real_time exp;
 

diff --git a/src/rgw/rgw_lc.h b/src/rgw/rgw_lc.h
index bd8efd9b6d03ec165924bb242451ec68846b6bed..edfb661c56b427479deab058f59219be204a1b55 100644 (file)
--- a/src/rgw/rgw_lc.h
+++ b/src/rgw/rgw_lc.h
@@ -44,26 +44,31 @@ protected:
   std::string days;
   //At present only current object has expiration date
   std::string date;
+  std::string newer_noncurrent;
 public:
   LCExpiration() {}
   LCExpiration(const std::string& _days, const std::string& _date) : days(_days), date(_date) {}
 
   void encode(bufferlist& bl) const {
-    ENCODE_START(3, 2, bl);
+    ENCODE_START(4, 2, bl);
     encode(days, bl);
     encode(date, bl);
+    encode(newer_noncurrent, bl);
     ENCODE_FINISH(bl);
   }
   void decode(bufferlist::const_iterator& bl) {
-    DECODE_START_LEGACY_COMPAT_LEN(3, 2, 2, bl);
+    DECODE_START_LEGACY_COMPAT_LEN(4, 2, 2, bl);
     decode(days, bl);
     if (struct_v >= 3) {
       decode(date, bl);
+      if (struct_v >= 4) {
+       decode(newer_noncurrent, bl);
+      }
     }
     DECODE_FINISH(bl);
   }
   void dump(Formatter *f) const;
-//  static void generate_test_instances(list<ACLOwner*>& o);
+  //  static void generate_test_instances(list<ACLOwner*>& o);
   void set_days(const std::string& _days) { days = _days; }
   std::string get_days_str() const {
     return days;
@@ -72,6 +77,11 @@ public:
   bool has_days() const {
     return !days.empty();
   }
+  void set_newer(const std::string& _newer) { newer_noncurrent = _newer; }
+  int get_newer() const {return atoi(newer_noncurrent.c_str()); }
+  bool has_newer() const {
+    return !newer_noncurrent.empty();
+  }
   void set_date(const std::string& _date) { date = _date; }
   std::string get_date() const {
     return date;
@@ -440,6 +450,7 @@ struct lc_op
   bool dm_expiration{false};
   int expiration{0};
   int noncur_expiration{0};
+  int newer_noncurrent{0};
   int mp_expiration{0};
   boost::optional<ceph::real_time> expiration_date;
   boost::optional<RGWObjTags> obj_tags;
@@ -463,7 +474,6 @@ protected:
   std::multimap<std::string, lc_op> prefix_map;
   std::multimap<std::string, LCRule> rule_map;
   bool _add_rule(const LCRule& rule);
-  bool has_same_action(const lc_op& first, const lc_op& second);
 public:
   explicit RGWLifecycleConfiguration(CephContext *_cct) : cct(_cct) {}
   RGWLifecycleConfiguration() : cct(NULL) {}

diff --git a/src/rgw/rgw_lc_s3.cc b/src/rgw/rgw_lc_s3.cc
index 1e1e16b7147f75473fad4f895f42ec4f3acba461..e0b4337e509749e9d0eb1217f104fa8e4ff4dcc3 100644 (file)
--- a/src/rgw/rgw_lc_s3.cc
+++ b/src/rgw/rgw_lc_s3.cc
@@ -64,11 +64,15 @@ void LCExpiration_S3::decode_xml(XMLObj *obj)
 
 void LCNoncurExpiration_S3::decode_xml(XMLObj *obj)
 {
+  RGWXMLDecoder::decode_xml("NewerNoncurrentVersions", newer_noncurrent, obj);
   RGWXMLDecoder::decode_xml("NoncurrentDays", days, obj, true);
 }
 
 void LCNoncurExpiration_S3::dump_xml(Formatter *f) const
 {
+  if(has_newer()) {
+    encode_xml("NewerNoncurrentVersions", newer_noncurrent, f);
+  }
   encode_xml("NoncurrentDays", days, f);
 }
 

diff --git a/src/test/rgw/test_rgw_lc.cc b/src/test/rgw/test_rgw_lc.cc
index 057deccbe207f2991cd5d150374024470762238e..f7a8c75d95aeed4e1aa15b31ed15e6153c7000aa 100644 (file)
--- a/src/test/rgw/test_rgw_lc.cc
+++ b/src/test/rgw/test_rgw_lc.cc
@@ -117,6 +117,36 @@ TEST(ExpHdr, ReplaceStrftime)
   auto exp_str = fmt::format("{:%a, %d %b %Y %T %Z}", fmt::gmtime(exp));
   std::cout << "exp_str: " << exp_str << std::endl;
   ASSERT_EQ(exp_str, "Fri, 21 Dec 2012 09:13:07 GMT");
+
+}
+
+static const char *xmldoc_4 =
+R"(<Rule>
+        <ID>noncur-cleanup-rule</ID>
+        <Filter>
+           <Prefix></Prefix>
+        </Filter>
+        <Status>Enabled</Status>
+       <NoncurrentVersionExpiration>
+            <NewerNoncurrentVersions>5</NewerNoncurrentVersions>
+            <NoncurrentDays>365</NoncurrentDays>
+       </NoncurrentVersionExpiration>
+    </Rule>
+)";
+
+TEST(TestLCConfigurationDecoder, XMLDoc4)
+{
+  RGWXMLDecoder::XMLParser parser;
+  ASSERT_TRUE(parser.init());
+  ASSERT_TRUE(parser.parse(xmldoc_4, strlen(xmldoc_4), 1));
+  LCRule_S3 rule;
+  auto result = RGWXMLDecoder::decode_xml("Rule", rule, &parser, true);
+  ASSERT_TRUE(result);
+  /* check results */
+  ASSERT_TRUE(rule.is_enabled());
+  const auto& noncur_expiration = rule.get_noncur_expiration();
+  ASSERT_EQ(noncur_expiration.get_days(), 365);
+  ASSERT_EQ(noncur_expiration.get_newer(), 5);
 }
 
 struct LCWorkTimeTests : ::testing::Test