From e0304eb96570e60cae02339a8e5174550a86ba02 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Tue, 22 Dec 2015 17:08:34 -0600 Subject: [PATCH 1/5] Fix bug in cache scanning; simplify deletion loop. --- src/XrdFileCache/XrdFileCacheFactory.cc | 36 ++++++++++++------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/XrdFileCache/XrdFileCacheFactory.cc b/src/XrdFileCache/XrdFileCacheFactory.cc index 27501beee50..e6ecea8f858 100644 --- a/src/XrdFileCache/XrdFileCacheFactory.cc +++ b/src/XrdFileCache/XrdFileCacheFactory.cc @@ -431,6 +431,7 @@ class FPurgeState { std::pair ret = fmap.equal_range(nt); for (map_i it2 = ret.first; it2 != ret.second; ++it2) nBlckAccum -= it2->second.nBlck; + fmap.erase(ret.first, ret.second); } } } @@ -539,27 +540,24 @@ void Factory::CacheDirCleanup() // loop over map and remove files with highest value of access time for (FPurgeState::map_i it = purgeState.fmap.begin(); it != purgeState.fmap.end(); ++it) { - std::pair ret = purgeState.fmap.equal_range(it->first); - for (FPurgeState::map_i it2 = ret.first; it2 != ret.second; ++it2) + std::string path = it->second.path; + // remove info file + if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) { - std::string path = it2->second.path; - // remove info file - if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) - { - bytesToRemove -= fstat.st_size; - oss->Unlink(path.c_str()); - clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); - } - - // remove data file - path = path.substr(0, path.size() - strlen(XrdFileCache::Info::m_infoExtension)); - if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) - { - bytesToRemove -= fstat.st_size; - oss->Unlink(path.c_str()); - clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); - } + bytesToRemove -= fstat.st_size; + oss->Unlink(path.c_str()); + clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); } + + // remove data file + path = path.substr(0, path.size() - strlen(XrdFileCache::Info::m_infoExtension)); + if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) + { + bytesToRemove -= fstat.st_size; + oss->Unlink(path.c_str()); + clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); + } + if (bytesToRemove <= 0) break; } From 0e1b8366cd845014bc4bdbda1a972e30f0edfa03 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Wed, 6 Jan 2016 14:48:24 -0800 Subject: [PATCH 2/5] Use bytes to calculate how many files to purge, not blocks; subtract actual size of the file, not the length of it returned by stat. --- src/XrdFileCache/XrdFileCacheFactory.cc | 65 +++++++++++++++---------- src/XrdFileCache/XrdFileCacheInfo.hh | 10 ++++ 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/XrdFileCache/XrdFileCacheFactory.cc b/src/XrdFileCache/XrdFileCacheFactory.cc index e6ecea8f858..48583625d68 100644 --- a/src/XrdFileCache/XrdFileCacheFactory.cc +++ b/src/XrdFileCache/XrdFileCacheFactory.cc @@ -403,42 +403,46 @@ bool Factory::ConfigParameters(std::string part, XrdOucStream& config ) //______________________________________________________________________________ //namespace { -class FPurgeState { +class FPurgeState +{ public: - struct FS { + struct FS + { std::string path; - int nBlck; + long long nByte; - FS(const char* p, int n) : path(p), nBlck(n) {} + FS(const char* p, int n) : path(p), nByte(n) {} }; typedef std::multimap map_t; typedef map_t::iterator map_i; - FPurgeState(long long iNBlckReq) : nBlckReq(iNBlckReq), nBlckAccum(0) {} + FPurgeState(long long iNByteReq) : nByteReq(iNByteReq), nByteAccum(0) {} map_t fmap; - void checkFile (time_t iTime, const char* iPath, int iNBlck) + void checkFile (time_t iTime, const char* iPath, int iNByte) { - if ( (nBlckAccum < nBlckReq ) || (iTime < fmap.rbegin()->first) ) { - fmap.insert(std::pair (iTime, FS(iPath, iNBlck))); - nBlckAccum += iNBlck; + if (nByteAccum < nByteReq || iTime < fmap.rbegin()->first) + { + fmap.insert(std::pair (iTime, FS(iPath, iNByte))); + nByteAccum += iNByte; // remove newest files from map if necessary - while (nBlckAccum > nBlckReq) { + while (nByteAccum > nByteReq) + { time_t nt = fmap.begin()->first; std::pair ret = fmap.equal_range(nt); for (map_i it2 = ret.first; it2 != ret.second; ++it2) - nBlckAccum -= it2->second.nBlck; + nByteAccum -= it2->second.nByte; fmap.erase(ret.first, ret.second); } } } - private: - long long nBlckReq; - long long nBlckAccum; +private: + long long nByteReq; + long long nByteAccum; }; @@ -453,12 +457,12 @@ void FillFileMapRecurse( XrdOssDF* iOssDF, const std::string& path, FPurgeState& XrdCl::Log *log = XrdCl::DefaultEnv::GetLog(); Factory& factory = Factory::GetInstance(); - while ( (rdr = iOssDF->Readdir(&buff[0], 256)) >= 0) + while ((rdr = iOssDF->Readdir(&buff[0], 256)) >= 0) { // printf("readdir [%s]\n", buff); std::string np = path + "/" + std::string(buff); size_t fname_len = strlen(&buff[0]); - if (fname_len == 0 ) + if (fname_len == 0) { // std::cout << "Finish read dir.[" << np <<"] Break loop \n"; break; @@ -471,21 +475,23 @@ void FillFileMapRecurse( XrdOssDF* iOssDF, const std::string& path, FPurgeState& if (fname_len > InfoExtLen && strncmp(&buff[fname_len - InfoExtLen ], XrdFileCache::Info::m_infoExtension, InfoExtLen) == 0) { - fh->Open((np).c_str(),O_RDONLY, 0600, env); + // XXXX MT - shouldn't we also check if it is currently opened? + + fh->Open(np.c_str(), O_RDONLY, 0600, env); Info cinfo(factory.RefConfiguration().m_bufferSize); time_t accessTime; cinfo.Read(fh); if (cinfo.GetLatestDetachTime(accessTime, fh)) { log->Debug(XrdCl::AppMsg, "FillFileMapRecurse() checking %s accessTime %d ", buff, (int)accessTime); - purgeState.checkFile(accessTime, np.c_str(), cinfo.GetNDownloadedBlocks()); + purgeState.checkFile(accessTime, np.c_str(), cinfo.GetNDownloadedBytes()); } else { log->Warning(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s \n", np.c_str()); } } - else if ( dh->Opendir(np.c_str(), env) >= 0 ) + else if (dh->Opendir(np.c_str(), env) >= 0) { FillFileMapRecurse(dh, np, purgeState); } @@ -511,7 +517,7 @@ void Factory::CacheDirCleanup() { // get amount of space to erase long long bytesToRemove = 0; - if( oss->StatVS(&sP, "public", 1) < 0 ) + if (oss->StatVS(&sP, "public", 1) < 0) { clLog()->Error(XrdCl::AppMsg, "Factory::CacheDirCleanup() can't get statvs for dir [%s] \n", m_configuration.m_cache_dir.c_str()); exit(1); @@ -519,7 +525,7 @@ void Factory::CacheDirCleanup() else { long long ausage = sP.Total - sP.Free; - clLog()->Debug(XrdCl::AppMsg, "Factory::CacheDirCleanup() occupates disk space == %lld", ausage); + clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() occupates disk space == %lld", ausage); if (ausage > m_configuration.m_diskUsageHWM) { bytesToRemove = ausage - m_configuration.m_diskUsageLWM; @@ -529,33 +535,37 @@ void Factory::CacheDirCleanup() if (bytesToRemove > 0) { - // make a sorted map of file patch by access time + // make a sorted map of file patch by access time XrdOssDF* dh = oss->newDir(m_configuration.m_username.c_str()); if (dh->Opendir(m_configuration.m_cache_dir.c_str(), env) >= 0) { - long long nReq = (long long) ((bytesToRemove*1.4)/m_configuration.m_bufferSize); // check more that required - FPurgeState purgeState(nReq); + FPurgeState purgeState(bytesToRemove * 5 / 4); // prepare 20% more volume than required + FillFileMapRecurse(dh, m_configuration.m_cache_dir, purgeState); // loop over map and remove files with highest value of access time for (FPurgeState::map_i it = purgeState.fmap.begin(); it != purgeState.fmap.end(); ++it) { + // XXXX MT - shouldn't we re-check if the file is currently opened? + std::string path = it->second.path; // remove info file if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) { bytesToRemove -= fstat.st_size; oss->Unlink(path.c_str()); - clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); + clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld", + path.c_str(), fstat.st_size); } // remove data file path = path.substr(0, path.size() - strlen(XrdFileCache::Info::m_infoExtension)); if (oss->Stat(path.c_str(), &fstat) == XrdOssOK) { - bytesToRemove -= fstat.st_size; + bytesToRemove -= it->second.nByte; oss->Unlink(path.c_str()); - clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s size %lld ", path.c_str(), fstat.st_size); + clLog()->Info(XrdCl::AppMsg, "Factory::CacheDirCleanup() removed %s bytes %lld, stat_size %lld", + path.c_str(), it->second.nByte, fstat.st_size); } if (bytesToRemove <= 0) @@ -565,6 +575,7 @@ void Factory::CacheDirCleanup() dh->Close(); delete dh; dh =0; } + sleep(sleept); } } diff --git a/src/XrdFileCache/XrdFileCacheInfo.hh b/src/XrdFileCache/XrdFileCacheInfo.hh index a65bd4eab1c..13f81f4e8de 100644 --- a/src/XrdFileCache/XrdFileCacheInfo.hh +++ b/src/XrdFileCache/XrdFileCacheInfo.hh @@ -150,6 +150,11 @@ namespace XrdFileCache //--------------------------------------------------------------------- int GetNDownloadedBlocks() const; + //--------------------------------------------------------------------- + //! Get number of downloaded bytes + //--------------------------------------------------------------------- + long long GetNDownloadedBytes() const; + //--------------------------------------------------------------------- //! Update complete status //--------------------------------------------------------------------- @@ -204,6 +209,11 @@ namespace XrdFileCache return cntd; } + inline long long Info::GetNDownloadedBytes() const + { + return m_bufferSize * GetNDownloadedBlocks(); + } + inline int Info::GetSizeInBytes() const { return ((m_sizeInBits -1)/8 + 1); From cafe7b8c73f406be5f6bf85f89e624f151295363 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Wed, 13 Jan 2016 16:04:54 -0800 Subject: [PATCH 3/5] In cache purge, use stat.mtime of cinfo file if last access time can not be determined from contents of cinfo file. --- src/XrdFileCache/XrdFileCacheFactory.cc | 28 +++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/XrdFileCache/XrdFileCacheFactory.cc b/src/XrdFileCache/XrdFileCacheFactory.cc index 48583625d68..76b7f64a78a 100644 --- a/src/XrdFileCache/XrdFileCacheFactory.cc +++ b/src/XrdFileCache/XrdFileCacheFactory.cc @@ -488,7 +488,31 @@ void FillFileMapRecurse( XrdOssDF* iOssDF, const std::string& path, FPurgeState& } else { - log->Warning(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s \n", np.c_str()); + // XXXX MT - get a lot of those ... use time from stat? + // Or just remove them? + + log->Info(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s, trying stat.\n", np.c_str()); + + XrdOss* oss = Factory::GetInstance().GetOss(); + struct stat fstat; + + if (oss->Stat(np.c_str(), &fstat) == XrdOssOK) + { + accessTime = fstat.st_mtim.tv_sec; + log->Info(XrdCl::AppMsg, "FillFileMapRecurse() determined access time for %s via stat: %lld\n", + np.c_str(), accessTime); + + purgeState.checkFile(accessTime, np.c_str(), cinfo.GetNDownloadedBytes()); + } + else + { + log->Warning(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s. Purging directly.\n", + np.c_str()); + + oss->Unlink(np.c_str()); + np = np.substr(0, np.size() - strlen(XrdFileCache::Info::m_infoExtension)); + oss->Unlink(np.c_str()); + } } } else if (dh->Opendir(np.c_str(), env) >= 0) @@ -510,7 +534,7 @@ void Factory::CacheDirCleanup() struct stat fstat; XrdOucEnv env; - XrdOss* oss = Factory::GetInstance().GetOss(); + XrdOss* oss = Factory::GetInstance().GetOss(); XrdOssVSInfo sP; while (1) From 1b373b9ead7b47da5b92163537f51876dade88e4 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Wed, 13 Jan 2016 16:19:29 -0800 Subject: [PATCH 4/5] Fix argument type from int to long long (was n_blocks, is size_in_bytes now). --- src/XrdFileCache/XrdFileCacheFactory.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/XrdFileCache/XrdFileCacheFactory.cc b/src/XrdFileCache/XrdFileCacheFactory.cc index 76b7f64a78a..b6f772d354a 100644 --- a/src/XrdFileCache/XrdFileCacheFactory.cc +++ b/src/XrdFileCache/XrdFileCacheFactory.cc @@ -421,7 +421,7 @@ class FPurgeState map_t fmap; - void checkFile (time_t iTime, const char* iPath, int iNByte) + void checkFile (time_t iTime, const char* iPath, long long iNByte) { if (nByteAccum < nByteReq || iTime < fmap.rbegin()->first) { From d9369e8a6bbb7704c33a0b0bad78bb33316c2188 Mon Sep 17 00:00:00 2001 From: Matevz Tadel Date: Wed, 13 Jan 2016 16:52:59 -0800 Subject: [PATCH 5/5] One more int->long long. Update comments. --- src/XrdFileCache/XrdFileCacheFactory.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/XrdFileCache/XrdFileCacheFactory.cc b/src/XrdFileCache/XrdFileCacheFactory.cc index b6f772d354a..8d41ba46d18 100644 --- a/src/XrdFileCache/XrdFileCacheFactory.cc +++ b/src/XrdFileCache/XrdFileCacheFactory.cc @@ -411,7 +411,7 @@ class FPurgeState std::string path; long long nByte; - FS(const char* p, int n) : path(p), nByte(n) {} + FS(const char* p, long long n) : path(p), nByte(n) {} }; typedef std::multimap map_t; @@ -488,8 +488,7 @@ void FillFileMapRecurse( XrdOssDF* iOssDF, const std::string& path, FPurgeState& } else { - // XXXX MT - get a lot of those ... use time from stat? - // Or just remove them? + // cinfo file does not contain any known accesses, use stat.mtime instead. log->Info(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s, trying stat.\n", np.c_str()); @@ -506,6 +505,8 @@ void FillFileMapRecurse( XrdOssDF* iOssDF, const std::string& path, FPurgeState& } else { + // This really shouldn't happen ... but if it does remove cinfo and the data file right away. + log->Warning(XrdCl::AppMsg, "FillFileMapRecurse() could not get access time for %s. Purging directly.\n", np.c_str());