From: Sage Weil <sage@inktank.com>
Date: Tue, 9 Jul 2013 04:54:53 +0000 (-0700)
Subject: mon: make service trim_to stateless
X-Git-Tag: v0.67-rc1~134^2~4
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=da248a9e1dc98e88e1f00b5104f77f98a2b8503b;p=ceph.git

mon: make service trim_to stateless

Call get_trim_to() when we need to know how much to trim (if any), and
calculate it then.  No need to keep this in a hidden trim_version
variable and remember to update it.  This drops several helpers and
accessors and makes get_trim_to() a single method that services need to
override.

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

diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index e61551b8ba2..8988046954a 100644
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -279,12 +279,13 @@ void AuthMonitor::encode_full(MonitorDBStore::Transaction *t)
   }
 }
 
-void AuthMonitor::update_trim()
+version_t AuthMonitor::get_trim_to()
 {
   unsigned max = g_conf->paxos_max_join_drift * 2;
   version_t version = get_last_committed();
   if (mon->is_leader() && (version > max))
-    set_trim_to(version - max);
+    return version - max;
+  return 0;
 }
 
 bool AuthMonitor::preprocess_query(PaxosServiceMessage *m)
diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h
index 12f7acb4f4b..f538e737856 100644
--- a/src/mon/AuthMonitor.h
+++ b/src/mon/AuthMonitor.h
@@ -135,7 +135,7 @@ private:
   // propose pending update to peers
   void encode_pending(MonitorDBStore::Transaction *t);
   virtual void encode_full(MonitorDBStore::Transaction *t);
-  void update_trim();
+  version_t get_trim_to();
 
   bool preprocess_query(PaxosServiceMessage *m);  // true if processed.
   bool prepare_update(PaxosServiceMessage *m);
diff --git a/src/mon/LogMonitor.cc b/src/mon/LogMonitor.cc
index 065a9bccc75..cab49060082 100644
--- a/src/mon/LogMonitor.cc
+++ b/src/mon/LogMonitor.cc
@@ -211,12 +211,13 @@ void LogMonitor::encode_full(MonitorDBStore::Transaction *t)
   put_version_latest_full(t, summary.version);
 }
 
-void LogMonitor::update_trim()
+version_t LogMonitor::get_trim_to()
 {
   unsigned max = g_conf->mon_max_log_epochs;
   version_t version = get_last_committed();
   if (mon->is_leader() && version > max)
-    set_trim_to(version - max);
+    return version - max;
+  return 0;
 }
 
 bool LogMonitor::preprocess_query(PaxosServiceMessage *m)
diff --git a/src/mon/LogMonitor.h b/src/mon/LogMonitor.h
index e20c81e227b..439ea4d999b 100644
--- a/src/mon/LogMonitor.h
+++ b/src/mon/LogMonitor.h
@@ -39,7 +39,7 @@ private:
   // propose pending update to peers
   void encode_pending(MonitorDBStore::Transaction *t);
   virtual void encode_full(MonitorDBStore::Transaction *t);
-  void update_trim();
+  version_t get_trim_to();
   bool preprocess_query(PaxosServiceMessage *m);  // true if processed.
   bool prepare_update(PaxosServiceMessage *m);
 
diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 541aac647f3..a00dc21eea1 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -533,7 +533,7 @@ void OSDMonitor::share_map_with_random_osd()
   mon->messenger->send_message(m, s->inst);
 }
 
-void OSDMonitor::update_trim()
+version_t OSDMonitor::get_trim_to()
 {
   if (mon->pgmon()->is_readable() &&
       mon->pgmon()->pg_map.creating_pgs.empty()) {
@@ -547,9 +547,9 @@ void OSDMonitor::update_trim()
 	floor = 0;
     }
     if (floor > get_first_committed())
-      if (get_trim_to() < floor)
-	set_trim_to(floor);
+      return floor;
   }
+  return 0;
 }
 
 void OSDMonitor::encode_trim_extra(MonitorDBStore::Transaction *tx, version_t first)
@@ -1795,8 +1795,6 @@ void OSDMonitor::tick()
   if (update_pools_status())
     do_propose = true;
 
-  update_trim();
-
   if (do_propose ||
       !pending_inc.new_pg_temp.empty())  // also propose if we adjusted pg_temp
     propose_pending();
diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h
index daf2e17dac1..3d901a8f139 100644
--- a/src/mon/OSDMonitor.h
+++ b/src/mon/OSDMonitor.h
@@ -183,7 +183,7 @@ private:
   bool prepare_update(PaxosServiceMessage *m);
   bool should_propose(double &delay);
 
-  void update_trim();
+  version_t get_trim_to();
 
   bool can_mark_down(int o);
   bool can_mark_up(int o);
diff --git a/src/mon/PGMonitor.cc b/src/mon/PGMonitor.cc
index ead42a05dca..9455fcec9a1 100644
--- a/src/mon/PGMonitor.cc
+++ b/src/mon/PGMonitor.cc
@@ -508,12 +508,13 @@ void PGMonitor::encode_pending(MonitorDBStore::Transaction *t)
   put_last_committed(t, version);
 }
 
-void PGMonitor::update_trim()
+version_t PGMonitor::get_trim_to()
 {
   unsigned max = g_conf->mon_max_pgmap_epochs;
   version_t version = get_last_committed();
   if (mon->is_leader() && (version > max))
-    set_trim_to(version - max);
+    return version - max;
+  return 0;
 }
 
 bool PGMonitor::preprocess_query(PaxosServiceMessage *m)
diff --git a/src/mon/PGMonitor.h b/src/mon/PGMonitor.h
index c6813eda3b1..e9b8f217255 100644
--- a/src/mon/PGMonitor.h
+++ b/src/mon/PGMonitor.h
@@ -60,7 +60,7 @@ private:
   void handle_osd_timeouts();
   void create_pending();  // prepare a new pending
   // propose pending update to peers
-  void update_trim();
+  version_t get_trim_to();
   void update_logger();
 
   void encode_pending(MonitorDBStore::Transaction *t);
diff --git a/src/mon/PaxosService.cc b/src/mon/PaxosService.cc
index 44cb64d9adb..2f3bacc36db 100644
--- a/src/mon/PaxosService.cc
+++ b/src/mon/PaxosService.cc
@@ -326,12 +326,11 @@ void PaxosService::maybe_trim()
   if (!is_writeable())
     return;
 
-  update_trim();
-  if (get_trim_to() == 0)
+  version_t trim_to = get_trim_to();
+  if (trim_to == 0)
     return;
 
   if (g_conf->paxos_service_trim_min > 0) {
-    version_t trim_to = get_trim_to();
     version_t first = get_first_committed();
     if ((trim_to > 0) &&
 	trim_to > first &&
@@ -339,10 +338,10 @@ void PaxosService::maybe_trim()
       return;  // not enough to trim
   }
 
-  dout(10) << __func__ << " trimming" << dendl;
+  dout(10) << __func__ << " trimming to " << trim_to << dendl;
 
   MonitorDBStore::Transaction t;
-  encode_trim(&t, get_trim_to());
+  encode_trim(&t, trim_to);
   bufferlist bl;
   t.encode(bl);
 
@@ -374,9 +373,6 @@ void PaxosService::encode_trim(MonitorDBStore::Transaction *t, version_t trim_to
 
   // let the service add any extra stuff
   encode_trim_extra(t, trim_to_max);
-
-  if (trim_to_max == trim_to)
-    set_trim_to(0);
 }
 
 void PaxosService::trim(MonitorDBStore::Transaction *t,
diff --git a/src/mon/PaxosService.h b/src/mon/PaxosService.h
index 8b11860b3f2..dc63e4e46b9 100644
--- a/src/mon/PaxosService.h
+++ b/src/mon/PaxosService.h
@@ -77,12 +77,6 @@ class PaxosService {
    * then have_pending should be true; otherwise, false.
    */
   bool have_pending; 
-  /**
-   * The version to trim to. If zero, we assume there is no version to be
-   * trimmed; otherwise, we assume we should trim to the version held by
-   * this variable.
-   */
-  version_t trim_version;
 
 protected:
 
@@ -200,7 +194,6 @@ public:
     : mon(mn), paxos(p), service_name(name),
       proposing(false),
       service_version(0), proposal_timer(0), have_pending(false),
-      trim_version(0),
       format_version(0),
       last_committed_name("last_committed"),
       first_committed_name("first_committed"),
@@ -684,31 +677,14 @@ public:
    */
   virtual void encode_trim_extra(MonitorDBStore::Transaction *tx, version_t first) {}
 
-  /**
-   * Update our trim status. We do nothing here, because there is no
-   * straightforward way to update the trim version, since that's service
-   * specific. However, we do not force services to implement it, since there
-   * a couple of services that do not trim anything at all, and we don't want
-   * to shove this function down their throats if they are never going to use
-   * it anyway.
-   */
-  virtual void update_trim() { }
-  /**
-   * Set the trim version variable to @p ver
-   *
-   * @param ver The version to trim to.
-   */
-  void set_trim_to(version_t ver) {
-    trim_version = ver;
-  }
   /**
    * Get the version we should trim to.
    *
    * @returns the version we should trim to; if we return zero, it should be
    *	      assumed that there's no version to trim to.
    */
-  version_t get_trim_to() {
-    return trim_version;
+  virtual version_t get_trim_to() {
+    return 0;
   }
   /**
    * @}