crimson: remove CommonClientRequest, move do_recover_missing to PG

do_recover_missing was the only thing left, and inheriting from a class
to get a static method is somewhat confusing.  Simply move
do_recover_missing to PG.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/osd/CMakeLists.txt b/src/crimson/osd/CMakeLists.txt
index 5796500080eaade411977a9cb3e2e683a9ff3a7a..2832c9d864b6b26e7d5f60ee2c1f4c0f362399fd 100644 (file)
--- a/src/crimson/osd/CMakeLists.txt
+++ b/src/crimson/osd/CMakeLists.txt
@@ -19,7 +19,6 @@ add_executable(crimson-osd
   ops_executer.cc
   osd_operation.cc
   osd_operations/client_request.cc
-  osd_operations/client_request_common.cc
   osd_operations/internal_client_request.cc
   osd_operations/peering_event.cc
   osd_operations/pg_advance_map.cc

diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc
index e97525f9ad13bf3da31a4374c4ac9b108ac1d06b..b29cf29c5ed4204a93244a35c60a00df7f4126d8 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -321,7 +321,7 @@ ClientRequest::recover_missing_snaps(
   co_await std::move(resolve_oids);
 
   for (auto &oid : ret) {
-    auto unfound = co_await do_recover_missing(pg, oid, m->get_reqid());
+    auto unfound = co_await pg->do_recover_missing(oid, m->get_reqid());
     if (unfound) {
       DEBUGDPP("{} unfound, hang it for now", *pg, oid);
       co_await interruptor::make_interruptible(
@@ -347,8 +347,8 @@ ClientRequest::process_op(
       "Skipping recover_missings on non primary pg for soid {}",
       *pg, m->get_hobj());
   } else {
-    auto unfound = co_await do_recover_missing(
-      pg, m->get_hobj().get_head(), m->get_reqid());
+    auto unfound = co_await pg->do_recover_missing(
+      m->get_hobj().get_head(), m->get_reqid());
     if (unfound) {
       DEBUGDPP("{} unfound, hang it for now", *pg, m->get_hobj().get_head());
       co_await interruptor::make_interruptible(

diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h
index c1ea50acc35369330c340e6bb48f466c40dfe183..bdc3ff0d241597424f86845663d46fb589e76b2e 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.h
+++ b/src/crimson/osd/osd_operations/client_request.h
@@ -15,7 +15,6 @@
 #include "crimson/osd/object_context_loader.h"
 #include "crimson/osd/osdmap_gate.h"
 #include "crimson/osd/osd_operation.h"
-#include "crimson/osd/osd_operations/client_request_common.h"
 #include "crimson/osd/pg_activation_blocker.h"
 #include "crimson/osd/pg_map.h"
 #include "crimson/osd/scrub/pg_scrubber.h"
@@ -28,8 +27,7 @@ class PG;
 class OSD;
 class ShardServices;
 
-class ClientRequest final : public PhasedOperationT<ClientRequest>,
-                            private CommonClientRequest {
+class ClientRequest final : public PhasedOperationT<ClientRequest> {
   // Initially set to primary core, updated to pg core after with_pg()
   ShardServices *shard_services = nullptr;
 

diff --git a/src/crimson/osd/osd_operations/client_request_common.cc b/src/crimson/osd/osd_operations/client_request_common.cc
deleted file mode 100644 (file)
index 06b9d4f..0000000
--- a/src/crimson/osd/osd_operations/client_request_common.cc
+++ /dev/null
@@ -1,80 +0,0 @@
-// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:nil -*-
-// vim: ts=8 sw=2 smarttab expandtab
-
-#include "crimson/common/coroutine.h"
-#include "crimson/common/log.h"
-#include "crimson/osd/osd_operations/client_request_common.h"
-#include "crimson/osd/pg.h"
-#include "crimson/osd/osd_operations/background_recovery.h"
-
-SET_SUBSYS(osd);
-
-namespace crimson::osd {
-
-typename InterruptibleOperation::template interruptible_future<bool>
-CommonClientRequest::do_recover_missing(
-  Ref<PG> pg,
-  const hobject_t& soid,
-  const osd_reqid_t& reqid)
-{
-  LOG_PREFIX(CommonCLientRequest::do_recover_missing);
-  DEBUGDPP(
-    "reqid {} check for recovery, {}",
-    *pg, reqid, soid);
-  assert(pg->is_primary());
-  eversion_t ver;
-  auto &peering_state = pg->get_peering_state();
-  auto &missing_loc = peering_state.get_missing_loc();
-  bool needs_recovery_or_backfill = false;
-
-  if (pg->is_unreadable_object(soid)) {
-    DEBUGDPP(
-      "reqid {}, {} is unreadable",
-      *pg, reqid, soid);
-    ceph_assert(missing_loc.needs_recovery(soid, &ver));
-    needs_recovery_or_backfill = true;
-  }
-
-  if (pg->is_degraded_or_backfilling_object(soid)) {
-    DEBUGDPP(
-      "reqid {}, {} is degraded or backfilling",
-      *pg, reqid, soid);
-    if (missing_loc.needs_recovery(soid, &ver)) {
-      needs_recovery_or_backfill = true;
-    }
-  }
-
-  if (!needs_recovery_or_backfill) {
-    DEBUGDPP(
-      "reqid {} nothing to recover {}",
-      *pg, reqid, soid);
-    co_return false;
-  }
-
-  if (pg->get_peering_state().get_missing_loc().is_unfound(soid)) {
-    co_return true;
-  }
-  DEBUGDPP(
-    "reqid {} need to wait for recovery, {} version {}",
-    *pg, reqid, soid);
-  if (pg->get_recovery_backend()->is_recovering(soid)) {
-    DEBUGDPP(
-      "reqid {} object {} version {}, already recovering",
-      *pg, reqid, soid, ver);
-    co_await PG::interruptor::make_interruptible(
-      pg->get_recovery_backend()->get_recovering(
-       soid).wait_for_recovered());
-    co_return false;
-  } else {
-    DEBUGDPP(
-      "reqid {} object {} version {}, starting recovery",
-      *pg, reqid, soid, ver);
-    auto [op, fut] =
-      pg->get_shard_services().start_operation<UrgentRecovery>(
-        soid, ver, pg, pg->get_shard_services(), pg->get_osdmap_epoch());
-    co_await PG::interruptor::make_interruptible(std::move(fut));
-    co_return false;
-  }
-}
-
-} // namespace crimson::osd

diff --git a/src/crimson/osd/osd_operations/client_request_common.h b/src/crimson/osd/osd_operations/client_request_common.h
deleted file mode 100644 (file)
index 4c3cf42..0000000
--- a/src/crimson/osd/osd_operations/client_request_common.h
+++ /dev/null
@@ -1,21 +0,0 @@
-// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
-// vim: ts=8 sw=2 smarttab
-
-#pragma once
-
-#include "crimson/common/operation.h"
-#include "crimson/common/type_helpers.h"
-#include "crimson/osd/osd_operation.h"
-
-namespace crimson::osd {
-
-struct CommonClientRequest {
-
-  static InterruptibleOperation::template interruptible_future<bool>
-  do_recover_missing(
-    Ref<PG> pg,
-    const hobject_t& soid,
-    const osd_reqid_t& reqid);
-};
-
-} // namespace crimson::osd

diff --git a/src/crimson/osd/osd_operations/internal_client_request.cc b/src/crimson/osd/osd_operations/internal_client_request.cc
index b8f7646bc7433659ced647d737f5ca85bd709516..e486b62ad7dc48773f5259c8ab2c9123490402ff 100644 (file)
--- a/src/crimson/osd/osd_operations/internal_client_request.cc
+++ b/src/crimson/osd/osd_operations/internal_client_request.cc
@@ -64,8 +64,8 @@ InternalClientRequest::with_interruption()
 
   co_await enter_stage<interruptor>(obc_orderer->obc_pp().process);
 
-  bool unfound = co_await do_recover_missing(
-    pg, get_target_oid(), osd_reqid_t());
+  bool unfound = co_await pg->do_recover_missing(
+    get_target_oid(), osd_reqid_t());
 
   if (unfound) {
     throw std::system_error(

diff --git a/src/crimson/osd/osd_operations/internal_client_request.h b/src/crimson/osd/osd_operations/internal_client_request.h
index 1cfde4ab080ecafd4afa6e41324ae031a1cdb49b..c4c98bfcec1cf0c9ea50f23334267aaf99d03971 100644 (file)
--- a/src/crimson/osd/osd_operations/internal_client_request.h
+++ b/src/crimson/osd/osd_operations/internal_client_request.h
@@ -6,14 +6,12 @@
 #include "crimson/common/type_helpers.h"
 #include "crimson/osd/object_context_loader.h"
 #include "crimson/osd/osd_operation.h"
-#include "crimson/osd/osd_operations/client_request_common.h"
 #include "crimson/osd/pg.h"
 #include "crimson/osd/pg_activation_blocker.h"
 
 namespace crimson::osd {
 
-class InternalClientRequest : public PhasedOperationT<InternalClientRequest>,
-                              private CommonClientRequest {
+class InternalClientRequest : public PhasedOperationT<InternalClientRequest> {
 public:
   explicit InternalClientRequest(Ref<PG> pg);
   ~InternalClientRequest();

diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index cb0689d2e31eb029160d37b2cd3a36f6a75849f6..155ca2d3565aa3806419e3e09730b958dfeb7e6a 100644 (file)
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -660,6 +660,69 @@ void PG::on_active_advmap(const OSDMapRef &osdmap)
   }
 }
 
+PG::interruptible_future<bool> PG::do_recover_missing(
+  const hobject_t& soid,
+  const osd_reqid_t& reqid)
+{
+  LOG_PREFIX(PG::do_recover_missing);
+  DEBUGDPP(
+    "reqid {} check for recovery, {}",
+    *this, reqid, soid);
+  assert(is_primary());
+  eversion_t ver;
+  auto &missing_loc = peering_state.get_missing_loc();
+  bool needs_recovery_or_backfill = false;
+
+  if (is_unreadable_object(soid)) {
+    DEBUGDPP(
+      "reqid {}, {} is unreadable",
+      *this, reqid, soid);
+    ceph_assert(missing_loc.needs_recovery(soid, &ver));
+    needs_recovery_or_backfill = true;
+  }
+
+  if (is_degraded_or_backfilling_object(soid)) {
+    DEBUGDPP(
+      "reqid {}, {} is degraded or backfilling",
+      *this, reqid, soid);
+    if (missing_loc.needs_recovery(soid, &ver)) {
+      needs_recovery_or_backfill = true;
+    }
+  }
+
+  if (!needs_recovery_or_backfill) {
+    DEBUGDPP(
+      "reqid {} nothing to recover {}",
+      *this, reqid, soid);
+    co_return false;
+  }
+
+  if (peering_state.get_missing_loc().is_unfound(soid)) {
+    co_return true;
+  }
+  DEBUGDPP(
+    "reqid {} need to wait for recovery, {} version {}",
+    *this, reqid, soid);
+  if (recovery_backend->is_recovering(soid)) {
+    DEBUGDPP(
+      "reqid {} object {} version {}, already recovering",
+      *this, reqid, soid, ver);
+    co_await PG::interruptor::make_interruptible(
+      recovery_backend->get_recovering(
+       soid).wait_for_recovered());
+    co_return false;
+  } else {
+    DEBUGDPP(
+      "reqid {} object {} version {}, starting recovery",
+      *this, reqid, soid, ver);
+    auto [op, fut] =
+      shard_services.start_operation<UrgentRecovery>(
+        soid, ver, this, shard_services, get_osdmap_epoch());
+    co_await PG::interruptor::make_interruptible(std::move(fut));
+    co_return false;
+  }
+}
+
 void PG::scrub_requested(scrub_level_t scrub_level, scrub_type_t scrub_type)
 {
   /* We don't actually route the scrub request message into the state machine.

diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index a8cc6eefa792b4f73c9468e3d2f1f5b29693f279..42a283098dcdd99747c3fbb5551c55a3296fa893 100644 (file)
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -756,6 +756,10 @@ private:
 public:
   PeeringState peering_state;
 
+  interruptible_future<bool> do_recover_missing(
+    const hobject_t& soid,
+    const osd_reqid_t& reqid);
+
   // scrub state
 
   friend class ScrubScan;