From: Greg Farnum <greg@inktank.com>
Date: Tue, 14 Jan 2014 22:56:31 +0000 (-0800)
Subject: osd: do not misuse calc_pg_role
X-Git-Tag: v0.78~329^2~24
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=268ae82ac3fe7b541c450aae168dcf9fa7294508;p=ceph.git

osd: do not misuse calc_pg_role

We've been using the role returned from this to determine if we're
the primary or not. Don't.
This is mostly about removing a few asserts; while in there I also
redirected some calls to use static dereference instead of going through
the osdmap lookup path.

Signed-off-by: Greg Farnum <greg@inktank.com>
---

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 6d46a91cb1ab..f192f479c2ec 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1713,7 +1713,7 @@ void OSD::add_newly_split_pg(PG *pg, PG::RecoveryCtx *rctx)
   dout(10) << "Adding newly split pg " << *pg << dendl;
   vector<int> up, acting;
   pg->get_osdmap()->pg_to_up_acting_osds(pg->info.pgid, up, acting);
-  int role = pg->get_osdmap()->calc_pg_role(service.whoami, acting);
+  int role = OSDMap::calc_pg_role(service.whoami, acting);
   pg->set_role(role);
   pg->reg_next_scrub();
   pg->handle_loaded(rctx);
@@ -1967,7 +1967,7 @@ void OSD::load_pgs()
 
     // generate state for PG's current mapping
     pg->get_osdmap()->pg_to_up_acting_osds(pgid, pg->up, pg->acting);
-    int role = pg->get_osdmap()->calc_pg_role(whoami, pg->acting);
+    int role = OSDMap::calc_pg_role(whoami, pg->acting);
     pg->set_role(role);
 
     PG::RecoveryCtx rctx(0, 0, 0, 0, 0, 0);
@@ -2146,8 +2146,6 @@ void OSD::handle_pg_peering_evt(
 
     bool create = false;
     if (primary) {
-      assert(role == 0);  // otherwise, probably bug in project_pg_history.
-
       // DNE on source?
       if (info.dne()) {
 	// is there a creation pending on this pg?
@@ -2165,8 +2163,7 @@ void OSD::handle_pg_peering_evt(
       }
       creating_pgs.erase(info.pgid);
     } else {
-      assert(role != 0);    // i should be replica
-      assert(!info.dne());  // and pg exists if we are hearing about it
+      assert(!info.dne());  // pg exists if we are hearing about it
     }
 
     // do we need to resurrect a deleting pg?
@@ -5469,9 +5466,9 @@ void OSD::advance_map(ObjectStore::Transaction& t, C_Contexts *tfin)
 
     // am i still primary?
     vector<int> acting;
-    int nrep = osdmap->pg_to_acting_osds(pgid, acting);
-    int role = osdmap->calc_pg_role(whoami, acting, nrep);
-    if (role != 0) {
+    int primary;
+    osdmap->pg_to_acting_osds(pgid, &acting, &primary);
+    if (primary != whoami) {
       dout(10) << " no longer primary for " << pgid << ", stopping creation" << dendl;
       creating_pgs.erase(p);
     } else {
@@ -5488,7 +5485,6 @@ void OSD::advance_map(ObjectStore::Transaction& t, C_Contexts *tfin)
   while (p != waiting_for_pg.end()) {
     pg_t pgid = p->first;
 
-    // am i still primary?
     vector<int> acting;
     int nrep = osdmap->pg_to_acting_osds(pgid, acting);
     int role = osdmap->calc_pg_role(whoami, acting, nrep);
@@ -5951,10 +5947,11 @@ void OSD::handle_pg_create(OpRequestRef op)
    
     // is it still ours?
     vector<int> up, acting;
-    osdmap->pg_to_up_acting_osds(on, up, acting);
+    int up_primary, acting_primary;
+    osdmap->pg_to_up_acting_osds(on, &up, &up_primary, &acting, &acting_primary);
     int role = osdmap->calc_pg_role(whoami, acting, acting.size());
 
-    if (role != 0) {
+    if (up_primary != whoami) {
       dout(10) << "mkpg " << pgid << "  not primary (role=" << role << "), skipping" << dendl;
       continue;
     }
@@ -6601,7 +6598,6 @@ void OSD::handle_pg_query(OpRequestRef op)
     // get active crush mapping
     vector<int> up, acting;
     osdmap->pg_to_up_acting_osds(pgid, up, acting);
-    int role = osdmap->calc_pg_role(whoami, acting, acting.size());
 
     // same primary?
     pg_history_t history = it->second.history;
@@ -6616,7 +6612,6 @@ void OSD::handle_pg_query(OpRequestRef op)
       continue;
     }
 
-    assert(role != 0);
     dout(10) << " pg " << pgid << " dne" << dendl;
     pg_info_t empty(pgid);
     if (it->second.type == pg_query_t::LOG ||
diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 545aaa6ad7b6..5e9dd7bbbf8f 100644
--- a/src/osd/PG.cc
+++ b/src/osd/PG.cc
@@ -1753,7 +1753,7 @@ void PG::split_into(pg_t child_pgid, PG *child, unsigned split_bits)
 
   // There can't be recovery/backfill going on now
   get_osdmap()->pg_to_up_acting_osds(child->info.pgid, child->up, child->acting);
-  child->role = get_osdmap()->calc_pg_role(osd->whoami, child->acting);
+  child->role = OSDMap::calc_pg_role(osd->whoami, child->acting);
   if (get_primary() != child->get_primary())
     child->info.history.same_primary_since = get_osdmap()->get_epoch();
 
@@ -4652,7 +4652,7 @@ void PG::start_peering_interval(const OSDMapRef lastmap,
 	       << dendl;
     } else {
       // primary is the same.
-      if (role == 0) {
+      if (is_primary()) {
 	// i am (still) primary. but my replica set changed.
 	state_clear(PG_STATE_CLEAN);