crimson/osd: fold the split-child setup into handle_split_pg_creation

A readability cleanup with no behaviour change.

split_pg()'s loop still did all the per-child setup inline (core
mapping, make_pg, split_colls, split_into, the snapmapper touch) and
then called handle_split_pg_creation() to kick off the child's
PGAdvanceMap. That is a lot of detail for what the loop is really doing.

So let's move that setup into handle_split_pg_creation() and have it
return the child PG. The loop then just asks it to create each child and
collects the result, and the per-child PeeringCtx never has to leave the
function. Children are still created one at a time, each with its own
PeeringCtx.

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 730f27765cace60e22f9158fe456d0ddadedc73e..d4cc12847e2a9bae7d2127e8806bb1e8bc49d93f 100644 (file)
--- a/src/crimson/osd/osd_operations/pg_advance_map.cc
+++ b/src/crimson/osd/osd_operations/pg_advance_map.cc
@@ -160,42 +160,40 @@ seastar::future<> PGAdvanceMap::split_pg(
   // be applied in order. 
   for (auto child_pgid : split_children) {
     children_pgids.insert(child_pgid);
-    PeeringCtx child_rctx;
-
-    // Map each child pg ID to a core
-    auto core = co_await shard_services.create_split_pg_mapping(child_pgid, seastar::this_shard_id(), pg->get_store_index());
-    DEBUG(" PG {} mapped to {}", child_pgid.pgid, core);
-    DEBUG(" {} map epoch: {}", child_pgid.pgid, pg_epoch);
-    auto map = next_map;
-    auto child_pg = co_await shard_services.make_pg(std::move(map), child_pgid, pg->get_store_index(), true);
-
-    DEBUG(" Parent pgid: {}", pg->get_pgid());
-    DEBUG(" Child pgid: {}", child_pg->get_pgid());
-    unsigned new_pg_num = next_map->get_pg_num(pg->get_pgid().pool());
-    unsigned split_bits = child_pg->get_pgid().get_split_bits(new_pg_num);
-    DEBUG(" pg num is {}, m_seed is {}, split bits is {}",
-       new_pg_num, child_pg->get_pgid().ps(), split_bits);
-
-    co_await pg->split_colls(child_pg->get_pgid(), split_bits, child_pg->get_pgid().ps(),
-                             &child_pg->get_pgpool().info, child_rctx.transaction);
-    DEBUG(" {} split collection done", child_pg->get_pgid());
-    pg->split_into(child_pg->get_pgid().pgid, child_pg, split_bits);
-    auto child_coll_ref = child_pg->get_collection_ref();
-    child_rctx.transaction.touch(child_coll_ref->get_cid(), child_pg->get_pgid().make_snapmapper_oid());
-
-    co_await handle_split_pg_creation(child_pg, next_map, std::move(child_rctx));
+    auto child_pg = co_await handle_split_pg_creation(child_pgid, next_map);
     split_pgs.insert(child_pg);
   }
 
   split_stats(split_pgs, children_pgids);
 }
 
-seastar::future<> PGAdvanceMap::handle_split_pg_creation(
-    Ref<PG> child_pg,
-    cached_map_t next_map,
-    PeeringCtx child_rctx)
+seastar::future<Ref<PG>> PGAdvanceMap::handle_split_pg_creation(
+    spg_t child_pgid,
+    cached_map_t next_map)
 {
   LOG_PREFIX(PGAdvanceMap::handle_split_pg_creation);
+
+  // Map each child pg ID to a core
+  auto core = co_await shard_services.create_split_pg_mapping(child_pgid, seastar::this_shard_id(), pg->get_store_index());
+  DEBUG(" PG {} mapped to {}", child_pgid.pgid, core);
+  DEBUG(" {} map epoch: {}", child_pgid.pgid, next_map->get_epoch());
+  auto child_pg = co_await shard_services.make_pg(next_map, child_pgid, pg->get_store_index(), true);
+
+  DEBUG(" Parent pgid: {}", pg->get_pgid());
+  DEBUG(" Child pgid: {}", child_pg->get_pgid());
+  unsigned new_pg_num = next_map->get_pg_num(pg->get_pgid().pool());
+  unsigned split_bits = child_pg->get_pgid().get_split_bits(new_pg_num);
+  DEBUG(" pg num is {}, m_seed is {}, split bits is {}",
+       new_pg_num, child_pg->get_pgid().ps(), split_bits);
+
+  PeeringCtx child_rctx;
+  co_await pg->split_colls(child_pg->get_pgid(), split_bits, child_pg->get_pgid().ps(),
+                           &child_pg->get_pgpool().info, child_rctx.transaction);
+  DEBUG(" {} split collection done", child_pg->get_pgid());
+  pg->split_into(child_pg->get_pgid().pgid, child_pg, split_bits);
+  auto child_coll_ref = child_pg->get_collection_ref();
+  child_rctx.transaction.touch(child_coll_ref->get_cid(), child_pg->get_pgid().make_snapmapper_oid());
+
   // We must create a new Trigger instance for each pg.
   // The BlockingEvent object which tracks whether a pg creation is complete
   // or still blocking, shouldn't be used across multiple pgs so we can track
@@ -220,6 +218,7 @@ seastar::future<> PGAdvanceMap::handle_split_pg_creation(
       child_pg, shard_services, next_map->get_epoch(),
       std::move(child_rctx), true).second;
   co_await std::move(fut);
+  co_return child_pg;
 }
 
 

diff --git a/src/crimson/osd/osd_operations/pg_advance_map.h b/src/crimson/osd/osd_operations/pg_advance_map.h
index a8c554f64c2d4d60d371ac459aedb2006b219e4b..6dbb4a974c2fb592ddb03b569561f4a9330b5091 100644 (file)
--- a/src/crimson/osd/osd_operations/pg_advance_map.h
+++ b/src/crimson/osd/osd_operations/pg_advance_map.h
@@ -70,10 +70,9 @@ public:
 
 private:
   PGPeeringPipeline &peering_pp(PG &pg);
-  seastar::future<> handle_split_pg_creation(
-    Ref<PG> child_pg,
-    cached_map_t next_map,
-    PeeringCtx child_rctx);
+  seastar::future<Ref<PG>> handle_split_pg_creation(
+    spg_t child_pgid,
+    cached_map_t next_map);
 };
 
 }