osd: do not mark pg clean until active is durable

Do not mark a PG CLEAN or set last_epoch_clean until after the PG activate is stable on all replicas. This effectively means that last_epoch_clean will never fall in an interval that follows last_epoch_started's interval. It *can* be > last_epoch_started when it falls within the same interval. Signed-off-by: Sage Weil <[email protected]>
mitsu-ko · May 5, 2012 · 8ec476e · 8ec476e
1 parent 86aa07d
commit 8ec476e
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 16 deletions.
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
@@ -5065,22 +5065,22 @@ void OSD::do_recovery(PG *pg)
 	recovery_wq.lock();
 	pg->recovery_item.remove_myself();	// sigh...
 	recovery_wq.unlock();
-
       }
     }
 
     do_notifies(notify_list, pg->get_osdmap()->get_epoch());  // notify? (residual|replica)
     do_queries(query_map);
     do_infos(info_map);
 
+    pg->write_if_dirty(*t);
+
     if (!t->empty()) {
       int tr = store->queue_transaction(&pg->osr, t, new ObjectStore::C_DeleteTransaction(t), fin);
       assert(tr == 0);
     } else {
       delete t;
       delete fin;
     }
-
     pg->unlock();
   }
   pg->put();

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
@@ -1521,9 +1521,17 @@ void PG::all_activated_and_committed()
   assert(peer_activated.size() == acting.size());
 
   info.history.last_epoch_started = get_osdmap()->get_epoch();
-  share_pg_info();
-
   dirty_info = true;
+
+  // make sure CLEAN is marked if we've been clean in this interval
+  if (info.last_complete == info.last_update &&
+      !state_test(PG_STATE_BACKFILL) &&
+      !state_test(PG_STATE_RECOVERING)) {
+    mark_clean();
+  }
+
+  share_pg_info();
+  update_stats();
 }
 
 void PG::queue_snap_trim()
@@ -1555,30 +1563,38 @@ struct C_PG_FinishRecovery : public Context {
   }
 };
 
-void PG::finish_recovery(ObjectStore::Transaction& t, list<Context*>& tfin)
+void PG::mark_clean()
 {
-  dout(10) << "finish_recovery" << dendl;
-  state_clear(PG_STATE_BACKFILL);
-  state_clear(PG_STATE_RECOVERING);
-
   // only mark CLEAN if we have the desired number of replicas AND we
   // are not remapped.
   if (acting.size() == get_osdmap()->get_pg_size(info.pgid) &&
       up == acting)
     state_set(PG_STATE_CLEAN);
 
-  assert(info.last_complete == info.last_update);
-
   // NOTE: this is actually a bit premature: we haven't purged the
   // strays yet.
   info.history.last_epoch_clean = get_osdmap()->get_epoch();
-  share_pg_info();
-
-  clear_recovery_state();
 
   trim_past_intervals();
-
-  write_info(t);
+
+  dirty_info = true;
+}
+
+void PG::finish_recovery(ObjectStore::Transaction& t, list<Context*>& tfin)
+{
+  dout(10) << "finish_recovery" << dendl;
+  assert(info.last_complete == info.last_update);
+
+  state_clear(PG_STATE_BACKFILL);
+  state_clear(PG_STATE_RECOVERING);
+
+  // only mark CLEAN if last_epoch_started is already stable.
+  if (info.history.last_epoch_started >= info.history.same_interval_since) {
+    mark_clean();
+    share_pg_info();
+  }
+
+  clear_recovery_state();
 
   /*
    * sync all this before purging strays.  but don't block!

diff --git a/src/osd/PG.h b/src/osd/PG.h
@@ -638,6 +638,7 @@ class PG {
 
   bool needs_recovery() const;
 
+  void mark_clean();  ///< mark an active pg clean
   void generate_past_intervals();
   void trim_past_intervals();
   void build_prior(std::auto_ptr<PriorSet> &prior_set);