From: Matan Breizman <mbreizma@redhat.com>
Date: Mon, 17 Jul 2023 13:12:27 +0000 (+0000)
Subject: osd/OSD: remove `skip_maps` comment
X-Git-Tag: v19.0.0~199^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=05aeeeebe634213c882fb25842afb4679e6fd61d;p=ceph.git

osd/OSD: remove `skip_maps` comment

Now that oldest/newest maps are stored as an interval set we no longer
move the oldest_map forward to `first` epoch. We simply add the new osdmap range
from `first` to `last` regardless of `skip_maps`.
trim_maps now erases the oldest_map each iteration and support epoch gaps.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
---

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 93f5ca238fab..2c1d2441b769 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -7932,22 +7932,6 @@ void OSD::osdmap_subscribe(version_t epoch, bool force_request)
 
 void OSD::trim_maps(epoch_t oldest, bool skip_maps)
 {
-  /* There's a possible leak here. skip_maps is set to true if the received
-   * MOSDMap message indicates that there's a discontinuity between
-   * the Monitor cluster's stored set of maps and our set of stored
-   * maps such that there is a "gap". This happens generally when an OSD
-   * is down for a while and the cluster has trimmed maps in the mean time.
-   *
-   * Because the superblock cannot represent two discontinuous sets of maps,
-   * OSD::handle_osd_map unconditionally sets superblock.oldest_map to the first
-   * map in the message. OSD::trim_maps here, however, will only trim up to
-   * service.map_cache.cached_key_lower_bound() resulting in the maps between
-   * service.map_cache.cached_key_lower_bound() and MOSDMap::get_first() being
-   * leaked. Note, trimming past service.map_cache.cached_key_lower_bound()
-   * here won't work as there may still be PGs with those map epochs recorded.
-   *
-   * Fixing this is future work: https://tracker.ceph.com/issues/61962
-   */
   epoch_t min = std::min(oldest, service.map_cache.cached_key_lower_bound());
   dout(20) <<  __func__ << ": min=" << min << " oldest_map="
            << superblock.get_oldest_map() << " skip_maps=" << skip_maps
@@ -7980,7 +7964,8 @@ void OSD::trim_maps(epoch_t oldest, bool skip_maps)
     int tr = store->queue_transaction(service.meta_ch, std::move(t), nullptr);
     ceph_assert(tr == 0);
   }
-  // we should not remove the cached maps
+  // we should not trim past service.map_cache.cached_key_lower_bound() 
+  // as there may still be PGs with those map epochs recorded.
   ceph_assert(min <= service.map_cache.cached_key_lower_bound());
 }