From: Sage Weil <sage@inktank.com>
Date: Fri, 7 Mar 2014 00:12:30 +0000 (-0800)
Subject: osd: fix agent thread shutdown
X-Git-Tag: v0.78~70^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=09668a49584d0738a50f6536910a6be198cfb14d;p=ceph.git

osd: fix agent thread shutdown

We had an old invariant that agent_queue would have at least 1 entry in
it to simplify some other code paths, but it turns out that it is simpler
not to do that.

In particular, this was triggering a failed assertion on shutdown when we
assert that the queue is empty.

Dump offending items on shutdown if they are there, tho, to catch any
future bugs.

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

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index f229bc08503e..8aee20185f99 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -471,11 +471,12 @@ void OSDService::agent_entry()
   dout(10) << __func__ << " start" << dendl;
   agent_lock.Lock();
 
-  // stick at least one level in there to simplify other paths
-  if (agent_queue.empty())
-    agent_queue[0];
-
   while (!agent_stop_flag) {
+    if (agent_queue.empty()) {
+      dout(20) << __func__ << " empty queue" << dendl;
+      agent_cond.Wait(agent_lock);
+      continue;
+    }
     uint64_t level = agent_queue.rbegin()->first;
     set<PGRef>& top = agent_queue.rbegin()->second;
     dout(10) << __func__
@@ -515,7 +516,11 @@ void OSDService::agent_stop()
     // By this time all ops should be cancelled
     assert(agent_ops == 0);
     // By this time all PGs are shutdown and dequeued
-    assert(agent_queue.empty());
+    if (!agent_queue.empty()) {
+      set<PGRef>& top = agent_queue.rbegin()->second;
+      derr << "agent queue not empty, for example " << (*top.begin())->info.pgid << dendl;
+      assert(0 == "agent queue not empty");
+    }
 
     agent_stop_flag = true;
     agent_cond.Signal();
diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index b3e290aab6ea..7ee3442793c7 100644
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -487,7 +487,7 @@ public:
     if (p == agent_queue_pos)
       ++agent_queue_pos;
     oq.erase(p);
-    if (oq.empty() && agent_queue.size() > 1) {
+    if (oq.empty()) {
       if (agent_queue.rbegin()->first == old_priority)
 	agent_valid_iterator = false;
       agent_queue.erase(old_priority);