mds: remove CDir::pop_spread

This is a old artifact of when it was thought MDS requests would be
forwarded. It seemed good to track popularity based on who sent the
request. So, "who" is almost always -1. Because of this, a directory's
CDir::pop_spread is never actually hit. This has the side-effect of
preventing any replication except from hits from open/getattr!

Instead of using pop_spread, just use CDir::pop_me. Also, replicate
for any read load including READDIR.

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

diff --git a/src/mds/CDir.cc b/src/mds/CDir.cc
index f53e44e6f1892a710093a77b8611dbfe659f53f8..7744a47e45405648e1a6f3f902eb612aca5e9577 100644 (file)
--- a/src/mds/CDir.cc
+++ b/src/mds/CDir.cc
@@ -208,7 +208,6 @@ CDir::CDir(CInode *in, frag_t fg, MDCache *mdc, bool auth) :
   pop_nested(mdc->decayrate),
   pop_auth_subtree(mdc->decayrate),
   pop_auth_subtree_nested(mdc->decayrate),
-  pop_spread(mdc->decayrate),
   pop_lru_subdirs(member_offset(CInode, item_pop_lru)),
   dir_auth(CDIR_AUTH_DEFAULT)
 {

diff --git a/src/mds/CDir.h b/src/mds/CDir.h
index 852f86ba826e442d2a027c0894ed78a4cc4b7a6e..f6eb938638ed7d238230be7f129916e93a194ef7 100644 (file)
--- a/src/mds/CDir.h
+++ b/src/mds/CDir.h
@@ -738,8 +738,6 @@ protected:
 
   ceph::coarse_mono_time last_popularity_sample = ceph::coarse_mono_clock::zero();
 
-  load_spread_t pop_spread;
-
   elist<CInode*> pop_lru_subdirs;
 
   std::unique_ptr<bloom_filter> bloom; // XXX not part of mempool::mds_co

diff --git a/src/mds/MDBalancer.cc b/src/mds/MDBalancer.cc
index 138f65fb57941281e86f8fc2f5dc9150f04d0bcb..7eb7722b190fde0eba31a3281a1ec12d752890d6 100644 (file)
--- a/src/mds/MDBalancer.cc
+++ b/src/mds/MDBalancer.cc
@@ -1190,13 +1190,13 @@ void MDBalancer::find_exports(CDir *dir,
   }
 }
 
-void MDBalancer::hit_inode(CInode *in, int type, int who)
+void MDBalancer::hit_inode(CInode *in, int type)
 {
   // hit inode
   in->pop.get(type).hit();
 
   if (in->get_parent_dn())
-    hit_dir(in->get_parent_dn()->get_dir(), type, who);
+    hit_dir(in->get_parent_dn()->get_dir(), type);
 }
 
 void MDBalancer::maybe_fragment(CDir *dir, bool hot)
@@ -1229,7 +1229,7 @@ void MDBalancer::maybe_fragment(CDir *dir, bool hot)
   }
 }
 
-void MDBalancer::hit_dir(CDir *dir, int type, int who, double amount)
+void MDBalancer::hit_dir(CDir *dir, int type, double amount)
 {
   if (dir->inode->is_stray())
     return;
@@ -1245,30 +1245,16 @@ void MDBalancer::hit_dir(CDir *dir, int type, int who, double amount)
   maybe_fragment(dir, hot);
 
   // replicate?
-  if (type == META_POP_IRD && who >= 0) {
-    dir->pop_spread.hit(who);
-  }
-
+  const bool readop = (type == META_POP_IRD || type == META_POP_READDIR);
   double rd_adj = 0.0;
-  if (type == META_POP_IRD &&
-      dir->last_popularity_sample < last_sample) {
+  if (readop && dir->last_popularity_sample < last_sample) {
     double dir_pop = dir->pop_auth_subtree.get(type).get();    // hmm??
+    dir_pop += v * 10;
     dir->last_popularity_sample = last_sample;
-    double pop_sp = dir->pop_spread.get();
-    dir_pop += pop_sp * 10;
-
-    if (pop_sp > 0) {
-      dout(20) << type << " pop " << dir_pop << " spread " << pop_sp
-             << " " << dir->pop_spread.last[0]
-             << " " << dir->pop_spread.last[1]
-             << " " << dir->pop_spread.last[2]
-             << " " << dir->pop_spread.last[3]
-             << " in " << *dir << dendl;
-    }
 
-    if (dir->is_auth() && !dir->is_ambiguous_auth()) {
-      if (dir->can_rep() &&
-         dir_pop >= g_conf()->mds_bal_replicate_threshold) {
+    dout(20) << type << " pop " << dir_pop << " spread in " << *dir << dendl;
+    if (dir->is_auth() && !dir->is_ambiguous_auth() && dir->can_rep()) {
+      if (dir_pop >= g_conf()->mds_bal_replicate_threshold) {
        // replicate
        double rdp = dir->pop_me.get(META_POP_IRD).get();
        rd_adj = rdp / mds->get_mds_map()->get_num_in_mds() - rdp;

diff --git a/src/mds/MDBalancer.h b/src/mds/MDBalancer.h
index e3ee3deabcff8ec78b6e7456fe5844baf72e1168..2f5cf730ec54f8dd37734abdd4690989aa5d4d82 100644 (file)
--- a/src/mds/MDBalancer.h
+++ b/src/mds/MDBalancer.h
@@ -55,8 +55,8 @@ public:
   void add_import(CDir *im);
   void adjust_pop_for_rename(CDir *pdir, CDir *dir, bool inc);
 
-  void hit_inode(CInode *in, int type, int who=-1);
-  void hit_dir(CDir *dir, int type, int who=-1, double amount=1.0);
+  void hit_inode(CInode *in, int type);
+  void hit_dir(CDir *dir, int type, double amount=1.0);
 
   void queue_split(const CDir *dir, bool fast);
   void queue_merge(CDir *dir);

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 5ec4aabb6e4f4f90715136e3b781a0213be4bd87..42d69e5faf21f3cb1208d158d8a843e3ca79c249 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -3988,7 +3988,7 @@ void Server::handle_client_getattr(MDRequestRef& mdr, bool is_lookup)
   // value for them.  (currently this matters for xattrs and inline data)
   mdr->getattr_caps = mask;
 
-  mds->balancer->hit_inode(ref, META_POP_IRD, req->get_source().num());
+  mds->balancer->hit_inode(ref, META_POP_IRD);
 
   // reply
   dout(10) << "reply to stat on " << *req << dendl;
@@ -4387,8 +4387,7 @@ void Server::handle_client_open(MDRequestRef& mdr)
   if (cmode & CEPH_FILE_MODE_WR)
     mds->balancer->hit_inode(cur, META_POP_IWR);
   else
-    mds->balancer->hit_inode(cur, META_POP_IRD,
-                            mdr->client_request->get_source().num());
+    mds->balancer->hit_inode(cur, META_POP_IRD);
 
   CDentry *dn = 0;
   if (req->get_dentry_wanted()) {
@@ -4871,7 +4870,7 @@ void Server::handle_client_readdir(MDRequestRef& mdr)
   mdr->reply_extra_bl = dirbl;
 
   // bump popularity.  NOTE: this doesn't quite capture it.
-  mds->balancer->hit_dir(dir, META_POP_READDIR, -1, numfiles);
+  mds->balancer->hit_dir(dir, META_POP_READDIR, numfiles);
   
   // reply
   mdr->tracei = diri;

diff --git a/src/mds/mdstypes.h b/src/mds/mdstypes.h
index fd53f39b71ba3e9347f0521f8a157bae7e50b25f..70679e229a6d435d156a502ee903b90562c986fe 100644 (file)
--- a/src/mds/mdstypes.h
+++ b/src/mds/mdstypes.h
@@ -1870,40 +1870,6 @@ inline std::ostream& operator<<(std::ostream& out, const mds_load_t& load)
              << ">";
 }
 
-class load_spread_t {
-public:
-  using time = DecayCounter::time;
-  using clock = DecayCounter::clock;
-  static const int MAX = 4;
-
-  load_spread_t(const DecayRate &rate) : count(rate)
-  {}
-
-  load_spread_t() = delete;
-
-  double hit(int who) {
-    for (int i=0; i<n; i++)
-      if (last[i] == who) 
-       return count.get_last();
-
-    // we're new(ish)
-    last[p++] = who;
-    if (n < MAX) n++;
-    if (n == 1) return 0.0;
-
-    if (p == MAX) p = 0;
-
-    return count.hit();
-  }
-  double get() const {
-    return count.get();
-  }
-
-  std::array<int, MAX> last = {-1, -1, -1, -1};
-  int p = 0, n = 0;
-  DecayCounter count;
-};
-
 // ================================================================
 typedef std::pair<mds_rank_t, mds_rank_t> mds_authority_t;