From d7eb8ce54d5eb3971b6b2e1095c69b5fd6c1b1c2 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 12 Feb 2010 13:26:19 -0800 Subject: [PATCH] osd: always update up_thru if pg changes before going active We already required this if prior PG members were down, so this affected the 'failure' case. We now also require it for non-failure PG changes (expansion, migration). This fixes our maybe_went_rw calculation for prior PG intervals, which is based on up_thru. If maybe_went_rw is false when the pg actually went rw, we can lose (and have lost) data. But it is not practical to calculate without up_thru being consistently updated, because determining whether a pg would have been able to go active depends on knowing last_epoch_started at a previous point in time, which then determines how many prior intervals may have been considered, which in turn determines whether up_thru would have been updated, etc. Much simpler to update it all the time. This should not impose a significantly greater cost, since we already need it for the failure case. And in general the migration/expansion/whatever case is no more common nor critical than the failure case. --- src/osd/PG.cc | 7 ++----- src/osd/PG.h | 3 +-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 87e1745f1e5bb..1034278faa28c 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -806,7 +806,6 @@ void PG::clear_prior() prior_set.clear(); prior_set_down.clear(); prior_set_up_thru.clear(); - must_notify_mon = false; } @@ -937,8 +936,7 @@ void PG::build_prior() prior_set_down.insert(o); } else { dout(10) << "build_prior prior osd" << o - << " is down, must notify mon" << dendl; - must_notify_mon = true; + << " is down" << dendl; need_down++; prior_set_down.insert(o); } @@ -976,7 +974,6 @@ void PG::build_prior() << (is_crashed() ? " crashed":"") << (is_down() ? " down":"") << (some_down ? " some_down":"") - << (must_notify_mon ? " must_notify_mon":"") << dendl; } @@ -1350,7 +1347,7 @@ void PG::peer(ObjectStore::Transaction& t, list& tfin, assert(info.last_complete >= log.tail || log.backlog); // -- do need to notify the monitor? - if (must_notify_mon) { + if (true) { if (osd->osdmap->get_up_thru(osd->whoami) < info.history.same_acting_since) { dout(10) << "up_thru " << osd->osdmap->get_up_thru(osd->whoami) << " < same_since " << info.history.same_acting_since diff --git a/src/osd/PG.h b/src/osd/PG.h index 6d457376e2cbc..d085e449b640d 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -711,7 +711,6 @@ public: set prior_set; // current+prior OSDs, as defined by info.history.last_epoch_started. set prior_set_down; // down osds exluded from prior_set map prior_set_up_thru; // osds whose up_thru we care about - bool must_notify_mon; bool need_up_thru; set stray_set; // non-acting osds that have PG data. set uptodate_set; // current OSDs that are uptodate @@ -855,7 +854,7 @@ public: role(0), state(0), have_master_log(true), - must_notify_mon(false), need_up_thru(false), + need_up_thru(false), pg_stats_lock("PG::pg_stats_lock"), pg_stats_valid(false), finish_sync_event(NULL) -- 2.39.5