From: Alex Rousskov Date: Sat, 10 Sep 2011 01:25:27 +0000 (-0600) Subject: Moved squid.conf global disk_io_timeout to cache_dir-local swap-timeout. X-Git-Tag: take08~17 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=43ebbac3285cd00b3f01ebd3b4c0d9867c98913f;p=thirdparty%2Fsquid.git 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 0b820765ba..612badc748 100644 --- 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 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 callback) = 0; virtual void create(int flags, mode_t mode, RefCount callback) = 0; virtual void read(ReadRequest *) = 0; diff --git a/src/DiskIO/IpcIo/IpcIoFile.cc b/src/DiskIO/IpcIo/IpcIoFile.cc index 3d50a70b9a..34ebdb90f3 100644 --- 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 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(expectedWait) < Config.Timeout.disk_io) + static_cast(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 4bf39c4243..3a43d59365 100644 --- 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 callback); virtual void create(int flags, mode_t mode, RefCount 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 d2adc20bdb..2b3f195aea 100644 --- 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 6277da17bc..066d7c5da9 100644 --- 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(i) << 20; // MBytes to Bytes } +ConfigOption * +Rock::SwapDir::getOptionTree() const +{ + ConfigOptionVector *vector = dynamic_cast(::SwapDir::getOptionTree()); + assert(vector); + vector->options.push_back(new ConfigOptionAdapter(*const_cast(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(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 30a33d5e02..00659d5b0c 100644 --- 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 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 7aada04b95..359b44d7ce 100644 --- 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