crimson/osd: make PGAdvanceMap idempotent

PGAdvanceMap is scheduled by two independent callers: pg creation
(do_init=true, to=current_epoch) and broadcast_map_to_pgs. They do
not coordinate, so a broadcast advance can race with an init advance
that has already pushed the pg past the broadcast's target. The op
carried a std::optional<from> that was overwritten at start-time,
guarded by ceph_assert(from <= to) which fires in this race.

The "from" parameter was never really an input. It was always
re-read inside the pipeline from pg->get_osdmap_epoch(); the value
passed in (when there was one) was discarded. Drop the member and
let the op contract be: "ensure pg has processed osdmaps up to at
least 'to'". If the pg is already past 'to', skip. This matches
OSD::advance_pg() in the classic OSD and removes the need to
serialize the two callers.

Fixes: https://tracker.ceph.com/issues/61744
Signed-off-by: Kefu Chai <k.chai@proxmox.com>

diff --git a/src/crimson/osd/osd_operations/pg_advance_map.cc b/src/crimson/osd/osd_operations/pg_advance_map.cc
index 6e01e2751edf7dd68defa165bcf2f3afc1c7346f..6ae061904dec85da491ebf481fc23960b41f71a3 100644 (file)
--- a/src/crimson/osd/osd_operations/pg_advance_map.cc
+++ b/src/crimson/osd/osd_operations/pg_advance_map.cc
@@ -35,7 +35,6 @@ void PGAdvanceMap::print(std::ostream &lhs) const
 {
   lhs << "PGAdvanceMap("
       << "pg=" << pg->get_pgid()
-      << " from=" << (from ? *from : -1)
       << " to=" << to;
   if (do_init) {
     lhs << " do_init";
@@ -47,9 +46,6 @@ void PGAdvanceMap::dump_detail(Formatter *f) const
 {
   f->open_object_section("PGAdvanceMap");
   f->dump_stream("pgid") << pg->get_pgid();
-  if (from) {
-    f->dump_int("from", *from);
-  }
   f->dump_int("to", to);
   f->dump_bool("do_init", do_init);
   f->close_section();
@@ -77,23 +73,13 @@ seastar::future<> PGAdvanceMap::start()
       return seastar::now();
     }
 
-    /*
-     * PGAdvanceMap is scheduled at pg creation and when
-     * broadcasting new osdmaps to pgs. We are not able to serialize
-     * between the two different PGAdvanceMap callers since a new pg
-     * will get advanced to the latest osdmap at it's creation.
-     * As a result, we may need to adjust the PGAdvance operation
-     * 'from' epoch.
-     * See: https://tracker.ceph.com/issues/61744
-     */
-    from = pg->get_osdmap_epoch();
+    epoch_t from = pg->get_osdmap_epoch();
     if (do_init) {
       pg->handle_initialize(rctx);
       pg->handle_activate_map(rctx);
     }
-    ceph_assert(std::cmp_less_equal(*from, to));
     return seastar::do_for_each(
-      boost::make_counting_iterator(*from + 1),
+      boost::make_counting_iterator(from + 1),
       boost::make_counting_iterator(to + 1),
       [this, FNAME](epoch_t next_epoch) {
        DEBUG("{}: start: getting map {}",

diff --git a/src/crimson/osd/osd_operations/pg_advance_map.h b/src/crimson/osd/osd_operations/pg_advance_map.h
index 67c56e62d2d3ede846d8082beaede6fd39020da8..43b57cd8c28bb677f5b54fb5b5c0c746666f4ec7 100644 (file)
--- a/src/crimson/osd/osd_operations/pg_advance_map.h
+++ b/src/crimson/osd/osd_operations/pg_advance_map.h
@@ -31,7 +31,6 @@ protected:
   ShardServices &shard_services;
   PipelineHandle handle;
 
-  std::optional<epoch_t> from;
   epoch_t to;
 
   PeeringCtx rctx;