From: Sage Weil Date: Thu, 4 Dec 2008 20:18:07 +0000 (-0800) Subject: osd: some scrub fixes X-Git-Tag: v0.6~185 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ee18c91c9ed60657a5fdb6734431a8966b2c685d;p=ceph.git osd: some scrub fixes Don't drop locks just yet; atm this leaves the dout() prefix exposed to concurrent modifications of pg state. Don't requeue for scrub if already scrubbing. Fix missing object detection bugs. --- diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 585a7011783a..f86db67529d2 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -1610,7 +1610,7 @@ void OSD::handle_scrub(MOSDScrub *m) p != pg_map.end(); p++) { PG *pg = p->second; - if (pg->is_primary()) + if (pg->is_primary() && !pg->is_scrubbing()) scrub_wq.queue(pg); } } else { @@ -1619,7 +1619,7 @@ void OSD::handle_scrub(MOSDScrub *m) p++) if (pg_map.count(*p)) { PG *pg = pg_map[*p]; - if (pg->is_primary()) + if (pg->is_primary() && !pg->is_scrubbing()) scrub_wq.queue(pg); } } @@ -2881,16 +2881,16 @@ void OSD::handle_pg_scrub(MOSDPGScrub *m) } else { if (pg->is_primary()) { if (pg->peer_scrub_map.count(from)) { - dout(10) << "handle_pg_scrub got peer osd" << from << " scrub map -- had it already" << dendl; + dout(10) << *pg << " already had osd" << from << " scrub map" << dendl; } else { - dout(10) << "handle_pg_scrub got peer osd" << from << " scrub map" << dendl; + dout(10) << *pg << " got osd" << from << " scrub map" << dendl; bufferlist::iterator p = m->map.begin(); pg->peer_scrub_map[from].decode(p); pg->kick(); } } else { // replica, reply - dout(10) << "handle_pg_scrub generating scrub map for primary" << dendl; + dout(10) << *pg << " building scrub map for primary" << dendl; // do this is a separate thread.. FIXME ScrubMap map; @@ -3322,7 +3322,7 @@ void OSD::handle_op(MOSDOp *op) } // scrubbing? - if (pg->state_test(PG_STATE_SCRUBBING)) { + if (pg->is_scrubbing()) { dout(10) << *pg << " is scrubbing, deferring op " << *op << dendl; pg->waiting_for_active.push_back(op); pg->unlock(); diff --git a/src/osd/PG.cc b/src/osd/PG.cc index a58c2cdeeb70..5252073e44fe 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -1734,7 +1734,7 @@ void PG::build_scrub_map(ScrubMap &map) osd->store->collection_list(c, ls); // sort - dout(10) << "sorting" << dendl; + dout(10) << "sorting " << ls.size() << " objects" << dendl; vector< pair > tab(ls.size()); vector< pair >::iterator q = tab.begin(); int i = 0; @@ -1754,7 +1754,7 @@ void PG::build_scrub_map(ScrubMap &map) pos[p->second] = i; // now, pos[orig pos] = sorted pos - dout(10) << " " << ls.size() << " objects" << dendl; + dout(10) << " scanning " << ls.size() << " objects" << dendl; map.objects.resize(ls.size()); i = 0; for (vector::iterator p = ls.begin(); @@ -1780,7 +1780,7 @@ void PG::build_scrub_map(ScrubMap &map) // log osd->store->read(coll_t(), info.pgid.to_log_pobject(), 0, 0, map.logbl); - dout(10) << " log " << map.logbl.length() << " bytes" << dendl; + dout(10) << " done. pg log is " << map.logbl.length() << " bytes" << dendl; } @@ -1826,22 +1826,24 @@ void PG::scrub() wait(); } - unlock(); + //unlock(); - dout(10) << "scrub building my map" << dendl; + dout(10) << "scrub building my map" << dendl; ScrubMap scrubmap; build_scrub_map(scrubmap); + /* lock(); if (epoch != info.history.same_since) { dout(10) << "scrub pg changed, aborting" << dendl; unlock(); return; } + */ while (peer_scrub_map.size() < acting.size() - 1) { - dout(10) << " have " << (peer_scrub_map.size()+1) << " / " << acting.size() - << " scrub maps, waiting" << dendl; + dout(10) << "scrub has " << (peer_scrub_map.size()+1) << " / " << acting.size() + << " maps, waiting" << dendl; wait(); if (epoch != info.history.same_since) { @@ -1851,117 +1853,134 @@ void PG::scrub() } } + /* unlock(); + */ - // first, compare scrub maps - vector m(acting.size()); - m[0] = &scrubmap; - for (unsigned i=1; i::iterator p[acting.size()]; - for (unsigned i=0; iobjects.begin(); - - int num_missing = 0; - int num_bad = 0; - - while (1) { - ScrubMap::object *po = 0; - bool missing = false; - for (unsigned i=0; iobjects.end()) - continue; - if (!po) - po = &(*p[i]); - else if (po->poid != p[i]->poid) { - missing = true; - if (po->poid > p[i]->poid) - po = &(*p[i]); - } - } - if (!po) - break; - if (missing) { - for (unsigned i=0; ipoid != p[i]->poid) { - dout(0) << " osd" << acting[i] << " missing " << po->poid << dendl; - num_missing++; - } else - p[i]++; - } - continue; - } + if (acting.size() > 1) { + dout(10) << "scrub comparing replica scrub maps" << dendl; - // compare - bool ok = true; - for (unsigned i=1; isize != p[i]->size) { - dout(0) << " osd" << acting[i] << " " << po->poid - << " size " << p[i]->size << " != " << po->size << dendl; - ok = false; - num_bad++; + // first, compare scrub maps + vector m(acting.size()); + m[0] = &scrubmap; + for (unsigned i=1; i::iterator p[acting.size()]; + for (unsigned i=0; iobjects.begin(); + + int num_missing = 0; + int num_bad = 0; + + while (1) { + ScrubMap::object *po = 0; + bool missing = false; + for (unsigned i=0; iobjects.end()) { + missing = true; + continue; + } + if (!po) + po = &(*p[i]); + else if (po->poid != p[i]->poid) { + missing = true; + if (po->poid > p[i]->poid) + po = &(*p[i]); + } } - if (po->attrs.size() != p[i]->attrs.size()) { - dout(0) << " osd" << acting[i] << " " << po->poid - << " attr count " << p[i]->attrs.size() << " != " << po->attrs.size() << dendl; - ok = false; - num_bad++; + if (!po) + break; + if (missing) { + for (unsigned i=0; iobjects.end() || po->poid != p[i]->poid) { + dout(0) << "scrub osd" << acting[i] << " missing " << po->poid << dendl; + num_missing++; + } else + p[i]++; + } + continue; } - for (map::iterator q = po->attrs.begin(); q != po->attrs.end(); q++) { - if (p[i]->attrs.count(q->first)) { - if (q->second.cmp(p[i]->attrs[q->first])) { - dout(0) << " osd" << acting[i] << " " << po->poid - << " attr " << q->first << " value mismatch" << dendl; + + // compare + dout(10) << " po is " << (void*)po << dendl; + dout(10) << " po is " << po->poid << dendl; + + bool ok = true; + for (unsigned i=1; isize != p[i]->size) { + dout(0) << "scrub osd" << acting[i] << " " << po->poid + << " size " << p[i]->size << " != " << po->size << dendl; + ok = false; + num_bad++; + } + if (po->attrs.size() != p[i]->attrs.size()) { + dout(0) << "scrub osd" << acting[i] << " " << po->poid + << " attr count " << p[i]->attrs.size() << " != " << po->attrs.size() << dendl; + ok = false; + num_bad++; + } + for (map::iterator q = po->attrs.begin(); q != po->attrs.end(); q++) { + if (p[i]->attrs.count(q->first)) { + if (q->second.cmp(p[i]->attrs[q->first])) { + dout(0) << "scrub osd" << acting[i] << " " << po->poid + << " attr " << q->first << " value mismatch" << dendl; + ok = false; + num_bad++; + } + } else { + dout(0) << "scrub osd" << acting[i] << " " << po->poid + << " attr " << q->first << " missing" << dendl; ok = false; num_bad++; } - } else { - dout(0) << " osd" << acting[i] << " " << po->poid - << " attr " << q->first << " missing" << dendl; - ok = false; - num_bad++; } } + + if (ok) + dout(20) << "scrub " << po->poid << " size " << po->size << " ok" << dendl; + + // next + for (unsigned i=0; iget_logclient()->log(LOG_ERROR, s); } - - if (ok) - dout(10) << "scrub " << po->poid << " size " << po->size << " ok" << dendl; - - // next - for (unsigned i=0; iget_logclient()->log(LOG_ERROR, s); } + /* lock(); if (epoch != info.history.same_since) { dout(10) << "scrub pg changed, aborting" << dendl; unlock(); return; } + */ // discard peer scrub info. peer_scrub_map.clear(); + /* unlock(); - + */ + // ok, do the pg-type specific scrubbing _scrub(scrubmap); + /* lock(); if (epoch != info.history.same_since) { dout(10) << "scrub pg changed, aborting" << dendl; unlock(); return; } + */ // finish up info.stats.last_scrub = info.last_update; diff --git a/src/osd/PG.h b/src/osd/PG.h index 37cd5f827963..0e92507bc67a 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -506,6 +506,7 @@ public: _lock.Unlock(); } void wait() { + assert(_lock.is_locked()); _cond.Wait(_lock); } void kick() { @@ -754,6 +755,8 @@ public: bool is_clean() const { return state_test(PG_STATE_CLEAN); } bool is_stray() const { return state_test(PG_STATE_STRAY); } + bool is_scrubbing() const { return state_test(PG_STATE_SCRUBBING); } + bool is_empty() const { return info.last_update == eversion_t(0,0); } bool is_complete_pg() { return acting.size() == info.pgid.size(); }