From 80f6459ce498a88a84c1b331b410e2ac42a2b12b Mon Sep 17 00:00:00 2001
From: Cedric Caffy <cedric.caffy@cern.ch>
Date: Mon, 9 Nov 2020 11:41:22 +0100
Subject: [PATCH] [repack] Repack now fails if the repack buffer VID directory
 cannot be created during expansion

---
 ReleaseNotes.md         | 10 +++++++++
 cta.spec.in             |  2 ++
 disk/DiskFile.cpp       |  1 +
 disk/DiskFile.hpp       |  4 ++++
 scheduler/Scheduler.cpp | 49 ++++++++++++++++++++---------------------
 scheduler/Scheduler.hpp |  2 +-
 6 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/ReleaseNotes.md b/ReleaseNotes.md
index 4935aaa16c..5bdf835179 100644
--- a/ReleaseNotes.md
+++ b/ReleaseNotes.md
@@ -1,3 +1,13 @@
+# v3.1-10
+
+## Summary
+
+### Features
+
+### Bug fixes
+
+cta/CTA#837 Repack now fails if the repack buffer VID directory cannot be created during expansion
+
 # v3.1-9
 
 ## Summary
diff --git a/cta.spec.in b/cta.spec.in
index 3ca0ab69ac..383d1de474 100644
--- a/cta.spec.in
+++ b/cta.spec.in
@@ -490,6 +490,8 @@ Currently contains a helper for the client-ar script, which should be installed
 %attr(0755,root,root) /usr/bin/cta-client-ar-abortPrepare
 
 %changelog
+* Wed Nov 18 2020 julien.leduc (at) cern.ch - 3.1-10
+- cta/CTA#837 Repack now fails if the repack buffer VID directory cannot be created during expansion
 * Wed Nov 04 2020 julien.leduc (at) cern.ch - 3.1-9
 - Upstream EOS 4.8.26-1
 - cta/CTA#907 For backpressure, the EOS free space can be fetched by calling an external script
diff --git a/disk/DiskFile.cpp b/disk/DiskFile.cpp
index 32a31aa32c..25d1810fb6 100644
--- a/disk/DiskFile.cpp
+++ b/disk/DiskFile.cpp
@@ -771,6 +771,7 @@ void XRootdDirectory::rmdir() {
 bool XRootdDirectory::exist() {
   XrdCl::StatInfo *statInfo;
   XrdCl::XRootDStatus statStatus = m_xrootFileSystem.Stat(m_truncatedDirectoryURL,statInfo,c_xrootTimeout);
+  cta::exception::XrootCl::throwOnError(statStatus,"In XRootdDirectory::exist() : failed to stat the directory at "+m_URL);
   if(statStatus.errNo == XErrorCode::kXR_NotFound){
     return false;
   }
diff --git a/disk/DiskFile.hpp b/disk/DiskFile.hpp
index 75919c4e2d..663175eec2 100644
--- a/disk/DiskFile.hpp
+++ b/disk/DiskFile.hpp
@@ -230,6 +230,10 @@ namespace cta {
      * Remove the directory located at this->m_URL 
      */
     virtual void rmdir() = 0;
+    
+    std::string getURL() {
+      return m_URL;
+    }
 
     virtual ~Directory() throw() {}
   protected:
diff --git a/scheduler/Scheduler.cpp b/scheduler/Scheduler.cpp
index 4126aa925d..fef11751a5 100644
--- a/scheduler/Scheduler.cpp
+++ b/scheduler/Scheduler.cpp
@@ -461,27 +461,20 @@ void Scheduler::expandRepackRequest(std::unique_ptr<RepackRequest>& repackReques
     //We only create the folder if there are some files to Repack
     cta::disk::DirectoryFactory dirFactory;
     dir.reset(dirFactory.createDirectory(dirBufferURL.str()));
-    if(dir->exist()){
-      //Repack tape repair workflow
-      try{
+    try {  
+      if(dir->exist()){
+        //Repack tape repair workflow
         filesInDirectory = dir->getFilesName();
-      } catch (const cta::exception::XrootCl &ex) {
-        log::ScopedParamContainer spc(lc);
-        spc.add("vid",repackInfo.vid);
-        spc.add("errorMessage",ex.getMessageValue());
-        lc.log(log::WARNING,"In Scheduler::expandRepackRequest(), received XRootdException while listing files in the buffer");
-      }
-    } else {
-      if(repackInfo.noRecall){
-        //The buffer directory should be created if the --no-recall flag has been passed
-        //So we throw an exception 
-        throw ExpandRepackRequestException("In Scheduler::expandRepackRequest(): the flag --no-recall is set but no buffer directory has been created.");
-      }
-      try {
+      } else {
+        if(repackInfo.noRecall){
+          //The buffer directory should be created if the --no-recall flag has been passed
+          //So we throw an exception 
+          throw ExpandRepackRequestException("In Scheduler::expandRepackRequest(): the flag --no-recall is set but no buffer directory has been created.");
+        }
         dir->mkdir();
-      } catch (const cta::exception::XrootCl &ex){
-        throw ExpandRepackRequestException(ex.getMessageValue());
       }
+    } catch (const cta::exception::XrootCl &ex) {
+      throw ExpandRepackRequestException("In Scheduler::expandRepackRequest(): errors while doing some checks on the repack buffer. ExceptionMsg = " + ex.getMessageValue());
     }
   }
   
@@ -580,7 +573,7 @@ void Scheduler::expandRepackRequest(std::unique_ptr<RepackRequest>& repackReques
             }
           }
           if(retrieveSubRequest.copyNbsToRearchive.size() < filesToArchive){
-            deleteRepackBuffer(std::move(dir));
+            deleteRepackBuffer(std::move(dir),lc);
             throw ExpandRepackRequestException("In Scheduler::expandRepackRequest(): Missing archive routes for the creation of the new copies of the files");
           }
         } else {
@@ -592,7 +585,7 @@ void Scheduler::expandRepackRequest(std::unique_ptr<RepackRequest>& repackReques
         }
       } else {
         //No storage class have been found for the current tapefile throw an exception
-        deleteRepackBuffer(std::move(dir));
+        deleteRepackBuffer(std::move(dir),lc);
         throw ExpandRepackRequestException("In Scheduler::expandRepackRequest(): No storage class have been found for the file to add copies");
       }
     }
@@ -642,7 +635,7 @@ void Scheduler::expandRepackRequest(std::unique_ptr<RepackRequest>& repackReques
     // value in case of crash.
     nbRetrieveSubrequestsQueued = repackRequest->m_dbReq->addSubrequestsAndUpdateStats(retrieveSubrequests, archiveRoutesMap, fSeq, maxAddedFSeq, totalStatsFile, diskSystemList, lc);
   } catch(const cta::ExpandRepackRequestException& e){
-    deleteRepackBuffer(std::move(dir));
+    deleteRepackBuffer(std::move(dir),lc);
     throw e;
   }
   timingList.insertAndReset("addSubrequestsAndUpdateStatsTime",t);
@@ -654,7 +647,7 @@ void Scheduler::expandRepackRequest(std::unique_ptr<RepackRequest>& repackReques
   if(archiveFilesFromCatalogue.empty() && totalStatsFile.totalFilesToArchive == 0 && (totalStatsFile.totalFilesToRetrieve == 0 || nbRetrieveSubrequestsQueued == 0)){
     //If no files have been retrieve, the repack buffer will have to be deleted
     //TODO : in case of Repack tape repair, we should not try to delete the buffer
-    deleteRepackBuffer(std::move(dir));      
+    deleteRepackBuffer(std::move(dir),lc);      
   }
   repackRequest->m_dbReq->expandDone();
   lc.log(log::INFO,"In Scheduler::expandRepackRequest(), repack request expanded");
@@ -1096,9 +1089,15 @@ cta::optional<common::dataStructures::LogicalLibrary> Scheduler::getLogicalLibra
   return ret;
 }
 
-void Scheduler::deleteRepackBuffer(std::unique_ptr<cta::disk::Directory> repackBuffer) {
-  if(repackBuffer != nullptr && repackBuffer->exist()){
-    repackBuffer->rmdir();
+void Scheduler::deleteRepackBuffer(std::unique_ptr<cta::disk::Directory> repackBuffer, cta::log::LogContext & lc) {
+  try{
+    if(repackBuffer != nullptr && repackBuffer->exist()){
+      repackBuffer->rmdir();
+    }
+  } catch (const cta::exception::XrootCl & ex) {
+    log::ScopedParamContainer spc(lc);
+    spc.add("exceptionMsg",ex.getMessageValue());
+    lc.log(log::ERR,"In Scheduler::deleteRepackBuffer() unable to delete the directory located in " + repackBuffer->getURL());
   }
 }
 
diff --git a/scheduler/Scheduler.hpp b/scheduler/Scheduler.hpp
index e46dc47df8..29a76a8b72 100644
--- a/scheduler/Scheduler.hpp
+++ b/scheduler/Scheduler.hpp
@@ -306,7 +306,7 @@ private:
   
   cta::optional<common::dataStructures::LogicalLibrary> getLogicalLibrary(const std::string &libraryName, double &getLogicalLibraryTime);
   
-  void deleteRepackBuffer(std::unique_ptr<cta::disk::Directory> repackBuffer);
+  void deleteRepackBuffer(std::unique_ptr<cta::disk::Directory> repackBuffer, cta::log::LogContext & lc);
   
   uint64_t getNbFilesAlreadyArchived(const common::dataStructures::ArchiveFile& archiveFile);
   
-- 
GitLab