From a8b061d3288b1ec30b7a5de7d6871366eaa9aab0 Mon Sep 17 00:00:00 2001
From: Michael Davis <michael.davis@cern.ch>
Date: Mon, 5 Nov 2018 16:42:03 +0100
Subject: [PATCH] [os-generic-queues] Correctly log
 totalReportRetries/maxReportRetries

---
 objectstore/ArchiveRequest.cpp  |  4 ++--
 objectstore/ArchiveRequest.hpp  |  2 +-
 objectstore/RetrieveRequest.cpp |  6 ++++--
 objectstore/RetrieveRequest.hpp |  2 +-
 scheduler/OStoreDB/OStoreDB.cpp | 24 ------------------------
 scheduler/SchedulerTest.cpp     |  8 ++++++++
 6 files changed, 16 insertions(+), 30 deletions(-)

diff --git a/objectstore/ArchiveRequest.cpp b/objectstore/ArchiveRequest.cpp
index 86044d85e0..b80116171b 100644
--- a/objectstore/ArchiveRequest.cpp
+++ b/objectstore/ArchiveRequest.cpp
@@ -54,7 +54,7 @@ void cta::objectstore::ArchiveRequest::initialize() {
 
 void cta::objectstore::ArchiveRequest::addJob(uint16_t copyNumber,
   const std::string& tapepool, const std::string& initialOwner, 
-    uint16_t maxRetiesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries) {
+    uint16_t maxRetriesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries) {
   checkPayloadWritable();
   auto *j = m_payload.add_jobs();
   j->set_copynb(copyNumber);
@@ -65,7 +65,7 @@ void cta::objectstore::ArchiveRequest::addJob(uint16_t copyNumber,
   j->set_totalretries(0);
   j->set_retrieswithinmount(0);
   j->set_lastmountwithfailure(0);
-  j->set_maxretrieswithinmount(maxRetiesWithinMount);
+  j->set_maxretrieswithinmount(maxRetriesWithinMount);
   j->set_maxtotalretries(maxTotalRetries);
   j->set_totalreportretries(0);
   j->set_maxreportretries(maxReportRetries);
diff --git a/objectstore/ArchiveRequest.hpp b/objectstore/ArchiveRequest.hpp
index 9020ca385f..7929ebe3b9 100644
--- a/objectstore/ArchiveRequest.hpp
+++ b/objectstore/ArchiveRequest.hpp
@@ -49,7 +49,7 @@ public:
   void setOwner(const std::string &) = delete;
   // Job management ============================================================
   void addJob(uint16_t copyNumber, const std::string & tapepool,
-    const std::string & initialOwner, uint16_t maxRetiesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries);
+    const std::string & initialOwner, uint16_t maxRetriesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries);
   struct RetryStatus {
     uint64_t retriesWithinMount = 0;
     uint64_t maxRetriesWithinMount = 0;
diff --git a/objectstore/RetrieveRequest.cpp b/objectstore/RetrieveRequest.cpp
index f978e4f107..a071fa5dfe 100644
--- a/objectstore/RetrieveRequest.cpp
+++ b/objectstore/RetrieveRequest.cpp
@@ -268,14 +268,14 @@ queueForTransfer:;
 //------------------------------------------------------------------------------
 // RetrieveRequest::addJob()
 //------------------------------------------------------------------------------
-void RetrieveRequest::addJob(uint64_t copyNb, uint16_t maxRetiesWithinMount, uint16_t maxTotalRetries,
+void RetrieveRequest::addJob(uint64_t copyNb, uint16_t maxRetriesWithinMount, uint16_t maxTotalRetries,
   uint16_t maxReportRetries)
 {
   checkPayloadWritable();
   auto *tf = m_payload.add_jobs();
   tf->set_copynb(copyNb);
   tf->set_lastmountwithfailure(0);
-  tf->set_maxretrieswithinmount(maxRetiesWithinMount);
+  tf->set_maxretrieswithinmount(maxRetriesWithinMount);
   tf->set_maxtotalretries(maxTotalRetries);
   tf->set_retrieswithinmount(0);
   tf->set_totalretries(0);
@@ -521,6 +521,8 @@ RetrieveRequest::RetryStatus RetrieveRequest::getRetryStatus(const uint16_t copy
       ret.maxRetriesWithinMount = j.maxretrieswithinmount();
       ret.totalRetries = j.totalretries();
       ret.maxTotalRetries = j.maxtotalretries();
+      ret.totalReportRetries = j.totalreportretries();
+      ret.maxReportRetries = j.maxreportretries();
       return ret;
     }
   }
diff --git a/objectstore/RetrieveRequest.hpp b/objectstore/RetrieveRequest.hpp
index 665ac0cb83..182751e8bd 100644
--- a/objectstore/RetrieveRequest.hpp
+++ b/objectstore/RetrieveRequest.hpp
@@ -46,7 +46,7 @@ public:
   void garbageCollect(const std::string &presumedOwner, AgentReference & agentReference, log::LogContext & lc,
     cta::catalogue::Catalogue & catalogue) override;
   // Job management ============================================================
-  void addJob(uint64_t copyNumber, uint16_t maxRetiesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries);
+  void addJob(uint64_t copyNumber, uint16_t maxRetriesWithinMount, uint16_t maxTotalRetries, uint16_t maxReportRetries);
   std::string getLastActiveVid();
   void setFailureReason(const std::string & reason);
   class JobDump {
diff --git a/scheduler/OStoreDB/OStoreDB.cpp b/scheduler/OStoreDB/OStoreDB.cpp
index 258d23db54..bf516da7c0 100644
--- a/scheduler/OStoreDB/OStoreDB.cpp
+++ b/scheduler/OStoreDB/OStoreDB.cpp
@@ -2716,30 +2716,6 @@ void OStoreDB::RetrieveJob::failReport(const std::string &failureReason, log::Lo
       return;
     }
   }
-#if 0
-// store job in failed container
--      // Algorithms suppose the objects are not locked
--      auto retryStatus = m_retrieveRequest.getRetryStatus(selectedCopyNb);
--      m_retrieveRequest.commit();
--      rel.release();
--      typedef objectstore::ContainerAlgorithms<RetrieveQueue,RetrieveQueueFailed> CaRqtr;
--      CaRqtr caRqtr(m_oStoreDB.m_objectStore, *m_oStoreDB.m_agentReference);
--      CaRqtr::InsertedElement::list insertedElements;
--      insertedElements.push_back(CaRqtr::InsertedElement{&m_retrieveRequest, selectedCopyNb, archiveFile, cta::nullopt, cta::nullopt });
--      caRqtr.referenceAndSwitchOwnership(tapeFile.vid, objectstore::QueueType::FailedJobs, insertedElements, lc);
--      log::ScopedParamContainer params(lc);
--      params.add("fileId", archiveFile.archiveFileID)
--            .add("copyNb", selectedCopyNb)
--            .add("failureReason", failureReason)
--            .add("requestObject", m_retrieveRequest.getAddressIfSet())
--            .add("retriesWithinMount", retryStatus.retriesWithinMount)
--            .add("maxRetriesWithinMount", retryStatus.maxRetriesWithinMount)
--            .add("totalRetries", retryStatus.totalRetries)
--            .add("maxTotalRetries", retryStatus.maxTotalRetries);
--      lc.log(log::INFO,
--          "In RetrieveJob::failTransfer(): stored job in failed container for operator handling.");
--      return;
-#endif
 }
 
 //------------------------------------------------------------------------------
diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp
index 3b494bbbc4..e4e6de4060 100644
--- a/scheduler/SchedulerTest.cpp
+++ b/scheduler/SchedulerTest.cpp
@@ -799,6 +799,14 @@ std::cerr << "Attempt " << i << std::endl;
     ASSERT_EQ(1, retrieveJobToReportList.size());
     // Fail the report
     retrieveJobToReportList.front()->reportFailed("Report failed", lc);
+    // Job should still be on the report queue
+    retrieveJobToReportList = scheduler.getNextRetrieveJobsToReportBatch(1,lc);
+    ASSERT_EQ(1, retrieveJobToReportList.size());
+    // Fail the report again
+    retrieveJobToReportList.front()->reportFailed("Report failed", lc);
+    // Job should be gone from the report queue
+    retrieveJobToReportList = scheduler.getNextRetrieveJobsToReportBatch(1,lc);
+    ASSERT_EQ(0, retrieveJobToReportList.size());
   }
 }
 
-- 
GitLab