Ignore and warn about attempts to reconfigure static Rock store options.

Some Rock store options cannot be changed dynamically: path, size, and
max-size.  Before the change, there were no checks during reconfigure
to prevent changing these options.  This may lead to Rock cache
corruption and other bugs.  The patch adds necessary checks to Rock
store code.  If user tries to change an option that cannot be updated
dynamically, a warning is reported and the value is left unchanged.

diff --git a/src/SwapDir.cc b/src/SwapDir.cc
index cb0fcf3c605715a2319686dfb7811f37923471fc..12ee586b573831d559b4515f22a0e59bca1fbc60 100644 (file)
--- a/src/SwapDir.cc
+++ b/src/SwapDir.cc
@@ -310,8 +310,17 @@ SwapDir::optionObjectSizeParse(char const *option, const char *value, int isaRec
 
     int64_t size = strtoll(value, NULL, 10);
 
-    if (isaReconfig && *val != size)
-        debugs(3, 1, "Cache dir '" << path << "' object " << option << " now " << size);
+    if (isaReconfig && *val != size) {
+        if (allowOptionReconfigure(option)) {
+            debugs(3, DBG_IMPORTANT, "cache_dir '" << path << "' object " <<
+                   option << " now " << size << " Bytes");
+        } else {
+            debugs(3, DBG_IMPORTANT, "WARNING: cache_dir '" << path << "' "
+                   "object " << option << " cannot be changed dynamically, " <<
+                   "value left unchanged (" << *val << " Bytes)");
+            return true;
+        }
+    }
 
     *val = size;
 

diff --git a/src/SwapDir.h b/src/SwapDir.h
index 4e5558491cdf2ad13aa67608b411b585203429ab..42058d165822e26378ab1a998d07370f5fdfb6a7 100644 (file)
--- a/src/SwapDir.h
+++ b/src/SwapDir.h
@@ -162,6 +162,7 @@ protected:
     void parseOptions(int reconfiguring);
     void dumpOptions(StoreEntry * e) const;
     virtual ConfigOption *getOptionTree() const;
+    virtual bool allowOptionReconfigure(const char *const) const { return true; }
 
     int64_t sizeInBlocks(const int64_t size) const { return (size + fs.blksize - 1) / fs.blksize; }
 

diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index b580f0b4d73a812b7f2d9af598b3f61a4ba365bb..7522df9e29dcbd56f2f8cb0ef16e6ff984ebe8d6 100644 (file)
--- a/src/fs/rock/RockSwapDir.cc
+++ b/src/fs/rock/RockSwapDir.cc
@@ -254,7 +254,7 @@ Rock::SwapDir::parse(int anIndex, char *aPath)
     fname.append("/rock");
     filePath = xstrdup(fname.termedBuf());
 
-    parseSize();
+    parseSize(false);
     parseOptions(0);
 
     // Current openForWriting() code overwrites the old slot if needed
@@ -265,11 +265,9 @@ Rock::SwapDir::parse(int anIndex, char *aPath)
 }
 
 void
-Rock::SwapDir::reconfigure(int, char *)
+Rock::SwapDir::reconfigure(int, char *aPath)
 {
-    // TODO: do not update a parameter if we cannot propagate that change
-    // TODO: warn if reconfigure changes any parameter that we cannot update
-    parseSize();
+    parseSize(true);
     parseOptions(1);
     // TODO: can we reconfigure the replacement policy (repl)?
     validateOptions();
@@ -277,12 +275,20 @@ Rock::SwapDir::reconfigure(int, char *)
 
 /// parse maximum db disk size
 void
-Rock::SwapDir::parseSize()
+Rock::SwapDir::parseSize(const bool reconfiguring)
 {
     const int i = GetInteger();
     if (i < 0)
         fatal("negative Rock cache_dir size value");
-    max_size = static_cast<uint64_t>(i) << 20; // MBytes to Bytes
+    const uint64_t new_max_size =
+        static_cast<uint64_t>(i) << 20; // MBytes to Bytes
+    if (!reconfiguring)
+        max_size = new_max_size;
+    else if (new_max_size != max_size) {
+        debugs(3, DBG_IMPORTANT, "WARNING: cache_dir '" << path << "' size "
+               "cannot be changed dynamically, value left unchanged (" <<
+               (max_size >> 20) << " MB)");
+    }
 }
 
 ConfigOption *
@@ -294,6 +300,13 @@ Rock::SwapDir::getOptionTree() const
     return vector;
 }
 
+bool
+Rock::SwapDir::allowOptionReconfigure(const char *const option) const
+{
+    return strcmp(option, "max-size") != 0 &&
+        ::SwapDir::allowOptionReconfigure(option);
+}
+
 /// parses time-specific options; mimics ::SwapDir::optionObjectSizeParse()
 bool
 Rock::SwapDir::parseTimeOption(char const *option, const char *value, int reconfiguring)

diff --git a/src/fs/rock/RockSwapDir.h b/src/fs/rock/RockSwapDir.h
index a39eaf9ed91f95e69e7911e891547d3a6af9b57a..92da4095346da65d421ad1036d77eddb3b2ff9fb 100644 (file)
--- a/src/fs/rock/RockSwapDir.h
+++ b/src/fs/rock/RockSwapDir.h
@@ -45,6 +45,7 @@ protected:
     virtual void create();
     virtual void init();
     virtual ConfigOption *getOptionTree() const;
+    virtual bool allowOptionReconfigure(const char *const option) const;
     virtual bool canStore(const StoreEntry &e, int64_t diskSpaceNeeded, int &load) const;
     virtual StoreIOState::Pointer createStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
     virtual StoreIOState::Pointer openStoreIO(StoreEntry &, StoreIOState::STFNCB *, StoreIOState::STIOCB *, void *);
@@ -62,7 +63,7 @@ protected:
     virtual void writeCompleted(int errflag, size_t len, RefCount< ::WriteRequest>);
 
     virtual void parse(int index, char *path);
-    void parseSize(); ///< parses anonymous cache_dir size option
+    void parseSize(const bool reconfiguring); ///< parses anonymous cache_dir size option
     void validateOptions(); ///< warns of configuration problems; may quit
     bool parseTimeOption(char const *option, const char *value, int reconfiguring);
     void dumpTimeOption(StoreEntry * e) const;