interval_set: hide data members

This change makes interval_set::m and interval_set::_size private data
members in interval_set, instead of public. This change also creates a
non-const iterator. Using this iterator, users can modify the length of
an interval. So now, all users can use the iterators rather than
interacting with the class internals directly.

diff --git a/src/include/interval_set.h b/src/include/interval_set.h
index 4617a724415941aa031642199d29f152db033e47..fa824e5ffbcdd130bfeb7865ce8a1d4dbb561035 100644 (file)
--- a/src/include/interval_set.h
+++ b/src/include/interval_set.h
@@ -34,9 +34,65 @@ using namespace std;
 template<typename T>
 class interval_set {
  public:
-  map<T,T> m;   // map start -> len  
-  int64_t _size;
-  
+  class iterator : public std::iterator <std::forward_iterator_tag, T>
+  {
+    public:
+        explicit iterator(typename std::map<T,T>::iterator iter)
+          : _iter(iter)
+        { }
+
+        // For the copy constructor and assignment operator, the compiler-generated functions, which
+        // perform simple bitwise copying, should be fine.
+
+        bool operator==(const iterator& rhs) const {
+          return (_iter == rhs._iter);
+        }
+
+        bool operator!=(const iterator& rhs) const {
+          return (_iter != rhs._iter);
+        }
+
+        // Dereference this iterator to get a pair.
+        pair < T, T > &operator*() {
+                return *_iter;
+        }
+
+        // Return the interval start.
+        T get_start() const {
+                return _iter->first;
+        }
+
+        // Return the interval length.
+        T get_len() const {
+                return _iter->second;
+        }
+
+        // Set the interval length.
+        void set_len(T len) {
+                _iter->second = len;
+        }
+
+        // Preincrement
+        iterator &operator++()
+        {
+                ++_iter;
+                return *this;
+        }
+
+        // Postincrement
+        iterator operator++(int)
+        {
+                iterator prev(_iter);
+                ++_iter;
+                return prev;
+        }
+
+    friend class interval_set<T>::const_iterator;
+
+    protected:
+        typename map<T,T>::iterator _iter;
+  };
+
   class const_iterator : public std::iterator <std::forward_iterator_tag, T>
   {
     public:
@@ -44,6 +100,10 @@ class interval_set {
           : _iter(iter)
         { }
 
+        const_iterator(const iterator &i)
+         : _iter(i._iter)
+        { }
+
         // For the copy constructor and assignment operator, the compiler-generated functions, which
         // perform simple bitwise copying, should be fine.
 
@@ -56,17 +116,17 @@ class interval_set {
         }
 
         // Dereference this iterator to get a pair.
-        pair < T, T > &operator*() {
+        pair < T, T > operator*() const {
                 return *_iter;
         }
 
         // Return the interval start.
-        const T& get_start() const {
+        T get_start() const {
                 return _iter->first;
         }
 
         // Return the interval length.
-        const T& get_len() const {
+        T get_len() const {
                 return _iter->second;
         }
 
@@ -91,13 +151,24 @@ class interval_set {
 
   interval_set() : _size(0) {}
 
-  typename interval_set<T>::const_iterator begin() const
+  int num_intervals() const
   {
+    return m.size();
+  }
+
+  typename interval_set<T>::iterator begin() {
+    return typename interval_set<T>::iterator(m.begin());
+  }
+
+  typename interval_set<T>::iterator end() {
+    return typename interval_set<T>::iterator(m.end());
+  }
+
+  typename interval_set<T>::const_iterator begin() const {
     return typename interval_set<T>::const_iterator(m.begin());
   }
 
-  typename interval_set<T>::const_iterator end() const
-  {
+  typename interval_set<T>::const_iterator end() const {
     return typename interval_set<T>::const_iterator(m.end());
   }
 
@@ -434,6 +505,10 @@ class interval_set {
     }
   }
 
+private:
+  // data
+  int64_t _size;
+  map<T,T> m;   // map start -> len
 };
 
 

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 343b8a17501a2e3a5a3f5d019b184562beaf0a2a..542005f8d7f55ed5e4ee3a83d10738ead00b12b7 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -1650,9 +1650,9 @@ void ReplicatedPG::make_writeable(OpContext *ctx)
 
 void ReplicatedPG::add_interval_usage(interval_set<uint64_t>& s, pg_stat_t& stats)
 {
-  for (map<uint64_t,uint64_t>::iterator p = s.m.begin(); p != s.m.end(); p++) {
-    stats.num_bytes += p->second;
-    stats.num_kb += SHIFT_ROUND_UP(p->first+p->second, 10) - (p->first >> 10);
+  for (interval_set<uint64_t>::const_iterator p = s.begin(); p != s.end(); ++p) {
+    stats.num_bytes += p.get_len();
+    stats.num_kb += SHIFT_ROUND_UP(p.get_start() + p.get_len(), 10) - (p.get_start() >> 10);
   }
 }
 
@@ -2894,15 +2894,16 @@ void ReplicatedPG::send_push_op(const sobject_t& soid, int peer,
   bufferlist bl;
   map<string,bufferptr> attrset;
 
-  for (map<uint64_t,uint64_t>::iterator p = data_subset.m.begin();
-       p != data_subset.m.end();
-       p++) {
+  for (interval_set<uint64_t>::iterator p = data_subset.begin();
+       p != data_subset.end();
+       ++p) {
     bufferlist bit;
-    osd->store->read(coll_t::build_pg_coll(info.pgid), soid, p->first, p->second, bit);
-    if (p->second != bit.length()) {
-      dout(10) << " extent " << p->first << "~" << p->second
-              << " is actually " << p->first << "~" << bit.length() << dendl;
-      p->second = bit.length();
+    osd->store->read(coll_t::build_pg_coll(info.pgid),
+                    soid, p.get_start(), p.get_len(), bit);
+    if (p.get_len() != bit.length()) {
+      dout(10) << " extent " << p.get_start() << "~" << p.get_len()
+              << " is actually " << p.get_start() << "~" << bit.length() << dendl;
+      p.set_len(bit.length());
     }
     bl.claim_append(bit);
   }
@@ -3178,23 +3179,23 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op)
 
        bufferlist result;
        int off = 0;
-       for (map<uint64_t,uint64_t>::iterator p = data_subset.m.begin();
-            p != data_subset.m.end();
-            p++) {
+       for (interval_set<uint64_t>::const_iterator p = data_subset.begin();
+            p != data_subset.end();
+            ++p) {
          interval_set<uint64_t> x;
-         x.insert(p->first, p->second);
+         x.insert(p.get_start(), p.get_len());
          x.intersection_of(data_needed);
-         dout(20) << " data_subset object extent " << p->first << "~" << p->second << " need " << x << dendl;
+         dout(20) << " data_subset object extent " << p.get_start() << "~" << p.get_len() << " need " << x << dendl;
          if (!x.empty()) {
-           uint64_t first = x.m.begin()->first;
-           uint64_t len = x.m.begin()->second;
+           uint64_t first = x.begin().get_start();
+           uint64_t len = x.begin().get_len();
            bufferlist sub;
-           int boff = off + (first - p->first);
+           int boff = off + (first - p.get_start());
            dout(20) << "   keeping buffer extent " << boff << "~" << len << dendl;
            sub.substr_of(data, boff, len);
            result.claim_append(sub);
          }
-         off += p->second;
+         off += p.get_len();
        }
        data.claim(result);
        dout(20) << " new data len is " << data.length() << dendl;
@@ -3255,14 +3256,14 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op)
 
   // write data
   uint64_t boff = 0;
-  for (map<uint64_t,uint64_t>::iterator p = data_subset.m.begin();
-       p != data_subset.m.end(); 
-       p++) {
+  for (interval_set<uint64_t>::const_iterator p = data_subset.begin();
+       p != data_subset.end();
+       ++p) {
     bufferlist bit;
-    bit.substr_of(data, boff, p->second);
-    dout(15) << " write " << p->first << "~" << p->second << dendl;
-    t->write(target, soid, p->first, p->second, bit);
-    boff += p->second;
+    bit.substr_of(data, boff, p.get_len());
+    dout(15) << " write " << p.get_start() << "~" << p.get_len() << dendl;
+    t->write(target, soid, p.get_start(), p.get_len(), bit);
+    boff += p.get_len();
   }
   
   if (complete) {
@@ -3273,15 +3274,20 @@ void ReplicatedPG::sub_op_push(MOSDSubOp *op)
     }
 
     // clone bits
-    for (map<sobject_t, interval_set<uint64_t> >::iterator p = clone_subsets.begin();
+    for (map<sobject_t, interval_set<uint64_t> >::const_iterator p = clone_subsets.begin();
         p != clone_subsets.end();
-        p++)
-      for (map<uint64_t,uint64_t>::iterator q = p->second.m.begin();
-          q != p->second.m.end(); 
-        q++) {
-       dout(15) << " clone_range " << p->first << " " << q->first << "~" << q->second << dendl;
-       t->clone_range(coll_t::build_pg_coll(info.pgid), p->first, soid, q->first, q->second);
+        ++p)
+    {
+      for (interval_set<uint64_t>::const_iterator q = p->second.begin();
+          q != p->second.end();
+          ++q)
+      {
+       dout(15) << " clone_range " << p->first << " "
+                << q.get_start() << "~" << q.get_len() << dendl;
+       t->clone_range(coll_t::build_pg_coll(info.pgid), p->first, soid,
+                      q.get_start(), q.get_len());
       }
+    }
 
     if (data_subset.empty())
       t->touch(coll_t::build_pg_coll(info.pgid), soid);
@@ -3877,12 +3883,12 @@ int ReplicatedPG::_scrub(ScrubMap& scrubmap, int& errors, int& fixed)
       // subtract off any clone overlap
       for (map<snapid_t,interval_set<uint64_t> >::iterator q = snapset.clone_overlap.begin();
           q != snapset.clone_overlap.end();
-          q++) {
-       for (map<uint64_t,uint64_t>::iterator r = q->second.m.begin();
-            r != q->second.m.end();
-            r++) {
-         stat.num_bytes -= r->second;
-         stat.num_kb -= SHIFT_ROUND_UP(r->first+r->second, 10) - (r->first >> 10);
+          ++q) {
+       for (interval_set<uint64_t>::const_iterator r = q->second.begin();
+            r != q->second.end();
+            ++r) {
+         stat.num_bytes -= r.get_len();
+         stat.num_kb -= SHIFT_ROUND_UP(r.get_start()+r.get_len(), 10) - (r.get_start() >> 10);
        }         
       }
     }

diff --git a/src/osd/osd_types.h b/src/osd/osd_types.h
index 4eedb91bd82e3abf27cea641edcdcb364778efdc..744abd1f9d7cb3b0451c62677958c3931e21d152 100644 (file)
--- a/src/osd/osd_types.h
+++ b/src/osd/osd_types.h
@@ -798,7 +798,7 @@ struct pg_pool_t {
     __u8 struct_v = CEPH_PG_POOL_VERSION;
     ::encode(struct_v, bl);
     v.num_snaps = snaps.size();
-    v.num_removed_snap_intervals = removed_snaps.m.size();
+    v.num_removed_snap_intervals = removed_snaps.num_intervals();
     ::encode(v, bl);
     ::encode_nohead(snaps, bl);
     removed_snaps.encode_nohead(bl);