Review changes to get from bag9_r13323-to-trunk_r13383.patch to bag9_r13325-to-trunk_r13383.patch. === modified file 'src/DiskIO/IpcIo/IpcIoFile.cc' --- src/DiskIO/IpcIo/IpcIoFile.cc 2014-04-04 16:10:40 +0000 +++ src/DiskIO/IpcIo/IpcIoFile.cc 2014-04-28 16:27:32 +0000 @@ -1,67 +1,68 @@ /* * DEBUG: section 47 Store Directory Routines */ #include "squid.h" #include "base/RunnersRegistry.h" #include "base/TextException.h" #include "disk.h" #include "DiskIO/IORequestor.h" #include "DiskIO/IpcIo/IpcIoFile.h" #include "DiskIO/ReadRequest.h" #include "DiskIO/WriteRequest.h" #include "fd.h" #include "globals.h" #include "ipc/mem/Pages.h" #include "ipc/Messages.h" #include "ipc/Port.h" #include "ipc/Queue.h" #include "ipc/StrandSearch.h" #include "ipc/UdsOp.h" +#include "SBuf.h" #include "SquidConfig.h" #include "SquidTime.h" #include "StatCounters.h" #include "tools.h" #if HAVE_ERRNO_H #include #endif CBDATA_CLASS_INIT(IpcIoFile); /// shared memory segment path to use for IpcIoFile maps static const char *const ShmLabel = "io_file"; /// a single worker-to-disker or disker-to-worker queue capacity; up /// to 2*QueueCapacity I/O requests queued between a single worker and /// a single disker // TODO: make configurable or compute from squid.conf settings if possible static const int QueueCapacity = 1024; const double IpcIoFile::Timeout = 7; // seconds; XXX: ALL,9 may require more IpcIoFile::IpcIoFileList IpcIoFile::WaitingForOpen; IpcIoFile::IpcIoFilesMap IpcIoFile::IpcIoFiles; std::auto_ptr IpcIoFile::queue; bool IpcIoFile::DiskerHandleMoreRequestsScheduled = false; -static bool DiskerOpen(const String &path, int flags, mode_t mode); -static void DiskerClose(const String &path); +static bool DiskerOpen(const SBuf &path, int flags, mode_t mode); +static void DiskerClose(const SBuf &path); /// IpcIo wrapper for debugs() streams; XXX: find a better class name struct SipcIo { SipcIo(int aWorker, const IpcIoMsg &aMsg, int aDisker): worker(aWorker), msg(aMsg), disker(aDisker) {} int worker; const IpcIoMsg &msg; int disker; }; std::ostream & operator <<(std::ostream &os, const SipcIo &sio) { return os << "ipcIo" << sio.worker << '.' << sio.msg.requestId << (sio.msg.command == IpcIo::cmdRead ? 'r' : 'w') << sio.disker; } IpcIoFile::IpcIoFile(char const *aDb): dbName(aDb), diskId(-1), error_(false), lastRequestId(0), @@ -81,41 +82,41 @@ } } void IpcIoFile::configure(const Config &cfg) { DiskFile::configure(cfg); config = cfg; } void IpcIoFile::open(int flags, mode_t mode, RefCount callback) { ioRequestor = callback; Must(diskId < 0); // we do not know our disker yet if (!queue.get()) queue.reset(new Queue(ShmLabel, IamWorkerProcess() ? Queue::groupA : Queue::groupB, KidIdentifier)); if (IamDiskProcess()) { - error_ = !DiskerOpen(dbName, flags, mode); + error_ = !DiskerOpen(SBuf(dbName.termedBuf()), flags, mode); if (error_) return; diskId = KidIdentifier; const bool inserted = IpcIoFiles.insert(std::make_pair(diskId, this)).second; Must(inserted); queue->localRateLimit() = static_cast(config.ioRate); Ipc::HereIamMessage ann(Ipc::StrandCoord(KidIdentifier, getpid())); ann.strand.tag = dbName; Ipc::TypedMsgHdr message; ann.pack(message); SendMessage(Ipc::Port::CoordinatorAddr(), message); ioRequestor->ioCompletedNotification(); return; } @@ -160,41 +161,41 @@ } /** * Alias for IpcIoFile::open(...) \copydoc IpcIoFile::open(int flags, mode_t mode, RefCount callback) */ void IpcIoFile::create(int flags, mode_t mode, RefCount callback) { assert(false); // check /* We use the same logic path for open */ open(flags, mode, callback); } void IpcIoFile::close() { assert(ioRequestor != NULL); if (IamDiskProcess()) - DiskerClose(dbName); + DiskerClose(SBuf(dbName.termedBuf())); // XXX: else nothing to do? ioRequestor->closeCompleted(); } bool IpcIoFile::canRead() const { return diskId >= 0 && !error_ && canWait(); } bool IpcIoFile::canWrite() const { return diskId >= 0 && !error_ && canWait(); } bool IpcIoFile::error() const { @@ -619,41 +620,41 @@ IpcIoPendingRequest::IpcIoPendingRequest(const IpcIoFile::Pointer &aFile): file(aFile), readRequest(NULL), writeRequest(NULL) { } void IpcIoPendingRequest::completeIo(IpcIoMsg *const response) { if (readRequest) file->readCompleted(readRequest, response); else if (writeRequest) file->writeCompleted(writeRequest, response); else { Must(!response); // only timeouts are handled here file->openCompleted(NULL); } } /* XXX: disker code that should probably be moved elsewhere */ -static String DbName; ///< full db file name +static SBuf DbName; ///< full db file name static int TheFile = -1; ///< db file descriptor static void diskerRead(IpcIoMsg &ipcIo) { if (!Ipc::Mem::GetPage(Ipc::Mem::PageId::ioPage, ipcIo.page)) { ipcIo.len = 0; debugs(47,2, HERE << "run out of shared memory pages for IPC I/O"); return; } char *const buf = Ipc::Mem::PagePointer(ipcIo.page); const ssize_t read = pread(TheFile, buf, min(ipcIo.len, Ipc::Mem::PageSize()), ipcIo.offset); ++statCounter.syscalls.disk.reads; fd_bytes(TheFile, read, FD_READ); if (read >= 0) { ipcIo.xerrno = 0; const size_t len = static_cast(read); // safe because read > 0 debugs(47,8, HERE << "disker" << KidIdentifier << " read " << @@ -867,69 +868,69 @@ debugs(47, 7, HERE << "pushing " << SipcIo(workerId, ipcIo, KidIdentifier)); try { if (queue->push(workerId, ipcIo)) Notify(workerId); // must notify worker } catch (const Queue::Full &) { // The worker queue should not overflow because the worker should pop() // before push()ing and because if disker pops N requests at a time, // we should make sure the worker pop() queue length is the worker // push queue length plus N+1. XXX: implement the N+1 difference. debugs(47, DBG_IMPORTANT, "BUG: Worker I/O pop queue for " << DbName << " overflow: " << SipcIo(workerId, ipcIo, KidIdentifier)); // TODO: report queue len // the I/O request we could not push will timeout } } static bool -DiskerOpen(const String &path, int flags, mode_t mode) +DiskerOpen(const SBuf &path, int flags, mode_t mode) { assert(TheFile < 0); DbName = path; - TheFile = file_open(DbName.termedBuf(), flags); + TheFile = file_open(DbName.c_str(), flags); if (TheFile < 0) { const int xerrno = errno; debugs(47, DBG_CRITICAL, "ERROR: cannot open " << DbName << ": " << xstrerr(xerrno)); return false; } ++store_open_disk_fd; - debugs(79,3, HERE << "rock db opened " << DbName << ": FD " << TheFile); + debugs(79,3, "rock db opened " << DbName << ": FD " << TheFile); return true; } static void -DiskerClose(const String &path) +DiskerClose(const SBuf &path) { if (TheFile >= 0) { file_close(TheFile); debugs(79,3, HERE << "rock db closed " << path << ": FD " << TheFile); TheFile = -1; --store_open_disk_fd; } - DbName.clean(); + DbName.clear(); } /// reports our needs for shared memory pages to Ipc::Mem::Pages /// and initializes shared memory segments used by IpcIoFile class IpcIoRr: public Ipc::Mem::RegisteredRunner { public: /* RegisteredRunner API */ IpcIoRr(): owner(NULL) {} virtual ~IpcIoRr(); virtual void claimMemoryNeeds(); protected: /* Ipc::Mem::RegisteredRunner API */ virtual void create(); private: Ipc::FewToFewBiQueue::Owner *owner; }; === modified file 'src/Store.h' --- src/Store.h 2014-03-19 04:04:52 +0000 +++ src/Store.h 2014-04-28 19:00:59 +0000 @@ -103,44 +103,44 @@ virtual bool mayStartSwapOut(); virtual void trimMemory(const bool preserveSwappable); void abort(); void unlink(); void makePublic(); void makePrivate(); void setPublicKey(); void setPrivateKey(); void expireNow(); void releaseRequest(); void negativeCache(); void cacheNegatively(); /** \todo argh, why both? */ void invokeHandlers(); void purgeMem(); void cacheInMemory(); ///< start or continue storing in memory cache void swapOut(); /// whether we are in the process of writing this entry to disk bool swappingOut() const { return swap_status == SWAPOUT_WRITING; } void swapOutFileClose(int how); const char *url() const; - /// generally cachable (checks agnostic to disk/RAM-specific requirements) - /// common part of mayStartSwapOut() and memoryCachable() - /// TODO: Make private so only those two methods can call this. - int checkCachable(); + /// Satisfies cachability requirements shared among disk and RAM caches. + /// Encapsulates common checks of mayStartSwapOut() and memoryCachable(). + /// TODO: Rename and make private so only those two methods can call this. + bool checkCachable(); int checkNegativeHit() const; int locked() const; int validToSend() const; bool memoryCachable(); ///< checkCachable() and can be cached in memory /// if needed, initialize mem_obj member w/o URI-related information MemObject *makeMemObject(); /// initialize mem_obj member (if needed) and supply URI-related info void createMemObject(const char *storeId, const char *logUri, const HttpRequestMethod &aMethod); void dump(int debug_lvl) const; void hashDelete(); void hashInsert(const cache_key *); void registerAbort(STABH * cb, void *); void reset(); void setMemStatus(mem_status_t); void timestampsSet(); void unregisterAbort(); void destroyMemObject(); === modified file 'src/ipc/StoreMap.cc' --- src/ipc/StoreMap.cc 2014-04-21 18:09:06 +0000 +++ src/ipc/StoreMap.cc 2014-04-28 15:53:24 +0000 @@ -57,41 +57,41 @@ return diff < 0 ? -1 : +1; return 0; } void Ipc::StoreMap::forgetWritingEntry(sfileno fileno) { Anchor &inode = anchorAt(fileno); assert(inode.writing()); // we do not iterate slices because we were told to forget about // them; the caller is responsible for freeing them (most likely // our slice list is incomplete or has holes) inode.waitingToBeFreed = false; inode.rewind(); inode.lock.unlockExclusive(); - anchors->count; + --anchors->count; debugs(54, 8, "closed entry " << fileno << " for writing " << path); } Ipc::StoreMap::Anchor * Ipc::StoreMap::openForWriting(const cache_key *const key, sfileno &fileno) { debugs(54, 5, "opening entry with key " << storeKeyText(key) << " for writing " << path); const int idx = anchorIndexByKey(key); if (Anchor *anchor = openForWritingAt(idx)) { fileno = idx; return anchor; } return NULL; } Ipc::StoreMap::Anchor * === modified file 'src/store.cc' --- src/store.cc 2014-04-19 23:05:51 +0000 +++ src/store.cc 2014-04-28 18:52:14 +0000 @@ -933,41 +933,41 @@ return 0; } int StoreEntry::checkTooSmall() { if (EBIT_TEST(flags, ENTRY_SPECIAL)) return 0; if (STORE_OK == store_status) if (mem_obj->object_sz < 0 || mem_obj->object_sz < Config.Store.minObjectSize) return 1; if (getReply()->content_length > -1) if (getReply()->content_length < Config.Store.minObjectSize) return 1; return 0; } // TODO: move "too many open..." checks outside -- we are called too early/late -int +bool StoreEntry::checkCachable() { // XXX: This method is used for both memory and disk caches, but some // checks are specific to disk caches. Move them to mayStartSwapOut(). // XXX: This method may be called several times, sometimes with different // outcomes, making store_check_cachable_hist counters misleading. // check this first to optimize handling of repeated calls for uncachables if (EBIT_TEST(flags, RELEASE_REQUEST)) { debugs(20, 2, "StoreEntry::checkCachable: NO: not cachable"); ++store_check_cachable_hist.no.not_entry_cachable; // TODO: rename? return 0; // avoid rerequesting release below } #if CACHE_ALL_METHODS if (mem_obj->method != Http::METHOD_GET) { debugs(20, 2, "StoreEntry::checkCachable: NO: non-GET method"); ++store_check_cachable_hist.no.non_get; === modified file 'src/tests/stub_store.cc' --- src/tests/stub_store.cc 2014-03-19 04:04:52 +0000 +++ src/tests/stub_store.cc 2014-04-28 19:02:20 +0000 @@ -29,41 +29,41 @@ void StoreEntry::complete() STUB store_client_t StoreEntry::storeClientType() const STUB_RETVAL(STORE_NON_CLIENT) char const *StoreEntry::getSerialisedMetaData() STUB_RETVAL(NULL) void StoreEntry::replaceHttpReply(HttpReply *, bool andStartWriting) STUB bool StoreEntry::mayStartSwapOut() STUB_RETVAL(false) void StoreEntry::trimMemory(const bool preserveSwappable) STUB void StoreEntry::abort() STUB void StoreEntry::unlink() STUB void StoreEntry::makePublic() STUB void StoreEntry::makePrivate() STUB void StoreEntry::setPublicKey() STUB void StoreEntry::setPrivateKey() STUB void StoreEntry::expireNow() STUB void StoreEntry::releaseRequest() STUB void StoreEntry::negativeCache() STUB void StoreEntry::cacheNegatively() STUB void StoreEntry::purgeMem() STUB void StoreEntry::swapOut() STUB void StoreEntry::swapOutFileClose(int how) STUB const char *StoreEntry::url() const STUB_RETVAL(NULL) -int StoreEntry::checkCachable() STUB_RETVAL(0) +bool StoreEntry::checkCachable() STUB_RETVAL(0) int StoreEntry::checkNegativeHit() const STUB_RETVAL(0) int StoreEntry::locked() const STUB_RETVAL(0) int StoreEntry::validToSend() const STUB_RETVAL(0) bool StoreEntry::memoryCachable() STUB_RETVAL(false) MemObject *StoreEntry::makeMemObject() STUB_RETVAL(NULL) void StoreEntry::createMemObject(const char *, const char *, const HttpRequestMethod &aMethod) STUB void StoreEntry::dump(int debug_lvl) const STUB void StoreEntry::hashDelete() STUB void StoreEntry::hashInsert(const cache_key *) STUB void StoreEntry::registerAbort(STABH * cb, void *) STUB void StoreEntry::reset() STUB void StoreEntry::setMemStatus(mem_status_t) STUB void StoreEntry::timestampsSet() STUB void StoreEntry::unregisterAbort() STUB void StoreEntry::destroyMemObject() STUB int StoreEntry::checkTooSmall() STUB_RETVAL(0) void StoreEntry::delayAwareRead(const Comm::ConnectionPointer&, char *buf, int len, AsyncCall::Pointer callback) STUB void StoreEntry::setNoDelay (bool const) STUB bool StoreEntry::modifiedSince(HttpRequest * request) const STUB_RETVAL(false) bool StoreEntry::hasIfMatchEtag(const HttpRequest &request) const STUB_RETVAL(false)