From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Mon, 7 Feb 2011 17:59:28 +0000 (-0700)
Subject: Made Rock::Rebuild an AsyncJob because it is.
X-Git-Tag: take02~8
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=078274f6c5d9547ffc6cfb2fcde9cf04ba3d8e14;p=thirdparty%2Fsquid.git

Made Rock::Rebuild an AsyncJob because it is.

Increment StoreController::store_dirs_rebuilding early, when SwapDir is
created and before the disk db file is opened and the actual rebuild starts.
Otherwise, if one SwapDir finishes rebuild before others start,
storeRebuildComplete() will see StoreController::store_dirs_rebuilding equal
to one, and think the rebuild is over.

This was not a problem for cache_dirs using blocking I/O because they either
did not try to open some file at SwapDir::init() time or did so synchronously,
resulting in "immediate" StoreController::store_dirs_rebuilding increment from
Store init loop point of view.
---

diff --git a/src/fs/rock/RockRebuild.cc b/src/fs/rock/RockRebuild.cc
index 6778c12b4a..b28bee415b 100644
--- a/src/fs/rock/RockRebuild.cc
+++ b/src/fs/rock/RockRebuild.cc
@@ -9,7 +9,7 @@
 
 CBDATA_NAMESPACED_CLASS_INIT(Rock, Rebuild);
 
-Rock::Rebuild::Rebuild(SwapDir *dir):
+Rock::Rebuild::Rebuild(SwapDir *dir): AsyncJob("Rock::Rebuild"),
     sd(dir),
     dbSize(0),
     dbEntrySize(0),
@@ -34,6 +34,14 @@ Rock::Rebuild::~Rebuild()
 /// prepares and initiates entry loading sequence
 void
 Rock::Rebuild::start() {
+    // in SMP mode, only the disker is responsible for populating the map
+    if (UsingSmp() && !IamDiskProcess()) {
+        debugs(47, 2, "Non-disker skips rebuilding of cache_dir #" <<
+           sd->index << " from " << sd->filePath);
+        mustStop("non-disker");
+        return;
+    }
+
     debugs(47, DBG_IMPORTANT, "Loading cache_dir #" << sd->index <<
            " from " << sd->filePath);
 
@@ -51,20 +59,25 @@ Rock::Rebuild::start() {
     checkpoint();
 }
 
-/// quits if done; otherwise continues after a pause
+/// continues after a pause if not done
 void
 Rock::Rebuild::checkpoint()
 {
-    if (dbOffset < dbSize)
+    if (!done())
         eventAdd("Rock::Rebuild", Rock::Rebuild::Steps, this, 0.01, 1, true);
-    else
-        complete();
+}
+
+bool
+Rock::Rebuild::doneAll() const
+{
+    return dbOffset >= dbSize && AsyncJob::doneAll();
 }
 
 void
 Rock::Rebuild::Steps(void *data)
 {
-    static_cast<Rebuild*>(data)->steps();
+    // use async call to enable job call protection that time events lack
+    CallJobHere(47, 5, static_cast<Rebuild*>(data), Rock::Rebuild, steps);
 }
 
 void
@@ -116,12 +129,11 @@ Rock::Rebuild::doOneEntry() {
 }
 
 void
-Rock::Rebuild::complete() {
-    debugs(47,3, HERE << sd->index);
-    close(fd);
-    StoreController::store_dirs_rebuilding--;
+Rock::Rebuild::swanSong() {
+    debugs(47,3, HERE << "cache_dir #" << sd->index << " rebuild level: " <<
+        StoreController::store_dirs_rebuilding);
+    --StoreController::store_dirs_rebuilding;
     storeRebuildComplete(&counts);
-    delete this; // same as cbdataFree
 }
 
 void
diff --git a/src/fs/rock/RockRebuild.h b/src/fs/rock/RockRebuild.h
index 1f7a9fe991..f9bd43ab2a 100644
--- a/src/fs/rock/RockRebuild.h
+++ b/src/fs/rock/RockRebuild.h
@@ -2,6 +2,7 @@
 #define SQUID_FS_ROCK_REBUILD_H
 
 #include "config.h"
+#include "base/AsyncJob.h"
 #include "structs.h"
 
 namespace Rock {
@@ -10,19 +11,21 @@ class SwapDir;
 
 /// \ingroup Rock
 /// manages store rebuild process: loading meta information from db on disk
-class Rebuild {
+class Rebuild: public AsyncJob {
 public:
     Rebuild(SwapDir *dir);
     ~Rebuild();
-    void start();
 
-private:
-    CBDATA_CLASS2(Rebuild);
+protected:
+    /* AsyncJob API */
+    virtual void start();
+    virtual bool doneAll() const;
+    virtual void swanSong();
 
+private:
     void checkpoint();
     void steps();
     void doOneEntry();
-    void complete();
     void failure(const char *msg, int errNo = 0);
 
     SwapDir *sd;
@@ -38,6 +41,8 @@ private:
     struct _store_rebuild_data counts;
 
     static void Steps(void *data);
+
+    CBDATA_CLASS2(Rebuild);
 };
 
 } // namespace Rock
diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index 16ea34c474..f131875566 100644
--- a/src/fs/rock/RockSwapDir.cc
+++ b/src/fs/rock/RockSwapDir.cc
@@ -192,6 +192,11 @@ Rock::SwapDir::init()
 
     theFile = io->newFile(filePath);
     theFile->open(O_RDWR, 0644, this);
+
+    // Increment early. Otherwise, if one SwapDir finishes rebuild before
+    // others start, storeRebuildComplete() will think the rebuild is over!
+    // TODO: move store_dirs_rebuilding hack to store modules that need it.
+    ++StoreController::store_dirs_rebuilding;
 }
 
 bool
@@ -288,13 +293,8 @@ Rock::SwapDir::validateOptions()
 
 void
 Rock::SwapDir::rebuild() {
-    // in SMP mode, only the disker is responsible for populating the map
-    if (UsingSmp() && !IamDiskProcess())
-        return;
-
-    ++StoreController::store_dirs_rebuilding;
-    Rebuild *r = new Rebuild(this);
-    r->start(); // will delete self when done
+    //++StoreController::store_dirs_rebuilding; // see Rock::SwapDir::init()
+    AsyncJob::Start(new Rebuild(this));
 }
 
 /* Add a new object to the cache with empty memory copy and pointer to disk