Merge pull request ceph#62535 from Matan-B/wip-matanb-crimson-replicated-missing

osd/../client_request: Fix clone replicated reads

Reviewed-by: Samuel Just <[email protected]>

Author: Matan-B

src/crimson/osd/ops_executer.cc

@@ -903,6 +903,7 @@ pg_log_entry_t OpsExecuter::prepare_head_update(
 {
   LOG_PREFIX(OpsExecuter::prepare_head_update);
   assert(obc->obs.oi.soid.snap >= CEPH_MAXSNAP);
+  assert(obc->obs.oi.soid.is_head());
 
   update_clone_overlap();
   if (cloning_ctx) {

src/crimson/osd/osd_operations/client_request.cc

@@ -197,13 +197,16 @@ ClientRequest::interruptible_future<> ClientRequest::with_pg_process_interruptib
     }
 
     pg.get_perf_logger().inc(l_osd_replica_read);
-    if (pg.is_unreadable_object(m->get_hobj())) {
-      DEBUGDPP("{}.{}: {} missing on replica, bouncing to primary",
-	       pg, *this, this_instance_id, m->get_hobj());
+    if (pg.is_missing_head_and_clones(m->get_hobj())) {
+      DEBUGDPP("{}.{}: {} possibly missing head or clone object on replica,"
+               " bouncing to primary",
+               pg, *this, this_instance_id, m->get_hobj());
       pg.get_perf_logger().inc(l_osd_replica_read_redirect_missing);
       co_await reply_op_error(pgref, -EAGAIN);
       co_return;
     } else if (!pg.get_peering_state().can_serve_replica_read(m->get_hobj())) {
+      // Note: can_serve_replica_read checks for writes on the head object
+      //       as writes can only occur to head.
       DEBUGDPP("{}.{}: unstable write on replica, bouncing to primary",
 	       pg, *this, this_instance_id);
       pg.get_perf_logger().inc(l_osd_replica_read_redirect_conflict);
@@ -353,7 +356,8 @@ ClientRequest::process_op(
     }
 
     std::set<snapid_t> snaps = snaps_need_to_recover();
-    if (!snaps.empty()) {
+    if (!snaps.empty() &&
+        pg->is_missing_head_and_clones(m->get_hobj().get_head())) {
       co_await recover_missing_snaps(pg, snaps);
     }
   }

src/crimson/osd/pg.cc

@@ -1511,6 +1511,10 @@ void PG::context_registry_on_change() {
   }
 }
 
+bool PG::is_missing_head_and_clones(const hobject_t &hoid) {
+  return peering_state.is_missing_any_head_or_clone_of(hoid);
+}
+
 bool PG::can_discard_op(const MOSDOp& m) const {
   if (m.get_map_epoch() <
       peering_state.get_info().history.same_primary_since) {

src/crimson/osd/pg.h

@@ -952,6 +952,10 @@ class PG : public boost::intrusive_ref_counter<
       !peering_state.get_missing_loc().readable_with_acting(
 	oid, get_actingset(), v);
   }
+
+  // check if any head or clone of this object is missing
+  bool is_missing_head_and_clones(const hobject_t &hoid);
+
   bool is_missing_on_peer(
     const pg_shard_t &peer,
     const hobject_t &soid) const {

src/osd/PeeringState.h

@@ -2441,6 +2441,10 @@ class PeeringState : public MissingLoc::MappingInfo {
   unsigned int get_num_missing() const {
     return pg_log.get_missing().num_missing();
   }
+  bool is_missing_any_head_or_clone_of(const hobject_t &hoid) {
+    const auto& missing = pg_log.get_missing();
+    return missing.is_missing_any_head_or_clone_of(hoid);
+  }
 
   const MissingLoc &get_missing_loc() const {
     return missing_loc;

src/osd/osd_types.h

@@ -4997,6 +4997,12 @@ class pg_missing_set : public pg_missing_const_i {
       return false;
     return true;
   }
+
+  bool is_missing_any_head_or_clone_of(const hobject_t& oid) const {
+    return missing.lower_bound(oid.get_object_boundary()) !=
+      missing.lower_bound(oid.get_max_object_boundary());
+  }
+
   eversion_t get_oldest_need() const {
     if (missing.empty()) {
       return eversion_t();

src/test/osd/types.cc

@@ -1435,6 +1435,35 @@ TEST(pg_missing_t, split_into)
   EXPECT_TRUE(missing.is_missing(oid2));
 }
 
+TEST(pg_missing_t, is_missing_any_head_or_clone_of)
+{
+  hobject_t head_oid(object_t("objname"), "key", 123, 456, 0, "");
+  auto clone_oid = head_oid;
+  clone_oid.snap = 1;
+
+  // empty missing
+  pg_missing_t missing;
+  EXPECT_FALSE(missing.is_missing(head_oid));
+  EXPECT_FALSE(missing.is_missing_any_head_or_clone_of(head_oid));
+  EXPECT_FALSE(missing.is_missing(clone_oid));
+  EXPECT_FALSE(missing.is_missing_any_head_or_clone_of(clone_oid));
+
+  // only head is missing
+  missing.add(head_oid, eversion_t(), eversion_t(), false);
+  EXPECT_TRUE(missing.is_missing(head_oid));
+  EXPECT_TRUE(missing.is_missing_any_head_or_clone_of(head_oid));
+  EXPECT_FALSE(missing.is_missing(clone_oid));
+  EXPECT_TRUE(missing.is_missing_any_head_or_clone_of(clone_oid));
+
+  // only clone is missing
+  pg_missing_t missing2;
+  missing2.add(clone_oid, eversion_t(), eversion_t(), false);
+  EXPECT_FALSE(missing2.is_missing(head_oid));
+  EXPECT_TRUE(missing2.is_missing_any_head_or_clone_of(head_oid));
+  EXPECT_TRUE(missing2.is_missing(clone_oid));
+  EXPECT_TRUE(missing2.is_missing_any_head_or_clone_of(clone_oid));
+}
+
 TEST(pg_pool_t_test, get_pg_num_divisor) {
   pg_pool_t p;
   p.set_pg_num(16);