Moved squid.conf global disk_io_timeout to cache_dir-local swap-timeout.

The I/O timeout option belongs to cache_dir because not all cache_dir types
support it and because different cache_dirs may need different timeout values,
especially if some of them handle very large or otherwise unusual files.

To propagate the knowledge of the option down to DiskIO/IpcIoFile I decided
to add a DiskFile::Config class and DiskFile::configure() method. At first
glance that API does not belong to DiskFile because only IpcIoFile supports
it. However, DiskFile may be a better location for it because

  * Other specific DiskIO files may want to support the same configuration
    API.

  * Placing API in IpcIoFile would require either making Rock Store dependent
    on IpcIoFile (in terms of linking and in terms of availability) or more
    complex API with multiple inheritance, dynamic casting and such. We can
    introduce the "more complex API" mentioned above later if needed.

Renamed "disk_io" to "swap" timeout because the option is about the whole swap
out/in delay (something an admin may care about) and not individual I/O
(something only low-level code should know about).

diff --git a/src/DiskIO/DiskFile.h b/src/DiskIO/DiskFile.h
index 0b820765ba3efdce7b8a1f63159907f4c27121cb..612badc748300d528b41ff2e420de998eb71a658 100644 (file)
--- a/src/DiskIO/DiskFile.h
+++ b/src/DiskIO/DiskFile.h
@@ -47,8 +47,21 @@ class DiskFile : public RefCountable
 {
 
 public:
+
+    /// generally useful configuration options supported by some children
+    class Config {
+    public:
+        Config(): ioTimeout(0) {}
+
+        /// canRead/Write should return false if expected I/O delay exceeds it
+        time_msec_t ioTimeout; // not enforced if zero, which is the default
+    };
+
     typedef RefCount<DiskFile> Pointer;
 
+    /// notes supported configuration options; kids must call this first
+    virtual void configure(const Config &cfg) {}
+
     virtual void open(int flags, mode_t mode, RefCount<IORequestor> callback) = 0;
     virtual void create(int flags, mode_t mode, RefCount<IORequestor> callback) = 0;
     virtual void read(ReadRequest *) = 0;

diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc
index 3d50a70b9a7cd21a0c98f1260d48642863a2cbf6..34ebdb90f352b220dfa799a09ccfedbe68671a75 100644 (file)
--- a/src/DiskIO/IpcIo/IpcIoFile.cc
+++ b/src/DiskIO/IpcIo/IpcIoFile.cc
@@ -70,6 +70,13 @@ IpcIoFile::~IpcIoFile()
     }
 }
 
+void
+IpcIoFile::configure(const Config &cfg)
+{
+    DiskFile::configure(cfg);
+    config = cfg;
+}
+
 void
 IpcIoFile::open(int flags, mode_t mode, RefCount<IORequestor> callback)
 {
@@ -345,7 +352,7 @@ IpcIoFile::push(IpcIoPendingRequest *const pending)
 bool
 IpcIoFile::canWait() const
 {
-    if (!Config.Timeout.disk_io)
+    if (!config.ioTimeout)
         return true; // no timeout specified
 
     IpcIoMsg oldestIo;
@@ -354,7 +361,7 @@ IpcIoFile::canWait() const
 
     const int expectedWait = tvSubMsec(oldestIo.start, current_time);
     if (expectedWait < 0 ||
-            static_cast<time_msec_t>(expectedWait) < Config.Timeout.disk_io)
+            static_cast<time_msec_t>(expectedWait) < config.ioTimeout)
         return true; // expected wait time is acceptible
 
     debugs(47,2, HERE << "cannot wait: " << expectedWait <<

diff --git a/src/DiskIO/IpcIo/IpcIoFile.h b/src/DiskIO/IpcIo/IpcIoFile.h
index 4bf39c42438b9e07dbde31dace5ac3c7e58050fd..3a43d593650118f3bd1c8ee329e30b13312ecd91 100644 (file)
--- a/src/DiskIO/IpcIo/IpcIoFile.h
+++ b/src/DiskIO/IpcIo/IpcIoFile.h
@@ -57,6 +57,7 @@ public:
     virtual ~IpcIoFile();
 
     /* DiskFile API */
+    virtual void configure(const Config &cfg);
     virtual void open(int flags, mode_t mode, RefCount<IORequestor> callback);
     virtual void create(int flags, mode_t mode, RefCount<IORequestor> callback);
     virtual void read(ReadRequest *);
@@ -74,6 +75,8 @@ public:
     /// handle queue push notifications from worker or disker
     static void HandleNotification(const Ipc::TypedMsgHdr &msg);
 
+    DiskFile::Config config; ///< supported configuration options
+
 protected:
     friend class IpcIoPendingRequest;
     void openCompleted(const Ipc::StrandSearchResponse *const response);

diff --git a/src/cf.data.pre b/src/cf.data.pre
index d2adc20bdb8f3ac7df4b6fb2a6dad1b5477e79ba..2b3f195aeafb206e29bc1882ec34139b192dbeee 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -2764,6 +2764,14 @@ DOC_START
        slot size is specified in bytes using the max-size option. See
        below for more info on the max-size option.
 
+       swap-timeout=msec: Squid will not start writing a miss to or
+       reading a hit from disk if it estimates that the swap operation
+       will take more than the specified number of milliseconds. By
+       default and when set to zero, disables the disk I/O time limit
+       enforcement. Ignored when using blocking I/O module because
+       blocking synchronous I/O does not allow Squid to estimate the
+       expected swap wait time.
+
        The coss store type:
 
        NP: COSS filesystem in Squid-3 has been deemed too unstable for
@@ -4657,21 +4665,6 @@ DOC_START
        many ident requests going at once.
 DOC_END
 
-NAME: disk_io_timeout
-TYPE: time_msec
-DEFAULT: 0 milliseconds
-LOC: Config.Timeout.disk_io
-DOC_START
-       Squid will not start a new disk I/O operation if it estimates that the
-       operation will take more than the specified disk I/O timeout.
-       Only Rock Store supports this timeout for now.
-
-       By default and when set to zero, disables the disk I/O time limit
-       enforcement.
-DOC_END
-
-
-
 NAME: shutdown_lifetime
 COMMENT: time-units
 TYPE: time_t

diff --git a/src/fs/rock/RockSwapDir.cc b/src/fs/rock/RockSwapDir.cc
index 6277da17bc76ad0dc99cc2f430bb3e2d62def07b..066d7c5da97eaba828e4a99686baf4a54169b3b8 100644 (file)
--- a/src/fs/rock/RockSwapDir.cc
+++ b/src/fs/rock/RockSwapDir.cc
@@ -10,6 +10,7 @@
 #include "MemObject.h"
 #include "SquidMath.h"
 #include "base/RunnersRegistry.h"
+#include "ConfigOption.h"
 #include "DiskIO/DiskIOModule.h"
 #include "DiskIO/DiskIOStrategy.h"
 #include "DiskIO/ReadRequest.h"
@@ -226,6 +227,7 @@ Rock::SwapDir::init()
     }
 
     theFile = io->newFile(filePath);
+    theFile->configure(fileConfig);
     theFile->open(O_RDWR, 0644, this);
 
     // Increment early. Otherwise, if one SwapDir finishes rebuild before
@@ -281,6 +283,55 @@ Rock::SwapDir::parseSize()
     max_size = static_cast<uint64_t>(i) << 20; // MBytes to Bytes
 }
 
+ConfigOption *
+Rock::SwapDir::getOptionTree() const
+{
+    ConfigOptionVector *vector = dynamic_cast<ConfigOptionVector*>(::SwapDir::getOptionTree());
+    assert(vector);
+    vector->options.push_back(new ConfigOptionAdapter<SwapDir>(*const_cast<SwapDir *>(this), &SwapDir::parseTimeOption, &SwapDir::dumpTimeOption));
+    return vector;
+}
+
+/// parses time-specific options; mimics ::SwapDir::optionObjectSizeParse()
+bool
+Rock::SwapDir::parseTimeOption(char const *option, const char *value, int reconfiguring)
+{
+    // TODO: ::SwapDir or, better, Config should provide time-parsing routines,
+    // including time unit handling. Same for size.
+
+    time_msec_t *storedTime;
+    if (strcmp(option, "swap-timeout") == 0)
+        storedTime = &fileConfig.ioTimeout;
+    else
+        return false;
+
+    if (!value)
+        self_destruct();
+
+    const time_msec_t newTime = strtoll(value, NULL, 10);
+
+    if (newTime < 0) {
+        debugs(3, DBG_CRITICAL, "FATAL: cache_dir " << path << ' ' << option << " must not be negative but is: " << newTime);
+        self_destruct();
+    }
+
+    if (reconfiguring && *storedTime != newTime)
+        debugs(3, DBG_IMPORTANT, "cache_dir " << path << ' ' << option << " is now " << newTime);
+
+    *storedTime = newTime;
+
+    return true;
+}
+
+/// reports time-specific options; mimics ::SwapDir::optionObjectSizeDump()
+void
+Rock::SwapDir::dumpTimeOption(StoreEntry * e) const
+{
+    if (fileConfig.ioTimeout)
+        storeAppendPrintf(e, " swap-timeout=%"PRId64,
+                          static_cast<int64_t>(fileConfig.ioTimeout));
+}
+
 /// check the results of the configuration; only level-0 debugging works here
 void
 Rock::SwapDir::validateOptions()

diff --git a/src/fs/rock/RockSwapDir.h b/src/fs/rock/RockSwapDir.h
index 30a33d5e0204f0a3ac6387bffcb2d05e97279151..00659d5b0c04cf49ab88a888c4348d13c5cc2186 100644 (file)
--- a/src/fs/rock/RockSwapDir.h
+++ b/src/fs/rock/RockSwapDir.h
@@ -2,12 +2,12 @@
 #define SQUID_FS_ROCK_SWAP_DIR_H
 
 #include "SwapDir.h"
+#include "DiskIO/DiskFile.h"
 #include "DiskIO/IORequestor.h"
 #include "fs/rock/RockFile.h"
 #include "ipc/StoreMap.h"
 
 class DiskIOStrategy;
-class DiskFile;
 class ReadRequest;
 class WriteRequest;
 
@@ -43,6 +43,7 @@ protected:
     virtual bool needsDiskStrand() const;
     virtual void create();
     virtual void init();
+    virtual ConfigOption *getOptionTree() 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,6 +63,8 @@ protected:
     virtual void parse(int index, char *path);
     void parseSize(); ///< 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;
 
     void rebuild(); ///< starts loading and validating stored entry metadata
     ///< used to add entries successfully loaded during rebuild
@@ -83,6 +86,9 @@ private:
     RefCount<DiskFile> theFile; ///< cache storage for this cache_dir
     DirMap *map;
 
+    /* configurable options */
+    DiskFile::Config fileConfig; ///< file-level configuration options
+
     static const int64_t HeaderSize; ///< on-disk db header size
 };
 

diff --git a/src/structs.h b/src/structs.h
index 7aada04b954b8c8c9f6d9336e530101782d203c1..359b44d7ce5cff29d9377f13ee5777350081945e 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -215,7 +215,6 @@ struct SquidConfig {
         int icp_query_max;     /* msec */
         int icp_query_min;     /* msec */
         int mcast_icp_query;   /* msec */
-        time_msec_t disk_io;
 
 #if !USE_DNSSERVERS