From ef38c1620f06f6e8dbbddb47b9d780621354af5e Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 2 Feb 2016 02:17:44 +0000 Subject: [PATCH] refs #581 Refactor data cache revision file logic into new class CDataCacheRevision. --- src/blackmisc/datacache.cpp | 152 ++++++++++++++++++++++++++---------- src/blackmisc/datacache.h | 55 ++++++++++++- 2 files changed, 163 insertions(+), 44 deletions(-) diff --git a/src/blackmisc/datacache.cpp b/src/blackmisc/datacache.cpp index c50f6d7f5..52feb33ce 100644 --- a/src/blackmisc/datacache.cpp +++ b/src/blackmisc/datacache.cpp @@ -11,10 +11,33 @@ #include "blackmisc/logmessage.h" #include "blackmisc/identifier.h" #include -#include namespace BlackMisc { + class CDataCacheRevision::LockGuard + { + public: + LockGuard(const LockGuard &) = delete; + LockGuard &operator =(const LockGuard &) = delete; + LockGuard(LockGuard &&other) : m_movedFrom(true) { *this = std::move(other); } + LockGuard &operator =(LockGuard &&other) { std::swap(m_movedFrom, other.m_movedFrom); std::swap(m_rev, other.m_rev); return *this; } + + ~LockGuard() + { + if (! m_movedFrom) { m_rev->finishUpdate(); } + } + + operator bool() const { return ! m_movedFrom; } + + private: + LockGuard() : m_movedFrom(true) {} + LockGuard(CDataCacheRevision *rev) : m_movedFrom(! rev), m_rev(rev) {} + friend class CDataCacheRevision; + + bool m_movedFrom = false; + CDataCacheRevision *m_rev = nullptr; + }; + CDataCache::CDataCache() : CValueCache(CValueCache::Distributed) { @@ -109,25 +132,12 @@ namespace BlackMisc void CDataCacheSerializer::saveToStore(const BlackMisc::CVariantMap &values, const BlackMisc::CVariantMap &baseline) { - QLockFile revisionFileLock(m_revisionFileName + ".lock"); - if (! revisionFileLock.lock()) - { - CLogMessage(this).error("Failed to lock %1: %2") << m_revisionFileName << lockFileError(revisionFileLock); - return; - } - - loadFromStore(baseline, false, true); // last-minute check for remote changes before clobbering the revision file + m_cache->m_revision.notifyPendingWrite(); + auto lock = loadFromStore(baseline, true); // last-minute check for remote changes before clobbering the revision file for (const auto &key : values.keys()) { m_deferredChanges.remove(key); } // ignore changes that we are about to overwrite - QFile revisionFile(m_revisionFileName); - if (! revisionFile.open(QFile::WriteOnly)) - { - CLogMessage(this).error("Failed to open %1: %2") << m_revisionFileName << revisionFile.errorString(); - return; - } - m_revision = CIdentifier().toUuid(); - revisionFile.write(m_revision.toByteArray()); - + if (! lock) { return; } + m_cache->m_revision.writeNewRevision(); m_cache->saveToFiles(persistentStore(), values); if (! m_deferredChanges.isEmpty()) // apply changes which we grabbed at the last minute above @@ -138,30 +148,11 @@ namespace BlackMisc } } - void CDataCacheSerializer::loadFromStore(const BlackMisc::CVariantMap &baseline, bool revLock, bool defer) + CDataCacheRevision::LockGuard CDataCacheSerializer::loadFromStore(const BlackMisc::CVariantMap &baseline, bool defer) { - QLockFile revisionFileLock(m_revisionFileName + ".lock"); - if (revLock && ! revisionFileLock.lock()) + auto lock = m_cache->m_revision.beginUpdate(); + if (lock && m_cache->m_revision.isPendingRead()) { - CLogMessage(this).error("Failed to lock %1: %2") << m_revisionFileName << lockFileError(revisionFileLock); - return; - } - - QFile revisionFile(m_revisionFileName); - if (! revisionFile.exists()) - { - return; - } - if (! revisionFile.open(QFile::ReadOnly)) - { - CLogMessage(this).error("Failed to open %1: %2") << m_revisionFileName << revisionFile.errorString(); - return; - } - - QUuid newRevision(revisionFile.readAll()); - if (m_revision != newRevision) - { - m_revision = newRevision; CValueCachePacket newValues; m_cache->loadFromFiles(persistentStore(), baseline, newValues); m_deferredChanges.insert(newValues); @@ -173,5 +164,86 @@ namespace BlackMisc emit valuesLoadedFromStore(m_deferredChanges, CIdentifier::anonymous()); m_deferredChanges.clear(); } + return lock; + } + + CDataCacheRevision::LockGuard CDataCacheRevision::beginUpdate() + { + QMutexLocker lock(&m_mutex); + + Q_ASSERT(! m_updateInProgress); + Q_ASSERT(! m_lockFile.isLocked()); + + if (! m_lockFile.lock()) + { + CLogMessage(this).error("Failed to lock %1: %2") << m_basename << lockFileError(m_lockFile); + return {}; + } + m_updateInProgress = true; + LockGuard guard(this); + + QFile revisionFile(m_basename + "/.rev"); + if (revisionFile.exists()) + { + if (! revisionFile.open(QFile::ReadOnly)) + { + CLogMessage(this).error("Failed to open %1: %2") << revisionFile.fileName() << revisionFile.errorString(); + return {}; + } + + QUuid uuid(revisionFile.readAll()); + if (uuid == m_uuid) + { + if (m_pendingWrite) { return guard; } + return {}; + } + } + + m_pendingRead = true; + return guard; + } + + void CDataCacheRevision::writeNewRevision() + { + QMutexLocker lock(&m_mutex); + + Q_ASSERT(m_updateInProgress); + Q_ASSERT(m_lockFile.isLocked()); + + QFile revisionFile(m_basename + "/.rev"); + if (! revisionFile.open(QFile::WriteOnly)) + { + CLogMessage(this).error("Failed to open %1: %2") << revisionFile.fileName() << revisionFile.errorString(); + return; + } + + m_uuid = CIdentifier().toUuid(); + revisionFile.write(m_uuid.toByteArray()); + } + + void CDataCacheRevision::finishUpdate() + { + QMutexLocker lock(&m_mutex); + + Q_ASSERT(m_updateInProgress); + Q_ASSERT(m_lockFile.isLocked()); + + m_updateInProgress = false; + m_pendingRead = false; + m_pendingWrite = false; + m_lockFile.unlock(); + } + + bool CDataCacheRevision::isPendingRead() const + { + QMutexLocker lock(&m_mutex); + + Q_ASSERT(m_updateInProgress); + return ! m_timestamps.isEmpty(); + } + + void CDataCacheRevision::notifyPendingWrite() + { + m_pendingWrite = true; } } diff --git a/src/blackmisc/datacache.h b/src/blackmisc/datacache.h index 1af761b04..44f844e79 100644 --- a/src/blackmisc/datacache.h +++ b/src/blackmisc/datacache.h @@ -17,11 +17,57 @@ #include "blackmisc/worker.h" #include #include +#include namespace BlackMisc { class CDataCache; + /*! + * Encapsulates metastate about how the version of the cache in memory compares to the one on disk. + * \threadsafe + */ + class BLACKMISC_EXPORT CDataCacheRevision + { + public: + //! Construct the single instance of the revision metastate. + CDataCacheRevision(const QString &basename) : m_basename(basename) {} + + //! Non-copyable. + //! @{ + CDataCacheRevision(const CDataCacheRevision &) = delete; + CDataCacheRevision &operator =(const CDataCacheRevision &) = delete; + //! @} + + //! RAII class to keep the revision file locked during update. + class LockGuard; + + //! Get the state of the disk cache, and prepare to update values. + //! Return value can be converted to bool, false means update is not started (error, or already up-to-date). + LockGuard beginUpdate(); + + //! During update, writes a new revision file. + void writeNewRevision(); + + //! Release the revision file lock and mark everything up-to-date (called by LockGuard destructor). + void finishUpdate(); + + //! True if beginUpdate found some values with timestamps newer than in memory. + bool isPendingRead() const; + + //! Call before beginUpdate if there is a write pending, so update will start even if there is nothing to read. + void notifyPendingWrite(); + + private: + mutable QMutex m_mutex { QMutex::Recursive }; + bool m_updateInProgress = false; + bool m_pendingRead = false; + bool m_pendingWrite = false; + QString m_basename; + QLockFile m_lockFile { m_basename + "/.lock" }; + QUuid m_uuid; + }; + /*! * Worker which performs (de)serialization on behalf of CDataCache, in a separate thread * so that the main thread is not blocked by (de)serialization of large objects. @@ -40,9 +86,9 @@ namespace BlackMisc //! Load values from persistent store. Called once per second. //! Also called by saveToStore, to ensure that remote changes to unrelated values are not lost. //! \param baseline A snapshot of the currently loaded values, taken when the load is queued. - //! \param lock Whether to acquire the revision file lock. Used when called by saveToStore. //! \param defer Whether to defer applying the changes. Used when called by saveToStore. - void loadFromStore(const BlackMisc::CVariantMap &baseline, bool lock = true, bool defer = false); + //! \return Usually ignored, but can be held in order to retain the revision file lock. + CDataCacheRevision::LockGuard loadFromStore(const BlackMisc::CVariantMap &baseline, bool defer = false); signals: //! Signal back to the cache when values have been loaded. @@ -51,7 +97,7 @@ namespace BlackMisc private: const QString &persistentStore() const; - const CDataCache *const m_cache = nullptr; + CDataCache *const m_cache = nullptr; QUuid m_revision; const QString m_revisionFileName; BlackMisc::CValueCachePacket m_deferredChanges; @@ -89,7 +135,8 @@ namespace BlackMisc const QString m_revisionFileName { persistentStore() + "/.rev" }; CDataCacheSerializer m_serializer { this, m_revisionFileName }; - friend class CDataCacheSerializer; // to access protected members of CValueCache + CDataCacheRevision m_revision { persistentStore() + "/" }; + friend class CDataCacheSerializer; // to access m_revision and protected members of CValueCache }; /*!