objectcacher: remove uncommitted xlist<>; fix broken purge() cleanup

There was a problem where:

 - we would dirty some buffers on an object
   - bump dirty_tx count
 - flush()
   - this adds the Object to ObjectSet::uncommitted
 - truncate
   - client clears FILE_BUFFER cap_ref
   - Object::purge()
     - clear dirty_tx count
 - client puts last inode
   - Object::uncommitted is not empty in ~ObjectSet

(This was triggered after several runs of workunits/suites/blogbensh.sh
on sepia.)

It turns out the uncommitted xlist<> is pretty useless, though: the same
information is captured in the dirty_tx counter.  We add a separate
counter to the Object itself (for the benefit of Object::can_close()).

We also clean up Object::purge() to call truncate(0), a small
simplification.

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

diff --git a/src/client/Client.cc b/src/client/Client.cc
index f461864d706d3596b8aba567512273c7c956b0dd..bb9cafc6cbacefa4d1cfc0574e78353d0310075b 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1959,7 +1959,7 @@ int Client::get_caps(Inode *in, int need, int want, int *got, loff_t endoff)
        int revoking = implemented & ~have;
        ldout(cct, 10) << "get_caps " << *in << " have " << ccap_string(have)
                 << " need " << ccap_string(need) << " want " << ccap_string(want)
-                << " but not  " << ccap_string(butnot) << " revoking " << ccap_string(revoking)
+                << " but not " << ccap_string(butnot) << " revoking " << ccap_string(revoking)
                 << dendl;
        if ((revoking & butnot) == 0) {
          *got = need | (have & want);
@@ -2378,12 +2378,11 @@ public:
   }
 };
 
-
 bool Client::_flush(Inode *in)
 {
   ldout(cct, 10) << "_flush " << *in << dendl;
 
-  if (!in->oset.dirty_tx && in->oset.uncommitted.empty()) {
+  if (!in->oset.dirty_or_tx) {
     ldout(cct, 10) << " nothing to flush" << dendl;
     return true;
   }
@@ -5329,7 +5328,7 @@ int Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf)
 
   if (cct->_conf->client_oc && (got & CEPH_CAP_FILE_BUFFER)) {
     // do buffered write
-    if (!in->oset.dirty_tx && in->oset.uncommitted.empty())
+    if (!in->oset.dirty_or_tx)
       get_cap_ref(in, CEPH_CAP_FILE_BUFFER);
 
     get_cap_ref(in, CEPH_CAP_FILE_BUFFER);

diff --git a/src/client/Inode.cc b/src/client/Inode.cc
index 234542ee5f47f1c5f6c53ee415e8f22ee849f790..47edede4a16683906469052da3f166ef51d9a843 100644 (file)
--- a/src/client/Inode.cc
+++ b/src/client/Inode.cc
@@ -31,6 +31,9 @@ ostream& operator<<(ostream &out, Inode &in)
   if (in.flags & I_COMPLETE)
     out << " COMPLETE";
 
+  if (in.is_file())
+    out << " " << in.oset;
+
   set<Dentry*>::iterator i = in.dn_set.begin();
   while(i != in.dn_set.end()) {
       out << " parent=" << *i;

diff --git a/src/osdc/ObjectCacher.cc b/src/osdc/ObjectCacher.cc
index 3c6474a43f69c117948a21111504a3480c43c762..a23cf97ea6fac9b6e47ccb138fcbb56ad732042a 100644 (file)
--- a/src/osdc/ObjectCacher.cc
+++ b/src/osdc/ObjectCacher.cc
@@ -394,39 +394,35 @@ ObjectCacher::BufferHead *ObjectCacher::Object::map_write(OSDWrite *wr)
   return final;
 }
 
-
 void ObjectCacher::Object::truncate(loff_t s)
 {
-  ldout(oc->cct, 10) << "truncate to " << s << dendl;
-  
+  ldout(oc->cct, 10) << "truncate " << *this << " to " << s << dendl;
+
   while (!data.empty()) {
-       BufferHead *bh = data.rbegin()->second;
-       if (bh->end() <= s) 
-         break;
-       
-       // split bh at truncation point?
-       if (bh->start() < s) {
-         split(bh, s);
-         continue;
-       }
+    BufferHead *bh = data.rbegin()->second;
+    if (bh->end() <= s) 
+      break;
+
+    // split bh at truncation point?
+    if (bh->start() < s) {
+      split(bh, s);
+      continue;
+    }
 
-       // remove bh entirely
-       assert(bh->start() >= s);
-       oc->bh_remove(this, bh);
-       delete bh;
+    // remove bh entirely
+    assert(bh->start() >= s);
+    oc->bh_remove(this, bh);
+    delete bh;
   }
 }
 
 
 
-
-
 /*** ObjectCacher ***/
 
 #undef dout_prefix
 #define dout_prefix *_dout << objecter->messenger->get_myname() << ".objectcacher "
 
-
 /* private */
 
 void ObjectCacher::close_object(Object *ob) 
@@ -562,8 +558,6 @@ void ObjectCacher::bh_write(BufferHead *bh)
   oncommit->tid = tid;
   bh->ob->last_write_tid = tid;
   bh->last_write_tid = tid;
-  if (commit_set_callback)
-    oset->uncommitted.push_back(&bh->ob->uncommitted_item);
 
   mark_tx(bh);
 }
@@ -690,7 +684,7 @@ void ObjectCacher::bh_write_ack(int poolid, sobject_t oid, loff_t start, uint64_
     }
 
     // is the entire object set now clean?
-    if (flush_set_callback && ob->oset->dirty_tx == 0) {
+    if (flush_set_callback && ob->oset->dirty_or_tx == 0) {
       flush_set_callback(flush_set_callback_arg, ob->oset);
     }
   }
@@ -727,13 +721,13 @@ void ObjectCacher::bh_write_commit(int poolid, sobject_t oid, loff_t start, uint
     // is the entire object set now clean and fully committed?
     if (commit_set_callback &&
        ob->last_commit_tid == ob->last_write_tid) {
-      ob->uncommitted_item.remove_myself();
       ObjectSet *oset = ob->oset;
       if (ob->can_close())
        close_object(ob);
-      if (oset->uncommitted.empty()) {  // no uncommitted in flight
-       if (oset->dirty_tx == 0)        // AND nothing dirty/tx
+      if (commit_set_callback) {
+       if (oset->dirty_or_tx == 0) {        // nothing dirty/tx
          commit_set_callback(flush_set_callback_arg, oset);      
+       }
       }
     }
   }
@@ -1441,20 +1435,15 @@ void ObjectCacher::purge(Object *ob)
 {
   ldout(cct, 10) << "purge " << *ob << dendl;
 
-  while (!ob->data.empty()) {
-    BufferHead *bh = ob->data.begin()->second;
-    if (!bh->is_clean())
-      ldout(cct, 0) << "purge forcibly removing " << *ob << " " << *bh << dendl;
-    bh_remove(ob, bh);
-    delete bh;
-  }
-  
+  ob->truncate(0);
+
   if (ob->can_close()) {
-    ldout(cct, 10) << "trim trimming " << *ob << dendl;
+    ldout(cct, 10) << "purge closing " << *ob << dendl;
     close_object(ob);
   }
 }
 
+
 // flush.  non-blocking.  no callback.
 // true if clean, already flushed.  
 // false if we wrote something.
@@ -1695,7 +1684,7 @@ void ObjectCacher::truncate_set(ObjectSet *oset, vector<ObjectExtent>& exls)
   
   ldout(cct, 10) << "truncate_set " << oset << dendl;
 
-  bool were_dirty = oset->dirty_tx > 0;
+  bool were_dirty = oset->dirty_or_tx > 0;
 
   for (vector<ObjectExtent>::iterator p = exls.begin();
        p != exls.end();
@@ -1724,7 +1713,7 @@ void ObjectCacher::truncate_set(ObjectSet *oset, vector<ObjectExtent>& exls)
 
   // did we truncate off dirty data?
   if (commit_set_callback &&
-      were_dirty && oset->dirty_tx == 0)
+      were_dirty && oset->dirty_or_tx == 0)
     commit_set_callback(flush_set_callback_arg, oset);
 }
 

diff --git a/src/osdc/ObjectCacher.h b/src/osdc/ObjectCacher.h
index f0affa03bd06dc27675c75c658bb5a0080b435c3..73fd7abd35804a136503e522dcd50d46984e6822 100644 (file)
--- a/src/osdc/ObjectCacher.h
+++ b/src/osdc/ObjectCacher.h
@@ -148,13 +148,13 @@ class ObjectCacher {
     tid_t last_ack_tid;    // last update acked.
     tid_t last_commit_tid; // last update commited.
 
+    int dirty_or_tx;
+
     map< tid_t, list<Context*> > waitfor_ack;
     map< tid_t, list<Context*> > waitfor_commit;
     list<Context*> waitfor_rd;
     list<Context*> waitfor_wr;
 
-    xlist<Object*>::item uncommitted_item;
-
     // lock
     static const int LOCK_NONE = 0;
     static const int LOCK_WRLOCKING = 1;
@@ -177,13 +177,14 @@ class ObjectCacher {
       oc(_oc),
       oid(o), oset(os), set_item(this), oloc(l),
       last_write_tid(0), last_ack_tid(0), last_commit_tid(0),
-      uncommitted_item(this),
+      dirty_or_tx(0),
       lock_state(LOCK_NONE), wrlock_ref(0), rdlock_ref(0) {
       // add to set
       os->objects.push_back(&set_item);
     }
     ~Object() {
       assert(data.empty());
+      assert(dirty_or_tx == 0);
       set_item.remove_myself();
     }
 
@@ -199,7 +200,7 @@ class ObjectCacher {
       return data.empty() && lock_state == LOCK_NONE &&
         waitfor_ack.empty() && waitfor_commit.empty() &&
         waitfor_rd.empty() && waitfor_wr.empty() &&
-       !uncommitted_item.is_on_list();
+       dirty_or_tx == 0;
     }
 
     // bh
@@ -257,12 +258,11 @@ class ObjectCacher {
 
     int poolid;
     xlist<Object*> objects;
-    xlist<Object*> uncommitted;
 
-    int dirty_tx;
+    int dirty_or_tx;
 
     ObjectSet(void *p, int _poolid, inodeno_t i) : parent(p), ino(i), truncate_seq(0),
-                                      truncate_size(0), poolid(_poolid), dirty_tx(0) {}
+                                      truncate_size(0), poolid(_poolid), dirty_or_tx(0) {}
   };
 
 
@@ -344,11 +344,13 @@ class ObjectCacher {
       break;
     case BufferHead::STATE_DIRTY: 
       stat_dirty += bh->length(); 
-      bh->ob->oset->dirty_tx += bh->length();
+      bh->ob->dirty_or_tx += bh->length();
+      bh->ob->oset->dirty_or_tx += bh->length();
       break;
     case BufferHead::STATE_TX: 
       stat_tx += bh->length(); 
-      bh->ob->oset->dirty_tx += bh->length();
+      bh->ob->dirty_or_tx += bh->length();
+      bh->ob->oset->dirty_or_tx += bh->length();
       break;
     case BufferHead::STATE_RX:
       stat_rx += bh->length();
@@ -366,11 +368,13 @@ class ObjectCacher {
       break;
     case BufferHead::STATE_DIRTY: 
       stat_dirty -= bh->length(); 
-      bh->ob->oset->dirty_tx -= bh->length();
+      bh->ob->dirty_or_tx -= bh->length();
+      bh->ob->oset->dirty_or_tx -= bh->length();
       break;
     case BufferHead::STATE_TX: 
       stat_tx -= bh->length(); 
-      bh->ob->oset->dirty_tx -= bh->length();
+      bh->ob->dirty_or_tx -= bh->length();
+      bh->ob->oset->dirty_or_tx -= bh->length();
       break;
     case BufferHead::STATE_RX:
       stat_rx -= bh->length();
@@ -676,6 +680,15 @@ inline ostream& operator<<(ostream& out, ObjectCacher::BufferHead &bh)
   return out;
 }
 
+inline ostream& operator<<(ostream& out, ObjectCacher::ObjectSet &os)
+{
+  return out << "objectset[" << os.ino
+            << " ts " << os.truncate_seq << "/" << os.truncate_size
+            << " objects " << os.objects.size()
+            << " dirty_or_tx " << os.dirty_or_tx
+            << "]";
+}
+
 inline ostream& operator<<(ostream& out, ObjectCacher::Object &ob)
 {
   out << "object["