osd: do not keep ref of old osdmap in pg

do not hold a strong reference to last_persisted_osdmap in PG. as an
OSD tries to trim previously persisted osdmaps, if they are not
referenced anymore. this helps to keep the meta collection in a
manageable size. if we advance the osdmap many times, and some PGs
are not impacted by these changes, it's very likely that they are
still holding very old osdmap references in their `last_persisted_osdmap_ref`.
this practically prevents the OSD from removing the out-dated osdmap
in OSD::handle_osd_map() if `last_persisted_osdmap_ref` is not updated
in a timely manner, for example, due to a large
"osd_pg_epoch_persisted_max_stale". so, instead of holding a reference
of last_persisted_osdmap, we can simply remember its epoch.

See also: #13990

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index ffbd6ff8f0502a22704904cb9f783f8829755dc0..0788dbc870cb9898ecd40ad30fc49b72804ad460 100644 (file)
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -185,7 +185,7 @@ PG::PG(OSDService *o, OSDMapRef curmap,
     _pool.id,
     p.shard),
   map_lock("PG::map_lock"),
-  osdmap_ref(curmap), last_persisted_osdmap_ref(curmap), pool(_pool),
+  osdmap_ref(curmap), pool(_pool),
   _lock("PG::_lock"),
   ref(0),
   #ifdef PG_DEBUG_REFS
@@ -223,7 +223,8 @@ PG::PG(OSDService *o, OSDMapRef curmap,
   acting_features(CEPH_FEATURES_SUPPORTED_DEFAULT),
   upacting_features(CEPH_FEATURES_SUPPORTED_DEFAULT),
   do_sort_bitwise(false),
-  last_epoch(0)
+  last_epoch(0),
+  last_persisted_epoch(curmap->get_epoch())
 {
 #ifdef PG_DEBUG_REFS
   osd->add_pgid(p, this);
@@ -2788,7 +2789,7 @@ void PG::prepare_write_info(map<string,bufferlist> *km)
   assert(ret == 0);
   if (need_update_epoch)
     last_epoch = get_osdmap()->get_epoch();
-  last_persisted_osdmap_ref = osdmap_ref;
+  last_persisted_epoch = last_epoch;
 
   dirty_info = false;
   dirty_big_info = false;
@@ -5421,15 +5422,15 @@ void PG::handle_activate_map(RecoveryCtx *rctx)
   dout(10) << "handle_activate_map " << dendl;
   ActMap evt;
   recovery_state.handle_event(evt, rctx);
-  if (osdmap_ref->get_epoch() - last_persisted_osdmap_ref->get_epoch() >
+  if (osdmap_ref->get_epoch() - last_persisted_epoch >
     cct->_conf->osd_pg_epoch_persisted_max_stale) {
     dout(20) << __func__ << ": Dirtying info: last_persisted is "
-            << last_persisted_osdmap_ref->get_epoch()
+            << last_persisted_epoch
             << " while current is " << osdmap_ref->get_epoch() << dendl;
     dirty_info = true;
   } else {
     dout(20) << __func__ << ": Not dirtying info: last_persisted is "
-            << last_persisted_osdmap_ref->get_epoch()
+            << last_persisted_epoch
             << " while current is " << osdmap_ref->get_epoch() << dendl;
   }
   if (osdmap_ref->check_new_blacklist_entries()) check_blacklisted_watchers();

diff --git a/src/osd/PG.h b/src/osd/PG.h
index 5ab8d59d57f30e7d7a71c6f9137d46ae6c83d57b..1fb7920d193bb56848676a71f5742360c631d371 100644 (file)
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -206,7 +206,6 @@ protected:
   Mutex map_lock;
   list<OpRequestRef> waiting_for_map;
   OSDMapRef osdmap_ref;
-  OSDMapRef last_persisted_osdmap_ref;
   PGPool pool;
 
   void queue_op(OpRequestRef& op);
@@ -2056,6 +2055,7 @@ public:
 
   bool do_sort_bitwise;
   epoch_t last_epoch;
+  epoch_t last_persisted_epoch;
 
  public:
   const spg_t&      get_pgid() const { return pg_id; }