From: Sage Weil <sage@redhat.com>
Date: Mon, 9 Dec 2019 16:56:02 +0000 (-0600)
Subject: osd/OSDMap: rename old calc_pg_role -> calc_pg_role_broken
X-Git-Tag: v15.1.0~577^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2e03356f81f5dfb29ddc591b933b3cb2efdc6fba;p=ceph.git

osd/OSDMap: rename old calc_pg_role -> calc_pg_role_broken

Also primary_changed -> primary_changed_broken.

Logged this issue with https://tracker.ceph.com/issues/43213

Signed-off-by: Sage Weil <sage@redhat.com>
---

diff --git a/src/osd/OSDMap.cc b/src/osd/OSDMap.cc
index 263d6fa7cc97..68788240a2f0 100644
--- a/src/osd/OSDMap.cc
+++ b/src/osd/OSDMap.cc
@@ -2639,8 +2639,11 @@ void OSDMap::_pg_to_up_acting_osds(
     *acting_primary = _acting_primary;
 }
 
-int OSDMap::calc_pg_role(int osd, const vector<int>& acting, int nrep)
+int OSDMap::calc_pg_role_broken(int osd, const vector<int>& acting, int nrep)
 {
+  // This implementation is broken for EC PGs since the osd may appear
+  // multiple times in the acting set.  See
+  // https://tracker.ceph.com/issues/43213
   if (!nrep)
     nrep = acting.size();
   for (int i=0; i<nrep; i++) 
@@ -2666,7 +2669,7 @@ int OSDMap::calc_pg_role(pg_shard_t who, const vector<int>& acting)
   return -1;
 }
 
-bool OSDMap::primary_changed(
+bool OSDMap::primary_changed_broken(
   int oldprimary,
   const vector<int> &oldacting,
   int newprimary,
@@ -2678,8 +2681,8 @@ bool OSDMap::primary_changed(
     return true;     // was empty, now not, or vice versa
   if (oldprimary != newprimary)
     return true;     // primary changed
-  if (calc_pg_role(oldprimary, oldacting) !=
-      calc_pg_role(newprimary, newacting))
+  if (calc_pg_role_broken(oldprimary, oldacting) !=
+      calc_pg_role_broken(newprimary, newacting))
     return true;
   return false;      // same primary (tho replicas may have changed)
 }
diff --git a/src/osd/OSDMap.h b/src/osd/OSDMap.h
index ba546f37eb4b..db45f811ae82 100644
--- a/src/osd/OSDMap.h
+++ b/src/osd/OSDMap.h
@@ -1388,9 +1388,9 @@ public:
   }
 
 
-  static int calc_pg_role(int osd, const std::vector<int>& acting, int nrep=0);
+  static int calc_pg_role_broken(int osd, const std::vector<int>& acting, int nrep=0);
   static int calc_pg_role(pg_shard_t who, const std::vector<int>& acting);
-  static bool primary_changed(
+  static bool primary_changed_broken(
     int oldprimary,
     const std::vector<int> &oldacting,
     int newprimary,
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 6665957ddb3e..d1da49bfffb8 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -2640,7 +2640,7 @@ bool Objecter::is_pg_changed(
   const vector<int>& newacting,
   bool any_change)
 {
-  if (OSDMap::primary_changed(
+  if (OSDMap::primary_changed_broken( // https://tracker.ceph.com/issues/43213
 	oldprimary,
 	oldacting,
 	newprimary,