From 51c0fbbd487bd078399be0114ac7a76be419c984 Mon Sep 17 00:00:00 2001 From: Alja Mrak-Tadel Date: Tue, 4 Apr 2017 10:27:57 -0700 Subject: [PATCH 1/3] Log detach in debug mode. --- src/XrdFileCache/XrdFileCacheIOFileBlock.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc index 64d9a3f63c3..1a7eb658cb8 100644 --- a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc +++ b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc @@ -49,6 +49,9 @@ IOFileBlock::IOFileBlock(XrdOucCacheIO2 *io, XrdOucCacheStats &statsGlobal, Cach XrdOucCacheIO* IOFileBlock::Detach() { // this is called when this IO is no longer active + + TRACEIO(Debug, "IOFileBlock detaching file"); + XrdOucCacheIO * io = GetInput(); while (! m_blocks.empty()) From 1616580ecee0ae44bb4c0222a880bce65b7acccd Mon Sep 17 00:00:00 2001 From: Alja Mrak-Tadel Date: Thu, 6 Apr 2017 14:52:23 -0700 Subject: [PATCH 2/3] Keep IO object in XrdPosixFile::DelayDestroy list only if it is processing clinet requests. --- src/XrdFileCache/XrdFileCache.cc | 52 +++++++++++++++-- src/XrdFileCache/XrdFileCache.hh | 6 ++ src/XrdFileCache/XrdFileCacheFile.cc | 60 +++++++++++--------- src/XrdFileCache/XrdFileCacheFile.hh | 8 ++- src/XrdFileCache/XrdFileCacheIO.hh | 3 + src/XrdFileCache/XrdFileCacheIOEntireFile.cc | 50 +++++++++++++--- src/XrdFileCache/XrdFileCacheIOEntireFile.hh | 4 +- src/XrdFileCache/XrdFileCacheIOFileBlock.cc | 45 ++++++++++++--- src/XrdFileCache/XrdFileCacheIOFileBlock.hh | 4 +- 9 files changed, 180 insertions(+), 52 deletions(-) diff --git a/src/XrdFileCache/XrdFileCache.cc b/src/XrdFileCache/XrdFileCache.cc index 60228a1ba88..fe50c5c2f0e 100644 --- a/src/XrdFileCache/XrdFileCache.cc +++ b/src/XrdFileCache/XrdFileCache.cc @@ -61,6 +61,12 @@ void *PrefetchThread(void* ptr) return NULL; } +void *DyingFilesNeedSyncThread(void* ptr) +{ + Cache* cache = static_cast(ptr); + cache->DyingFilesNeedSync(); + return NULL; +} extern "C" { @@ -85,9 +91,12 @@ XrdOucCache2 *XrdOucGetCache2(XrdSysLogger *logger, pthread_t tid2; XrdSysThread::Run(&tid2, PrefetchThread, (void*)(&factory), 0, "XrdFileCache Prefetch "); - pthread_t tid; XrdSysThread::Run(&tid, CacheDirCleanupThread, NULL, 0, "XrdFileCache CacheDirCleanup"); + + pthread_t tids; + XrdSysThread::Run(&tids, DyingFilesNeedSyncThread, (void*)(&factory), 0, "XrdFileCache DyingFilesNeedSyncThread"); + return &factory; } } @@ -128,7 +137,8 @@ Cache::Cache() : XrdOucCache(), m_trace(0), m_traceID("Manager"), m_prefetch_condVar(0), - m_RAMblocks_used(0) + m_RAMblocks_used(0), + m_sync_condVar(0) { m_trace = new XrdOucTrace(&m_log); // default log level is Warning @@ -436,8 +446,6 @@ int Cache::Stat(const char *curl, struct stat &sbuff) } //______________________________________________________________________________ - - void Cache::Prefetch() { @@ -460,3 +468,39 @@ Cache::Prefetch() } } +//______________________________________________________________________________ +void +Cache::RegisterDyingFilesNeedSync(IO* io) +{ + m_sync_condVar.Lock(); + m_syncIOList.push_back(io); + m_sync_condVar.Signal(); + m_sync_condVar.UnLock(); +} + +//______________________________________________________________________________ +void +Cache::DyingFilesNeedSync() +{ + while (true) { + m_sync_condVar.Lock(); + while (m_syncIOList.empty()) + { + m_sync_condVar.Wait(); + } + + std::vector::iterator it = m_syncIOList.begin(); + while ( it != m_syncIOList.end()) + { + if ((*it)->FinalizeSyncBeforeExit() == false) + { + IO* bye = *it; + delete bye; + it = m_syncIOList.erase(it); + } + else ++it; + } + m_sync_condVar.UnLock(); + XrdSysTimer::Snooze(1); + } +} diff --git a/src/XrdFileCache/XrdFileCache.hh b/src/XrdFileCache/XrdFileCache.hh index 3daf63a6cc3..7b488f7c68b 100644 --- a/src/XrdFileCache/XrdFileCache.hh +++ b/src/XrdFileCache/XrdFileCache.hh @@ -202,6 +202,9 @@ public: XrdOucTrace* GetTrace() { return m_trace; } + void DyingFilesNeedSync(); + void RegisterDyingFilesNeedSync(IO*); + private: bool ConfigParameters(std::string, XrdOucStream&, TmpConfiguration &tmpc); bool ConfigXeq(char *, XrdOucStream &); @@ -250,6 +253,9 @@ private: // prefetching typedef std::vector PrefetchList; PrefetchList m_prefetchList; + + std::vector m_syncIOList; + XrdSysCondVar m_sync_condVar; }; } diff --git a/src/XrdFileCache/XrdFileCacheFile.cc b/src/XrdFileCache/XrdFileCacheFile.cc index 066385c5131..e24d9b4cb09 100644 --- a/src/XrdFileCache/XrdFileCacheFile.cc +++ b/src/XrdFileCache/XrdFileCacheFile.cc @@ -174,47 +174,51 @@ bool File::ioActive() blockMapEmpty = m_block_map.empty(); } + + + return !blockMapEmpty; +} + +bool File::FinalizeSyncBeforeExit() +{ + // returns true if sync is required + // this method is called after corresponding IO is detached from PosixCache - if (blockMapEmpty) + bool schedule_sync = false; { - // file is not active when block map is empty and sync is done - bool schedule_sync = false; - - { - XrdSysCondVarHelper _lck(m_downloadCond); + XrdSysCondVarHelper _lck(m_downloadCond); - if (m_in_sync) return true; + if (m_in_sync) return true; - if (m_writes_during_sync.empty() && m_non_flushed_cnt == 0) - { - if (! m_detachTimeIsLogged) - { - m_cfi.WriteIOStatDetach(m_stats); - m_detachTimeIsLogged = true; - schedule_sync = true; - } - } - else + if (m_writes_during_sync.empty() && m_non_flushed_cnt == 0) + { + if (! m_detachTimeIsLogged) { - // write leftovers + m_cfi.WriteIOStatDetach(m_stats); + m_detachTimeIsLogged = true; schedule_sync = true; } - - if (schedule_sync) - m_in_sync = true; - } - - if (schedule_sync) - { - XrdPosixGlobals::schedP->Schedule(m_syncer); } else { - return false; + // write leftovers + schedule_sync = true; } + + if (schedule_sync) + m_in_sync = true; + } + + if (schedule_sync) + { + XrdPosixGlobals::schedP->Schedule(m_syncer); + return true; + } + else + { + return false; } - return true; } //------------------------------------------------------------------------------ diff --git a/src/XrdFileCache/XrdFileCacheFile.hh b/src/XrdFileCache/XrdFileCacheFile.hh index 1a2058a8952..e79ee802755 100644 --- a/src/XrdFileCache/XrdFileCacheFile.hh +++ b/src/XrdFileCache/XrdFileCacheFile.hh @@ -155,7 +155,13 @@ public: //! Used in XrdPosixXrootd::Close() //---------------------------------------------------------------------- bool ioActive(); - + + //---------------------------------------------------------------------- + //! \brief I. Return true if any of blocks need sync. + //! Called from IO::DyingFilesNeedSync() + //---------------------------------------------------------------------- + bool FinalizeSyncBeforeExit(); + //---------------------------------------------------------------------- //! Sync file cache inf o and output data with disk //---------------------------------------------------------------------- diff --git a/src/XrdFileCache/XrdFileCacheIO.hh b/src/XrdFileCache/XrdFileCacheIO.hh index 2228df45df9..2b9a988405e 100644 --- a/src/XrdFileCache/XrdFileCacheIO.hh +++ b/src/XrdFileCache/XrdFileCacheIO.hh @@ -35,6 +35,8 @@ public: virtual void RelinquishFile(File*) = 0; + virtual bool FinalizeSyncBeforeExit() = 0; + XrdOucTrace* GetTrace() {return m_cache.GetTrace(); } XrdOucCacheIO2* GetInput(); @@ -47,6 +49,7 @@ protected: std::string m_path; const char* GetPath() { return m_path.c_str(); } + bool m_syncDiskAfterDetach; private: XrdOucCacheIO2 *m_io; //!< original data source XrdSysMutex updMutex; diff --git a/src/XrdFileCache/XrdFileCacheIOEntireFile.cc b/src/XrdFileCache/XrdFileCacheIOEntireFile.cc index cda08b38045..a5f6e4e19e8 100644 --- a/src/XrdFileCache/XrdFileCacheIOEntireFile.cc +++ b/src/XrdFileCache/XrdFileCacheIOEntireFile.cc @@ -58,12 +58,21 @@ IOEntireFile::IOEntireFile(XrdOucCacheIO2 *io, XrdOucCacheStats &stats, Cache & Cache::GetInstance().AddActive(m_file); } +//______________________________________________________________________________ IOEntireFile::~IOEntireFile() { + // called from Detach() if no sync is needed or + // from Cache's sync thread TRACEIO(Debug, "IOEntireFile::~IOEntireFile() "); + + if (m_file) { + m_cache.Detach(m_file); + m_file = 0; + } delete m_localStat; } +//______________________________________________________________________________ int IOEntireFile::Fstat(struct stat &sbuff) { XrdCl::URL url(GetPath()); @@ -81,11 +90,13 @@ int IOEntireFile::Fstat(struct stat &sbuff) return 0; } +//______________________________________________________________________________ long long IOEntireFile::FSize() { return m_file->GetFileSize(); } +//______________________________________________________________________________ void IOEntireFile::RelinquishFile(File* f) { TRACEIO(Info, "IOEntireFile::RelinquishFile"); @@ -93,6 +104,8 @@ void IOEntireFile::RelinquishFile(File* f) m_file = 0; } +//______________________________________________________________________________ + int IOEntireFile::initCachedStat(const char* path) { // Called indirectly from the constructor. @@ -141,6 +154,7 @@ int IOEntireFile::initCachedStat(const char* path) return res; } +//______________________________________________________________________________ bool IOEntireFile::ioActive() { if ( ! m_file) @@ -149,27 +163,45 @@ bool IOEntireFile::ioActive() } else { - bool active = m_file->ioActive(); - if (! active && m_file) - { - TRACEIO(Debug, "IOEntireFile::ioActive() detaching file"); - m_cache.Detach(m_file); - m_file = 0; - } - return active; + return m_file->ioActive(); } } +//______________________________________________________________________________ + XrdOucCacheIO *IOEntireFile::Detach() { + // Called from XrdPosixFile destructor + TRACEIO(Debug, "IOEntireFile::Detach() "); XrdOucCacheIO * io = GetInput(); - delete this; + if ( ! FinalizeSyncBeforeExit() ) + { + delete this; + } + else + { + m_cache.RegisterDyingFilesNeedSync(this); + } + return io; } + +//______________________________________________________________________________ + +bool IOEntireFile::FinalizeSyncBeforeExit() +{ + if (m_file) + return m_file->FinalizeSyncBeforeExit(); + else + return false; +} + +//______________________________________________________________________________ + int IOEntireFile::Read (char *buff, long long off, int size) { TRACEIO(Dump, "IOEntireFile::Read() "<< this << " off: " << off << " size: " << size ); diff --git a/src/XrdFileCache/XrdFileCacheIOEntireFile.hh b/src/XrdFileCache/XrdFileCacheIOEntireFile.hh index 8a8da613554..f4d1029379e 100644 --- a/src/XrdFileCache/XrdFileCacheIOEntireFile.hh +++ b/src/XrdFileCache/XrdFileCacheIOEntireFile.hh @@ -81,7 +81,9 @@ public: //! \brief Virtual method of XrdOucCacheIO. //! Called to check if destruction needs to be done in a separate task. virtual bool ioActive(); - + + virtual bool FinalizeSyncBeforeExit(); + virtual int Fstat(struct stat &sbuff); virtual long long FSize(); diff --git a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc index 1a7eb658cb8..a94c68bc9fd 100644 --- a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc +++ b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc @@ -45,26 +45,53 @@ IOFileBlock::IOFileBlock(XrdOucCacheIO2 *io, XrdOucCacheStats &statsGlobal, Cach if (m_infoFile) m_info.WriteIOStatAttach(); } +//______________________________________________________________________________ +IOFileBlock::~IOFileBlock() +{ + // called from Detach() if no sync is needed or + // from Cache's sync thread + while (! m_blocks.empty()) + { + std::map::iterator it = m_blocks.begin(); + m_cache.Detach(it->second); + m_blocks.erase(it); + } +} + //______________________________________________________________________________ XrdOucCacheIO* IOFileBlock::Detach() { - // this is called when this IO is no longer active + // Called from XrdPosixFile destructor TRACEIO(Debug, "IOFileBlock detaching file"); XrdOucCacheIO * io = GetInput(); - while (! m_blocks.empty()) + // call need sync on all + if ( ! FinalizeSyncBeforeExit() ) { - std::map::iterator it = m_blocks.begin(); - m_cache.Detach(it->second); - m_blocks.erase(it); + delete this; } - delete this; - + else + { + m_cache.RegisterDyingFilesNeedSync(this); + } + return io; } +//______________________________________________________________________________ +bool IOFileBlock::FinalizeSyncBeforeExit() +{ + bool syncDone = false; + for (std::map::iterator it = m_blocks.begin(); it != m_blocks.end(); ++it) + { + bool s = it->second->FinalizeSyncBeforeExit(); + syncDone = syncDone | s; + } + return syncDone; +} + //______________________________________________________________________________ void IOFileBlock::CloseInfoFile() { @@ -258,7 +285,9 @@ bool IOFileBlock::ioActive() bool active = false; for (std::map::iterator it = m_blocks.begin(); it != m_blocks.end(); ++it) { - if (it->second->ioActive()) active = true; + if (it->second->ioActive()) { + active = true; + } } return active; diff --git a/src/XrdFileCache/XrdFileCacheIOFileBlock.hh b/src/XrdFileCache/XrdFileCacheIOFileBlock.hh index 80a196e0c08..c6457f4cde5 100644 --- a/src/XrdFileCache/XrdFileCacheIOFileBlock.hh +++ b/src/XrdFileCache/XrdFileCacheIOFileBlock.hh @@ -46,7 +46,7 @@ public: //------------------------------------------------------------------------ //! Destructor. //------------------------------------------------------------------------ - ~IOFileBlock(){} + ~IOFileBlock(); //--------------------------------------------------------------------- //! Detach from Cache. Note: this will delete the object. @@ -64,6 +64,8 @@ public: //! Called to check if destruction needs to be done in a separate task. virtual bool ioActive(); + virtual bool FinalizeSyncBeforeExit(); + virtual int Fstat(struct stat &sbuff); virtual long long FSize(); From 3af362672ee89cf52cc621e2f97eec5b4003b826 Mon Sep 17 00:00:00 2001 From: Alja Mrak-Tadel Date: Thu, 6 Apr 2017 14:55:34 -0700 Subject: [PATCH 3/3] Log detach and destroy in debug level. --- src/XrdFileCache/XrdFileCacheIOFileBlock.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc index a94c68bc9fd..c954224294a 100644 --- a/src/XrdFileCache/XrdFileCacheIOFileBlock.cc +++ b/src/XrdFileCache/XrdFileCacheIOFileBlock.cc @@ -50,6 +50,9 @@ IOFileBlock::~IOFileBlock() { // called from Detach() if no sync is needed or // from Cache's sync thread + + TRACEIO(Debug, "deleting IOFileBlock"); + while (! m_blocks.empty()) { std::map::iterator it = m_blocks.begin(); @@ -63,7 +66,7 @@ XrdOucCacheIO* IOFileBlock::Detach() { // Called from XrdPosixFile destructor - TRACEIO(Debug, "IOFileBlock detaching file"); + TRACEIO(Debug, "detach IOFileBlock"); XrdOucCacheIO * io = GetInput();