Merge pull request ceph#56612 from Matan-B/wip-matanb-remove-snapset-snaps

osd: cleanup Snapset::snaps

Author: SrinivasaBharath

src/crimson/osd/ops_executer.cc

@@ -911,7 +911,6 @@ pg_log_entry_t OpsExecuter::prepare_head_update(
   if (snapc.seq > obc->ssc->snapset.seq) {
      // update snapset with latest snap context
      obc->ssc->snapset.seq = snapc.seq;
-     obc->ssc->snapset.snaps.clear();
   }
 
   pg_log_entry_t ret{

src/osd/PrimaryLogPG.cc

@@ -4908,15 +4908,6 @@ int PrimaryLogPG::trim_object(
     head_obc->obs.oi = object_info_t(head_oid);
     t->remove(head_oid);
   } else {
-    if (get_osdmap()->require_osd_release < ceph_release_t::octopus) {
-      // filter SnapSet::snaps for the benefit of pre-octopus
-      // peers. This is perhaps overly conservative in that I'm not
-      // certain they need this, but let's be conservative here.
-      dout(10) << coid << " filtering snapset on " << head_oid << dendl;
-      snapset.filter(pool.info);
-    } else {
-      snapset.snaps.clear();
-    }
     dout(10) << coid << " writing updated snapset on " << head_oid
 	     << ", snapset is " << snapset << dendl;
     ctx->log.push_back(
@@ -8794,11 +8785,6 @@ void PrimaryLogPG::make_writeable(OpContext *ctx)
   if (snapc.seq > ctx->new_snapset.seq) {
     // update snapset with latest snap context
     ctx->new_snapset.seq = snapc.seq;
-    if (get_osdmap()->require_osd_release < ceph_release_t::octopus) {
-      ctx->new_snapset.snaps = snapc.snaps;
-    } else {
-      ctx->new_snapset.snaps.clear();
-    }
   }
   dout(20) << "make_writeable " << soid
 	   << " done, snapset=" << ctx->new_snapset << dendl;
@@ -10223,11 +10209,6 @@ void PrimaryLogPG::finish_promote(int r, CopyResults *results,
 
     OpContextUPtr tctx = simple_opc_create(obc);
     tctx->at_version = get_next_version();
-    if (get_osdmap()->require_osd_release < ceph_release_t::octopus) {
-      filter_snapc(tctx->new_snapset.snaps);
-    } else {
-      tctx->new_snapset.snaps.clear();
-    }
     vector<snapid_t> new_clones;
     map<snapid_t, vector<snapid_t>> new_clone_snaps;
     for (vector<snapid_t>::iterator i = tctx->new_snapset.clones.begin();
@@ -10899,17 +10880,7 @@ int PrimaryLogPG::start_flush(
 	   << " " << (blocking ? "blocking" : "non-blocking/best-effort")
 	   << dendl;
 
-  bool preoctopus_compat =
-    get_osdmap()->require_osd_release < ceph_release_t::octopus;
-  SnapSet snapset;
-  if (preoctopus_compat) {
-    // for pre-octopus compatibility, filter SnapSet::snaps.  not
-    // certain we need this, but let's be conservative.
-    snapset = obc->ssc->snapset.get_filtered(pool.info);
-  } else {
-    // NOTE: change this to a const ref when we remove this compat code
-    snapset = obc->ssc->snapset;
-  }
+  const SnapSet& snapset = obc->ssc->snapset;
 
   if ((obc->obs.oi.has_manifest() && obc->obs.oi.manifest.is_chunked())
       || force_dedup) {
@@ -10922,7 +10893,7 @@ int PrimaryLogPG::start_flush(
   // verify there are no (older) check for dirty clones
   {
     dout(20) << " snapset " << snapset << dendl;
-    vector<snapid_t>::reverse_iterator p = snapset.clones.rbegin();
+    vector<snapid_t>::const_reverse_iterator p = snapset.clones.rbegin();
     while (p != snapset.clones.rend() && *p >= soid.snap)
       ++p;
     if (p != snapset.clones.rend()) {
@@ -11033,7 +11004,7 @@ int PrimaryLogPG::start_flush(
     }
 
     snapid_t prev_snapc = 0;
-    for (vector<snapid_t>::reverse_iterator citer = snapset.clones.rbegin();
+    for (vector<snapid_t>::const_reverse_iterator citer = snapset.clones.rbegin();
 	 citer != snapset.clones.rend();
 	 ++citer) {
       if (*citer < soid.snap) {

src/osd/osd_types.cc

@@ -5886,6 +5886,7 @@ void SnapSet::encode(ceph::buffer::list& bl) const
   ENCODE_START(3, 2, bl);
   encode(seq, bl);
   encode(true, bl);  // head_exists
+  std::vector<snapid_t> snaps;
   encode(snaps, bl);
   encode(clones, bl);
   encode(clone_overlap, bl);
@@ -5899,6 +5900,7 @@ void SnapSet::decode(ceph::buffer::list::const_iterator& bl)
   DECODE_START_LEGACY_COMPAT_LEN(3, 2, 2, bl);
   decode(seq, bl);
   bl += 1u;  // skip legacy head_exists (always true)
+  std::vector<snapid_t> snaps;
   decode(snaps, bl);
   decode(clones, bl);
   decode(clone_overlap, bl);
@@ -5946,12 +5948,8 @@ void SnapSet::generate_test_instances(list<SnapSet*>& o)
   o.push_back(new SnapSet);
   o.push_back(new SnapSet);
   o.back()->seq = 123;
-  o.back()->snaps.push_back(123);
-  o.back()->snaps.push_back(12);
   o.push_back(new SnapSet);
   o.back()->seq = 123;
-  o.back()->snaps.push_back(123);
-  o.back()->snaps.push_back(12);
   o.back()->clones.push_back(12);
   o.back()->clone_size[12] = 12345;
   o.back()->clone_overlap[12];
@@ -5960,8 +5958,7 @@ void SnapSet::generate_test_instances(list<SnapSet*>& o)
 
 ostream& operator<<(ostream& out, const SnapSet& cs)
 {
-  return out << cs.seq << "=" << cs.snaps << ":"
-	     << cs.clone_snaps;
+  return out << cs.seq << "=" << cs.clone_snaps;
 }
 
 void SnapSet::from_snap_set(const librados::snap_set_t& ss, bool legacy)
@@ -5999,13 +5996,6 @@ void SnapSet::from_snap_set(const librados::snap_set_t& ss, bool legacy)
   clones.reserve(_clones.size());
   for (auto p = _clones.begin(); p != _clones.end(); ++p)
     clones.push_back(*p);
-
-  // descending
-  snaps.clear();
-  snaps.reserve(_snaps.size());
-  for (auto p = _snaps.rbegin();
-       p != _snaps.rend(); ++p)
-    snaps.push_back(*p);
 }
 
 uint64_t SnapSet::get_clone_bytes(snapid_t clone) const
@@ -6018,23 +6008,6 @@ uint64_t SnapSet::get_clone_bytes(snapid_t clone) const
   return size - overlap.size();
 }
 
-void SnapSet::filter(const pg_pool_t &pinfo)
-{
-  vector<snapid_t> oldsnaps;
-  oldsnaps.swap(snaps);
-  for (auto i = oldsnaps.cbegin(); i != oldsnaps.cend(); ++i) {
-    if (!pinfo.is_removed_snap(*i))
-      snaps.push_back(*i);
-  }
-}
-
-SnapSet SnapSet::get_filtered(const pg_pool_t &pinfo) const
-{
-  SnapSet ss = *this;
-  ss.filter(pinfo);
-  return ss;
-}
-
 // -- watch_info_t --
 
 void watch_info_t::encode(ceph::buffer::list& bl, uint64_t features) const

src/osd/osd_types.h

@@ -5660,8 +5660,6 @@ inline std::ostream& operator<<(std::ostream& out, const OSDSuperblock& sb)
  */
 struct SnapSet {
   snapid_t seq;
-  // NOTE: this is for pre-octopus compatibility only! remove in Q release
-  std::vector<snapid_t> snaps;    // descending
   std::vector<snapid_t> clones;   // ascending
   std::map<snapid_t, interval_set<uint64_t> > clone_overlap;  // overlap w/ next newest
   std::map<snapid_t, uint64_t> clone_size;
@@ -5699,9 +5697,6 @@ struct SnapSet {
     return out;
   }
 
-
-  SnapSet get_filtered(const pg_pool_t &pinfo) const;
-  void filter(const pg_pool_t &pinfo);
 };
 WRITE_CLASS_ENCODER(SnapSet)
 

src/osd/osd_types_fmt.h

@@ -253,9 +253,8 @@ struct formatter<SnapSet> {
 
     } else {
       return fmt::format_to(ctx.out(),
-			    "{}={}:{}",
+			    "{}={}",
 			    snps.seq,
-			    snps.snaps,
 			    snps.clone_snaps);
     }
   }

src/test/osd/scrubber_generators.cc

@@ -34,7 +34,7 @@ std::pair<bufferlist, std::vector<snapid_t>> create_object_snapset(
   encode(sns, bl);
 
   // extract the set of object snaps
-  return {bl, sns.snaps};
+  return {bl, sns.clones};
 }
 
 RealObjsConfList ScrubGenerator::make_real_objs_conf(

src/test/osd/scrubber_generators.h

@@ -133,7 +133,6 @@ struct SnapsetMockData {
   using clone_snaps_cooker = CookedCloneSnaps (*)();
 
   snapid_t seq;
-  std::vector<snapid_t> snaps;	 // descending
   std::vector<snapid_t> clones;	 // ascending
 
   std::map<snapid_t, interval_set<uint64_t>> clone_overlap;  // overlap w/ next
@@ -143,25 +142,21 @@ struct SnapsetMockData {
 
 
   SnapsetMockData(snapid_t seq,
-		  std::vector<snapid_t> snaps,
 		  std::vector<snapid_t> clones,
 		  std::map<snapid_t, interval_set<uint64_t>> clone_overlap,
 		  std::map<snapid_t, uint64_t> clone_size,
 		  std::map<snapid_t, std::vector<snapid_t>> clone_snaps)
       : seq(seq)
-      , snaps(snaps)
       , clones(clones)
       , clone_overlap(clone_overlap)
       , clone_size(clone_size)
       , clone_snaps(clone_snaps)
   {}
 
   SnapsetMockData(snapid_t seq,
-		  std::vector<snapid_t> snaps,
 		  std::vector<snapid_t> clones,
 		  clone_snaps_cooker func)
       : seq{seq}
-      , snaps{snaps}
       , clones(clones)
   {
     auto [clone_size_, clone_snaps_, clone_overlap_] = func();
@@ -174,7 +169,6 @@ struct SnapsetMockData {
   {
     SnapSet ss;
     ss.seq = seq;
-    ss.snaps = snaps;
     ss.clones = clones;
     ss.clone_overlap = clone_overlap;
     ss.clone_size = clone_size;
@@ -257,10 +251,10 @@ struct fmt::formatter<ScrubGenerator::RealObj> {
   {
     using namespace ScrubGenerator;
     return fmt::format_to(ctx.out(),
-			  "RealObj(gh:{}, dt:{}, snaps:{})",
+			  "RealObj(gh:{}, dt:{}, clones:{})",
 			  rlo.ghobj,
 			  rlo.data.size,
-			  (rlo.snapset_mock_data ? rlo.snapset_mock_data->snaps
+			  (rlo.snapset_mock_data ? rlo.snapset_mock_data->clones
 						 : std::vector<snapid_t>{}));
   }
 };

src/test/osd/scrubber_test_datasets.cc

@@ -56,7 +56,6 @@ SnapsetMockData::CookedCloneSnaps ms1_fn()
 }
 
 static SnapsetMockData hobj_ms1_snapset{/* seq */ 0x40,
-					/* snaps */ {0x30, 0x20},
 					/* clones */ {0x20, 0x30},
 					ms1_fn};