osd: Handle recovery read errors

Fixes: ceph#9304 Signed-off-by: David Zafman <dzafman@redhat.com>
dzafman · May 25, 2016 · de97801 · de97801
1 parent f86fd4c
commit de97801
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 3 deletions.
diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
@@ -188,6 +188,22 @@ PGBackend::RecoveryHandle *ECBackend::open_recovery_op()
   return new ECRecoveryHandle;
 }
 
+void ECBackend::handle_recovery_read_errors(const hobject_t &hoid,
+  pair<RecoveryMessages *, ECBackend::read_result_t &> &in)
+{
+  ECBackend::read_result_t &res = in.second;
+  dout(10) << __func__ << ": Read error " << hoid << " r="
+	   << res.r << " errors=" << res.errors << dendl;
+  //assert(!op.recovery_progress.first);
+  dout(10) << __func__ << ": canceling recovery op for obj " << hoid
+	   << dendl;
+  //get_parent()->cancel_pull(hoid);
+  assert(recovery_ops.count(hoid));
+  recovery_ops.erase(hoid);
+
+  get_parent()->recovery_read_fail(hoid);
+}
+
 struct OnRecoveryReadComplete :
   public GenContext<pair<RecoveryMessages*, ECBackend::read_result_t& > &> {
   ECBackend *pg;
@@ -197,9 +213,10 @@ struct OnRecoveryReadComplete :
     : pg(pg), hoid(hoid) {}
   void finish(pair<RecoveryMessages *, ECBackend::read_result_t &> &in) {
     ECBackend::read_result_t &res = in.second;
-    // FIXME???
-    assert(res.r == 0);
-    assert(res.errors.empty());
+    if (!(res.r == 0 && res.errors.empty())) {
+        pg->handle_recovery_read_errors(hoid, in);
+        return;
+    }
     assert(res.returned.size() == 1);
     pg->handle_recovery_read_complete(
       hoid,
@@ -1071,6 +1088,7 @@ void ECBackend::handle_sub_read_reply(
   unsigned is_complete = 0;
   // For redundant reads check for completion as each shard comes in,
   // or in a non-recovery read check for completion once all the shards read.
+  // FIXME: Remove for_recovery check and see if it works
   if (rop.do_redundant_reads || (!rop.for_recovery && rop.in_progress.empty())) {
     for (map<hobject_t, read_result_t>::const_iterator iter =
         rop.complete.begin();

diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h
@@ -504,6 +504,8 @@ class ECBackend : public PGBackend {
   uint64_t be_get_ondisk_size(uint64_t logical_size) {
     return sinfo.logical_to_next_chunk_offset(logical_size);
   }
+  void handle_recovery_read_errors(const hobject_t &hoid,
+    pair<RecoveryMessages *, ECBackend::read_result_t &> &in);
 };
 
 #endif
diff --git a/src/osd/PG.h b/src/osd/PG.h
@@ -371,6 +371,9 @@ class PG : DoutPrefixProvider {
     void remove_location(const hobject_t &hoid, pg_shard_t location) {
       missing_loc[hoid].erase(location);
     }
+    void remove_all_locations(const hobject_t &hoid) {
+      missing_loc[hoid].clear();
+    }
     void add_active_missing(const pg_missing_t &missing) {
       for (map<hobject_t, pg_missing_t::item, hobject_t::BitwiseComparator>::const_iterator i =
 	     missing.missing.begin();

diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
@@ -125,6 +125,7 @@ typedef ceph::shared_ptr<const OSDMap> OSDMapRef;
      virtual const set<pg_shard_t> &get_actingbackfill_shards() const = 0;
      virtual const set<pg_shard_t> &get_acting_shards() const = 0;
      virtual const set<pg_shard_t> &get_backfill_shards() const = 0;
+     virtual void recovery_read_fail(const hobject_t &hoid) = 0;
 
      virtual std::string gen_dbg_prefix() const = 0;
 

diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
@@ -339,6 +339,13 @@ class ReplicatedPG : public PG, public PGBackend::Listener {
     return backfill_targets;
   }
 
+  void recovery_read_fail(const hobject_t &hoid) {
+    assert(recovering.count(hoid));
+    recovering.erase(hoid);
+    missing_loc.remove_all_locations(hoid);
+    finish_recovery_op(hoid);  // close out this attempt,
+  }
+
   std::string gen_dbg_prefix() const { return gen_prefix(); }
 
   const map<hobject_t, set<pg_shard_t>, hobject_t::BitwiseComparator> &get_missing_loc_shards() const {