mds: cleanup replica_map access

The gymnastics protecting the map failed as the code evolved. Just expose it
normally with a getter.

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

10 files changed, 88 insertions, 116 deletions

diff --git a/src/include/compact_map.h b/src/include/compact_map.h
index 12ba8b3504a..975baa064d3 100644
--- a/src/include/compact_map.h
+++ b/src/include/compact_map.h
@@ -61,6 +61,9 @@ protected:
       --it;
       return *this;
     }
+    const std::pair<const Key,T>& operator*() {
+      return *it;
+    }
     const std::pair<const Key,T>* operator->() {
       return it.operator->();
     }
@@ -103,6 +106,9 @@ protected:
       --it;
       return *this;
     }
+    std::pair<const Key,T>& operator*() {
+      return *it;
+    }
     std::pair<const Key,T>* operator->() {
       return it.operator->();
     }
diff --git a/src/mds/CDentry.h b/src/mds/CDentry.h
index 01e3d3c4992..29c5cd63308 100644
--- a/src/mds/CDentry.h
+++ b/src/mds/CDentry.h
@@ -266,7 +266,7 @@ public:
     ::encode(version, bl);
     ::encode(projected_version, bl);
     ::encode(lock, bl);
-    ::encode(replica_map, bl);
+    ::encode(get_replicas(), bl);
     get(PIN_TEMPEXPORTING);
   }
   void finish_export() {
@@ -288,14 +288,14 @@ public:
     ::decode(version, blp);
     ::decode(projected_version, blp);
     ::decode(lock, blp);
-    ::decode(replica_map, blp);
+    ::decode(get_replicas(), blp);
 
     // twiddle
     state &= MASK_STATE_IMPORT_KEPT;
     state_set(CDentry::STATE_AUTH);
     if (nstate & STATE_DIRTY)
       _mark_dirty(ls);
-    if (!replica_map.empty())
+    if (is_replicated())
       get(PIN_REPLICATED);
     replica_nonce = 0;
   }
diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
index c190cca175b..57b1e981f03 100644
--- a/src/mds/CDir.cc
+++ b/src/mds/CDir.cc
@@ -932,7 +932,7 @@ void CDir::finish_old_fragment(list<MDSInternalContextBase*>& waiters, bool repl
 
 void CDir::init_fragment_pins()
 {
-  if (!replica_map.empty())
+  if (is_replicated())
     get(PIN_REPLICATED);
   if (state_test(STATE_DIRTY))
     get(PIN_DIRTY);
@@ -976,7 +976,7 @@ void CDir::split(int bits, list<CDir*>& subs, list<MDSInternalContextBase*>& wai
   for (list<frag_t>::iterator p = frags.begin(); p != frags.end(); ++p) {
     CDir *f = new CDir(inode, *p, cache, is_auth());
     f->state_set(state & (MASK_STATE_FRAGMENT_KEPT | STATE_COMPLETE));
-    f->replica_map = replica_map;
+    f->get_replicas() = get_replicas();
     f->dir_auth = dir_auth;
     f->init_fragment_pins();
     f->set_version(get_version());
@@ -1085,12 +1085,10 @@ void CDir::merge(list<CDir*>& subs, list<MDSInternalContextBase*>& waiters, bool
       steal_dentry(dir->items.begin()->second);
     
     // merge replica map
-    for (compact_map<mds_rank_t,unsigned>::iterator p = dir->replicas_begin();
-	 p != dir->replicas_end();
-	 ++p) {
-      unsigned cur = replica_map[p->first];
-      if (p->second > cur)
-	replica_map[p->first] = p->second;
+    for (const auto &p : dir->get_replicas()) {
+      unsigned cur = get_replicas()[p.first];
+      if (p.second > cur)
+	get_replicas()[p.first] = p.second;
     }
 
     // merge version
@@ -2432,7 +2430,7 @@ void CDir::encode_export(bufferlist& bl)
   ::encode(pop_auth_subtree, bl);
 
   ::encode(dir_rep_by, bl);  
-  ::encode(replica_map, bl);
+  ::encode(get_replicas(), bl);
 
   get(PIN_TEMPEXPORTING);
 }
@@ -2473,8 +2471,8 @@ void CDir::decode_import(bufferlist::iterator& blp, utime_t now, LogSegment *ls)
   pop_auth_subtree_nested.add(now, cache->decayrate, pop_auth_subtree);
 
   ::decode(dir_rep_by, blp);
-  ::decode(replica_map, blp);
-  if (!replica_map.empty()) get(PIN_REPLICATED);
+  ::decode(get_replicas(), blp);
+  if (is_replicated()) get(PIN_REPLICATED);
 
   replica_nonce = 0;  // no longer defined
 
diff --git a/src/mds/CInode.cc b/src/mds/CInode.cc
index 3b0d6ba4a16..3b4e76d92a2 100644
--- a/src/mds/CInode.cc
+++ b/src/mds/CInode.cc
@@ -3622,7 +3622,7 @@ void CInode::encode_export(bufferlist& bl)
 
   ::encode(pop, bl);
 
-  ::encode(replica_map, bl);
+  ::encode(get_replicas(), bl);
 
   // include scatterlock info for any bounding CDirs
   bufferlist bounding;
@@ -3687,8 +3687,8 @@ void CInode::decode_import(bufferlist::iterator& p,
 
   ::decode(pop, ceph_clock_now(), p);
 
-  ::decode(replica_map, p);
-  if (!replica_map.empty())
+  ::decode(get_replicas(), p);
+  if (is_replicated())
     get(PIN_REPLICATED);
   replica_nonce = 0;
 
diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index f76c698d8be..01bc1e2c449 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -130,28 +130,24 @@ void Locker::tick()
 
 void Locker::send_lock_message(SimpleLock *lock, int msg)
 {
-  for (compact_map<mds_rank_t,unsigned>::iterator it = lock->get_parent()->replicas_begin();
-       it != lock->get_parent()->replicas_end();
-       ++it) {
+  for (const auto &it : lock->get_parent()->get_replicas()) {
     if (mds->is_cluster_degraded() &&
-	mds->mdsmap->get_state(it->first) < MDSMap::STATE_REJOIN) 
+	mds->mdsmap->get_state(it.first) < MDSMap::STATE_REJOIN)
       continue;
     MLock *m = new MLock(lock, msg, mds->get_nodeid());
-    mds->send_message_mds(m, it->first);
+    mds->send_message_mds(m, it.first);
   }
 }
 
 void Locker::send_lock_message(SimpleLock *lock, int msg, const bufferlist &data)
 {
-  for (compact_map<mds_rank_t,unsigned>::iterator it = lock->get_parent()->replicas_begin();
-       it != lock->get_parent()->replicas_end();
-       ++it) {
+  for (const auto &it : lock->get_parent()->get_replicas()) {
     if (mds->is_cluster_degraded() &&
-	mds->mdsmap->get_state(it->first) < MDSMap::STATE_REJOIN) 
+	mds->mdsmap->get_state(it.first) < MDSMap::STATE_REJOIN)
       continue;
     MLock *m = new MLock(lock, msg, mds->get_nodeid());
     m->set_data(data);
-    mds->send_message_mds(m, it->first);
+    mds->send_message_mds(m, it.first);
   }
 }
 
diff --git a/src/mds/MDCache.cc b/src/mds/MDCache.cc
index 0bb1cda9b90..9e423bada23 100644
--- a/src/mds/MDCache.cc
+++ b/src/mds/MDCache.cc
@@ -2017,12 +2017,10 @@ update:
     msg->quota = i->quota;
     mds->send_message_client_counted(msg, session->connection);
   }
-  for (compact_map<mds_rank_t, unsigned>::iterator it = in->replicas_begin();
-       it != in->replicas_end();
-       ++it) {
+  for (const auto &it : in->get_replicas()) {
     MGatherCaps *msg = new MGatherCaps;
     msg->ino = in->ino();
-    mds->send_message_mds(msg, it->first);
+    mds->send_message_mds(msg, it.first);
   }
 }
 
@@ -5953,13 +5951,11 @@ void MDCache::rejoin_send_acks()
       dq.pop_front();
       
       // dir
-      for (compact_map<mds_rank_t,unsigned>::iterator r = dir->replicas_begin();
-	   r != dir->replicas_end();
-	   ++r) {
-	auto it = acks.find(r->first);
+      for (auto &r : dir->get_replicas()) {
+	auto it = acks.find(r.first);
 	if (it == acks.end())
 	  continue;
-	it->second->add_strong_dirfrag(dir->dirfrag(), ++r->second, dir->dir_rep);
+	it->second->add_strong_dirfrag(dir->dirfrag(), ++r.second, dir->dir_rep);
 	it->second->add_dirfrag_base(dir);
       }
 	   
@@ -5975,36 +5971,32 @@ void MDCache::rejoin_send_acks()
 	  in = dnl->get_inode();
 
 	// dentry
-	for (compact_map<mds_rank_t,unsigned>::iterator r = dn->replicas_begin();
-	     r != dn->replicas_end();
-	     ++r) {
-	  auto it = acks.find(r->first);
+	for (auto &r : dn->get_replicas()) {
+	  auto it = acks.find(r.first);
 	  if (it == acks.end())
 	    continue;
 	  it->second->add_strong_dentry(dir->dirfrag(), dn->name, dn->first, dn->last,
 					   dnl->is_primary() ? dnl->get_inode()->ino():inodeno_t(0),
 					   dnl->is_remote() ? dnl->get_remote_ino():inodeno_t(0),
 					   dnl->is_remote() ? dnl->get_remote_d_type():0,
-					   ++r->second,
+					   ++r.second,
 					   dn->lock.get_replica_state());
 	  // peer missed MDentrylink message ?
-	  if (in && !in->is_replica(r->first))
-	    in->add_replica(r->first);
+	  if (in && !in->is_replica(r.first))
+	    in->add_replica(r.first);
 	}
 	
 	if (!in)
 	  continue;
 
-	for (compact_map<mds_rank_t,unsigned>::iterator r = in->replicas_begin();
-	     r != in->replicas_end();
-	     ++r) {
-	  auto it = acks.find(r->first);
+	for (auto &r : in->get_replicas()) {
+	  auto it = acks.find(r.first);
 	  if (it == acks.end())
 	    continue;
 	  it->second->add_inode_base(in, mds->mdsmap->get_up_features());
 	  bufferlist bl;
-	  in->_encode_locks_state_for_rejoin(bl, r->first);
-	  it->second->add_inode_locks(in, ++r->second, bl);
+	  in->_encode_locks_state_for_rejoin(bl, r.first);
+	  it->second->add_inode_locks(in, ++r.second, bl);
 	}
 	
 	// subdirs in this subtree?
@@ -6015,28 +6007,24 @@ void MDCache::rejoin_send_acks()
 
   // base inodes too
   if (root && root->is_auth()) 
-    for (compact_map<mds_rank_t,unsigned>::iterator r = root->replicas_begin();
-	 r != root->replicas_end();
-	 ++r) {
-      auto it = acks.find(r->first);
+    for (auto &r : root->get_replicas()) {
+      auto it = acks.find(r.first);
       if (it == acks.end())
 	continue;
       it->second->add_inode_base(root, mds->mdsmap->get_up_features());
       bufferlist bl;
-      root->_encode_locks_state_for_rejoin(bl, r->first);
-      it->second->add_inode_locks(root, ++r->second, bl);
+      root->_encode_locks_state_for_rejoin(bl, r.first);
+      it->second->add_inode_locks(root, ++r.second, bl);
     }
   if (myin)
-    for (compact_map<mds_rank_t,unsigned>::iterator r = myin->replicas_begin();
-	 r != myin->replicas_end();
-	 ++r) {
-      auto it = acks.find(r->first);
+    for (auto &r : myin->get_replicas()) {
+      auto it = acks.find(r.first);
       if (it == acks.end())
 	continue;
       it->second->add_inode_base(myin, mds->mdsmap->get_up_features());
       bufferlist bl;
-      myin->_encode_locks_state_for_rejoin(bl, r->first);
-      it->second->add_inode_locks(myin, ++r->second, bl);
+      myin->_encode_locks_state_for_rejoin(bl, r.first);
+      it->second->add_inode_locks(myin, ++r.second, bl);
     }
 
   // include inode base for any inodes whose scatterlocks may have updated
@@ -6044,10 +6032,8 @@ void MDCache::rejoin_send_acks()
        p != rejoin_potential_updated_scatterlocks.end();
        ++p) {
     CInode *in = *p;
-    for (compact_map<mds_rank_t,unsigned>::iterator r = in->replicas_begin();
-	 r != in->replicas_end();
-	 ++r) {
-      auto it = acks.find(r->first);
+    for (const auto &r : in->get_replicas()) {
+      auto it = acks.find(r.first);
       if (it == acks.end())
 	continue;
       it->second->add_inode_base(in, mds->mdsmap->get_up_features());
@@ -7251,7 +7237,7 @@ void MDCache::handle_cache_expire(MCacheExpire *m)
       if (nonce == dir->get_replica_nonce(from)) {
 	// remove from our cached_by
 	dout(7) << " dir expire on " << *dir << " from mds." << from
-		<< " replicas was " << dir->replica_map << dendl;
+		<< " replicas was " << dir->get_replicas() << dendl;
 	dir->remove_replica(from);
       } 
       else {
@@ -10295,10 +10281,9 @@ int MDCache::send_dir_updates(CDir *dir, bool bcast)
   if (bcast) {
     mds->get_mds_map()->get_active_mds_set(who);
   } else {
-    for (compact_map<mds_rank_t,unsigned>::iterator p = dir->replicas_begin();
-	 p != dir->replicas_end();
-	 ++p)
-      who.insert(p->first);
+    for (const auto &p : dir->get_replicas()) {
+      who.insert(p.first);
+    }
   }
   
   dout(7) << "sending dir_update on " << *dir << " bcast " << bcast << " to " << who << dendl;
@@ -10381,22 +10366,20 @@ void MDCache::send_dentry_link(CDentry *dn, MDRequestRef& mdr)
   dout(7) << "send_dentry_link " << *dn << dendl;
 
   CDir *subtree = get_subtree_root(dn->get_dir());
-  for (compact_map<mds_rank_t,unsigned>::iterator p = dn->replicas_begin();
-       p != dn->replicas_end();
-       ++p) {
+  for (const auto &p : dn->get_replicas()) {
     // don't tell (rename) witnesses; they already know
-    if (mdr.get() && mdr->more()->witnessed.count(p->first))
+    if (mdr.get() && mdr->more()->witnessed.count(p.first))
       continue;
-    if (mds->mdsmap->get_state(p->first) < MDSMap::STATE_REJOIN ||
-	(mds->mdsmap->get_state(p->first) == MDSMap::STATE_REJOIN &&
-	 rejoin_gather.count(p->first)))
+    if (mds->mdsmap->get_state(p.first) < MDSMap::STATE_REJOIN ||
+	(mds->mdsmap->get_state(p.first) == MDSMap::STATE_REJOIN &&
+	 rejoin_gather.count(p.first)))
       continue;
     CDentry::linkage_t *dnl = dn->get_linkage();
     MDentryLink *m = new MDentryLink(subtree->dirfrag(), dn->get_dir()->dirfrag(),
 				     dn->name, dnl->is_primary());
     if (dnl->is_primary()) {
       dout(10) << "  primary " << *dnl->get_inode() << dendl;
-      replicate_inode(dnl->get_inode(), p->first, m->bl,
+      replicate_inode(dnl->get_inode(), p.first, m->bl,
 		      mds->mdsmap->get_up_features());
     } else if (dnl->is_remote()) {
       inodeno_t ino = dnl->get_remote_ino();
@@ -10406,7 +10389,7 @@ void MDCache::send_dentry_link(CDentry *dn, MDRequestRef& mdr)
       ::encode(d_type, m->bl);
     } else
       ceph_abort();   // aie, bad caller!
-    mds->send_message_mds(m, p->first);
+    mds->send_message_mds(m, p.first);
   }
 }
 
@@ -11322,12 +11305,10 @@ void MDCache::_fragment_stored(MDRequestRef& mdr)
 
   // tell peers
   CDir *first = *info.resultfrags.begin();
-  for (compact_map<mds_rank_t,unsigned>::iterator p = first->replicas_begin();
-       p != first->replicas_end();
-       ++p) {
-    if (mds->mdsmap->get_state(p->first) < MDSMap::STATE_REJOIN ||
-	(mds->mdsmap->get_state(p->first) == MDSMap::STATE_REJOIN &&
-	 rejoin_gather.count(p->first)))
+  for (const auto &p : first->get_replicas()) {
+    if (mds->mdsmap->get_state(p.first) < MDSMap::STATE_REJOIN ||
+	(mds->mdsmap->get_state(p.first) == MDSMap::STATE_REJOIN &&
+	 rejoin_gather.count(p.first)))
       continue;
 
     MMDSFragmentNotify *notify = new MMDSFragmentNotify(basedirfrag, info.bits);
@@ -11336,9 +11317,9 @@ void MDCache::_fragment_stored(MDRequestRef& mdr)
     for (list<CDir*>::iterator q = info.resultfrags.begin();
 	 q != info.resultfrags.end();
 	 ++q)
-      replicate_dir(*q, p->first, notify->basebl);
+      replicate_dir(*q, p.first, notify->basebl);
 
-    mds->send_message_mds(notify, p->first);
+    mds->send_message_mds(notify, p.first);
   }
 
   // journal commit
diff --git a/src/mds/MDSCacheObject.cc b/src/mds/MDSCacheObject.cc
index 1265a4a9e89..eca6fac148b 100644
--- a/src/mds/MDSCacheObject.cc
+++ b/src/mds/MDSCacheObject.cc
@@ -21,12 +21,10 @@ void MDSCacheObject::dump(Formatter *f) const
   f->open_object_section("auth_state");
   {
     f->open_object_section("replicas");
-    const compact_map<mds_rank_t,unsigned>& replicas = get_replicas();
-    for (compact_map<mds_rank_t,unsigned>::const_iterator i = replicas.begin();
-         i != replicas.end(); ++i) {
+    for (const auto &it : get_replicas()) {
       std::ostringstream rank_str;
-      rank_str << i->first;
-      f->dump_int(rank_str.str().c_str(), i->second);
+      rank_str << it.first;
+      f->dump_int(rank_str.str().c_str(), it.second);
     }
     f->close_section();
   }
diff --git a/src/mds/MDSCacheObject.h b/src/mds/MDSCacheObject.h
index 1bc80cfed67..0d5c2aeeed4 100644
--- a/src/mds/MDSCacheObject.h
+++ b/src/mds/MDSCacheObject.h
@@ -286,14 +286,12 @@ protected:
       put(PIN_REPLICATED);
     replica_map.clear();
   }
-  compact_map<mds_rank_t,unsigned>::iterator replicas_begin() { return replica_map.begin(); }
-  compact_map<mds_rank_t,unsigned>::iterator replicas_end() { return replica_map.end(); }
+  compact_map<mds_rank_t,unsigned>& get_replicas() { return replica_map; }
   const compact_map<mds_rank_t,unsigned>& get_replicas() const { return replica_map; }
   void list_replicas(std::set<mds_rank_t>& ls) const {
-    for (compact_map<mds_rank_t,unsigned>::const_iterator p = replica_map.begin();
-	 p != replica_map.end();
-	 ++p)
-      ls.insert(p->first);
+    for (const auto &p : replica_map) {
+      ls.insert(p.first);
+    }
   }
 
   unsigned get_replica_nonce() const { return replica_nonce; }
diff --git a/src/mds/Migrator.cc b/src/mds/Migrator.cc
index 6ff5a931b95..59a82dcd42b 100644
--- a/src/mds/Migrator.cc
+++ b/src/mds/Migrator.cc
@@ -1087,12 +1087,10 @@ void Migrator::export_frozen(CDir *dir, uint64_t tid)
   MExportDirPrep *prep = new MExportDirPrep(dir->dirfrag(), it->second.tid);
 
   // include list of bystanders
-  for (compact_map<mds_rank_t,unsigned>::iterator p = dir->replicas_begin();
-       p != dir->replicas_end();
-       ++p) {
-    if (p->first != it->second.peer) {
-      dout(10) << "bystander mds." << p->first << dendl;
-      prep->add_bystander(p->first);
+  for (const auto &p : dir->get_replicas()) {
+    if (p.first != it->second.peer) {
+      dout(10) << "bystander mds." << p.first << dendl;
+      prep->add_bystander(p.first);
     }
   }
 
@@ -1272,22 +1270,20 @@ void Migrator::handle_export_prep_ack(MExportDirPrepAck *m)
 	  it->second.warning_ack_waiting.count(MDS_RANK_NONE) > 0));
   assert(it->second.notify_ack_waiting.empty());
 
-  for (compact_map<mds_rank_t,unsigned>::iterator p = dir->replicas_begin();
-       p != dir->replicas_end();
-       ++p) {
-    if (p->first == it->second.peer) continue;
+  for (const auto &p : dir->get_replicas()) {
+    if (p.first == it->second.peer) continue;
     if (mds->is_cluster_degraded() &&
-	!mds->mdsmap->is_clientreplay_or_active_or_stopping(p->first))
+	!mds->mdsmap->is_clientreplay_or_active_or_stopping(p.first))
       continue;  // only if active
-    it->second.warning_ack_waiting.insert(p->first);
-    it->second.notify_ack_waiting.insert(p->first);  // we'll eventually get a notifyack, too!
+    it->second.warning_ack_waiting.insert(p.first);
+    it->second.notify_ack_waiting.insert(p.first);  // we'll eventually get a notifyack, too!
 
     MExportDirNotify *notify = new MExportDirNotify(dir->dirfrag(), it->second.tid, true,
 						    mds_authority_t(mds->get_nodeid(),CDIR_AUTH_UNKNOWN),
 						    mds_authority_t(mds->get_nodeid(),it->second.peer));
     for (set<CDir*>::iterator q = bounds.begin(); q != bounds.end(); ++q)
       notify->get_bounds().push_back((*q)->dirfrag());
-    mds->send_message_mds(notify, p->first);
+    mds->send_message_mds(notify, p.first);
     
   }
 
diff --git a/src/mds/SimpleLock.h b/src/mds/SimpleLock.h
index 27eae7bebd4..56d1c4b0b7e 100644
--- a/src/mds/SimpleLock.h
+++ b/src/mds/SimpleLock.h
@@ -373,10 +373,9 @@ public:
   }
 
   void init_gather() {
-    for (compact_map<mds_rank_t,unsigned>::iterator p = parent->replicas_begin();
-	 p != parent->replicas_end();
-	 ++p)
-      more()->gather_set.insert(p->first);
+    for (const auto p : parent->get_replicas()) {
+      more()->gather_set.insert(p.first);
+    }
   }
   bool is_gathering() const {
     return have_more() && !more()->gather_set.empty();