From: Josh Durgin <josh.durgin@inktank.com>
Date: Fri, 22 Mar 2013 19:13:36 +0000 (-0700)
Subject: ObjectCacher: always complete flush_set() callback
X-Git-Tag: v0.56.5~12^2~16
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d86f9b1dd370563170ab875c6a7de511f05acbf7;p=ceph.git

ObjectCacher: always complete flush_set() callback

This removes the last remnants of
b5e9995f59d363ba00d9cac413d9b754ee44e370. If there's nothing to flush,
immediately call the callback instead of deleting it. Callers were
assuming they were responsible for completing the callback whenever
flush_set() returned true, and always called complete(0) in this
case. Simplify the interface and just do this in flush_set(), so that
it always calls the callback.

Since C_GatherBuilder deletes its finisher if there are no subs,
only set its finisher when subs are present. This way we can still
call ->complete() for the callback.

Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 92db06c05dc2cad8ed31648cb08866781aee2855)

Conflicts:

	src/client/Client.cc
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index e233a454ae45..2da7774bc798 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -2581,11 +2581,7 @@ bool Client::_flush(Inode *in)
   }
 
   Context *onfinish = new C_Client_PutInode(this, in);
-  bool safe = objectcacher->flush_set(&in->oset, onfinish);
-  if (safe) {
-    onfinish->complete(0);
-  }
-  return safe;
+  return objectcacher->flush_set(&in->oset, onfinish);
 }
 
 void Client::flush_set_callback(ObjectCacher::ObjectSet *oset)
diff --git a/src/osdc/ObjectCacher.cc b/src/osdc/ObjectCacher.cc
index 2017e6b6d7f6..c33d7fd4f91d 100644
--- a/src/osdc/ObjectCacher.cc
+++ b/src/osdc/ObjectCacher.cc
@@ -1503,14 +1503,14 @@ bool ObjectCacher::flush_set(ObjectSet *oset, Context *onfinish)
   assert(lock.is_locked());
   if (oset->objects.empty()) {
     ldout(cct, 10) << "flush_set on " << oset << " dne" << dendl;
-    delete onfinish;
+    onfinish->complete(0);
     return true;
   }
 
   ldout(cct, 10) << "flush_set " << oset << dendl;
 
   // we'll need to wait for all objects to flush!
-  C_GatherBuilder gather(cct, onfinish);
+  C_GatherBuilder gather(cct);
 
   bool safe = true;
   for (xlist<Object*>::iterator i = oset->objects.begin();
@@ -1529,11 +1529,14 @@ bool ObjectCacher::flush_set(ObjectSet *oset, Context *onfinish)
         ob->waitfor_commit[ob->last_write_tid].push_back(gather.new_sub());
     }
   }
+  if (gather.has_subs())
+    gather.set_finisher(onfinish);
   if (onfinish != NULL)
     gather.activate();
   
   if (safe) {
     ldout(cct, 10) << "flush_set " << oset << " has no dirty|tx bhs" << dendl;
+    onfinish->complete(0);
     return true;
   }
   return false;
@@ -1546,14 +1549,14 @@ bool ObjectCacher::flush_set(ObjectSet *oset, vector<ObjectExtent>& exv, Context
   assert(lock.is_locked());
   if (oset->objects.empty()) {
     ldout(cct, 10) << "flush_set on " << oset << " dne" << dendl;
-    delete onfinish;
+    onfinish->complete(0);
     return true;
   }
 
   ldout(cct, 10) << "flush_set " << oset << " on " << exv.size() << " ObjectExtents" << dendl;
 
   // we'll need to wait for all objects to flush!
-  C_GatherBuilder gather(cct, onfinish);
+  C_GatherBuilder gather(cct);
 
   bool safe = true;
   for (vector<ObjectExtent>::iterator p = exv.begin();
@@ -1577,11 +1580,14 @@ bool ObjectCacher::flush_set(ObjectSet *oset, vector<ObjectExtent>& exv, Context
         ob->waitfor_commit[ob->last_write_tid].push_back(gather.new_sub());
     }
   }
+  if (gather.has_subs())
+    gather.set_finisher(onfinish);
   if (onfinish != NULL)
     gather.activate();
   
   if (safe) {
     ldout(cct, 10) << "flush_set " << oset << " has no dirty|tx bhs" << dendl;
+    onfinish->complete(0);
     return true;
   }
   return false;
diff --git a/src/test/osdc/object_cacher_stress.cc b/src/test/osdc/object_cacher_stress.cc
index e2c58e8da9eb..ef713ae98cc9 100644
--- a/src/test/osdc/object_cacher_stress.cc
+++ b/src/test/osdc/object_cacher_stress.cc
@@ -147,13 +147,11 @@ int stress_test(uint64_t num_ops, uint64_t num_objs,
   bool already_flushed = obc.flush_set(&object_set, onfinish);
   std::cout << "already flushed = " << already_flushed << std::endl;
   lock.Unlock();
-  if (!already_flushed) {
-    mylock.Lock();
-    while (!done) {
-      cond.Wait(mylock);
-    }
-    mylock.Unlock();
+  mylock.Lock();
+  while (!done) {
+    cond.Wait(mylock);
   }
+  mylock.Unlock();
 
   lock.Lock();
   bool unclean = obc.release_set(&object_set);