[cta-admin] Adds "failedrequest rm --objectid"

Also deletes all unused options which were defined for commands that
no longer exist

ReleaseNotes.md

@@ -7,6 +7,7 @@ This release is a bug fix release.
 ### Features
 
 - cta/CTA#863 Prevent SQLite database files from being used as the CTA catalogue database backend
+- cta/CTA#870 Adds "cta-admin failedrequest rm" command
 
 ### Modifications
 
@@ -16,7 +17,7 @@ This release is a bug fix release.
 
 - cta/CTA#862 Unable to delete tabtest tape pool because it is in an archive route
 - cta/CTA#860 Correct contents of cta-lib-catalogue RPM and correct dependencies on it
-- Reinstates "cta-admin failedrequest --summary" option
+- Reinstates missing "cta-admin failedrequest ls --summary" option
 - cta/CTA#865 Empty the RetrieveQueue in the case of cancellation of a retrieve request when the drive is down
 
 

cmdline/CtaAdminCmdParse.hpp

@@ -233,7 +233,6 @@ const subcmdLookup_t subcmdLookup = {
    { "reclaim",                 AdminCmd::SUBCMD_RECLAIM },
    { "retry",                   AdminCmd::SUBCMD_RETRY },
    { "rm",                      AdminCmd::SUBCMD_RM },
-   { "show",                    AdminCmd::SUBCMD_SHOW },
    { "up",                      AdminCmd::SUBCMD_UP },
    { "down",                    AdminCmd::SUBCMD_DOWN },
 };
@@ -253,7 +252,6 @@ const std::map<std::string, OptionBoolean::Key> boolOptions = {
    { "--readonly",              OptionBoolean::READ_ONLY },
 
    // hasOption options
-   { "--checkchecksum",         OptionBoolean::CHECK_CHECKSUM },
    { "--disabledtape",          OptionBoolean::DISABLED },
    { "--justarchive",           OptionBoolean::JUSTARCHIVE },
    { "--justmove",              OptionBoolean::JUSTMOVE },
@@ -275,27 +273,19 @@ const std::map<std::string, OptionUInt64::Key> uint64Options = {
    { "--archivepriority",       OptionUInt64::ARCHIVE_PRIORITY },
    { "--capacity",              OptionUInt64::CAPACITY },
    { "--copynb",                OptionUInt64::COPY_NUMBER },
-   { "--firstfseq",             OptionUInt64::FIRST_FSEQ },
-   { "--gid",                   OptionUInt64::GID },
-   { "--id",                    OptionUInt64::ARCHIVE_FILE_ID },
-   { "--lastfseq",              OptionUInt64::LAST_FSEQ },
    { "--maxdrivesallowed",      OptionUInt64::MAX_DRIVES_ALLOWED },
    { "--maxlpos",               OptionUInt64::MAX_LPOS },
    { "--minarchiverequestage",  OptionUInt64::MIN_ARCHIVE_REQUEST_AGE },
    { "--minlpos",               OptionUInt64::MIN_LPOS },
    { "--minretrieverequestage", OptionUInt64::MIN_RETRIEVE_REQUEST_AGE },
-   { "--nbfiles",               OptionUInt64::NUMBER_OF_FILES },
    { "--nbwraps",               OptionUInt64::NUMBER_OF_WRAPS },
-   { "--partial",               OptionUInt64::PARTIAL }, 
    { "--partialtapesnumber",    OptionUInt64::PARTIAL_TAPES_NUMBER },
    { "--primarydensitycode",    OptionUInt64::PRIMARY_DENSITY_CODE },
    { "--retrievepriority",      OptionUInt64::RETRIEVE_PRIORITY },
    { "--secondarydensitycode",  OptionUInt64::SECONDARY_DENSITY_CODE },
-   { "--size",                  OptionUInt64::FILE_SIZE },
    { "--refreshinterval",       OptionUInt64::REFRESH_INTERVAL },
    { "--targetedfreespace",     OptionUInt64::TARGETED_FREE_SPACE },
    { "--sleeptime",             OptionUInt64::SLEEP_TIME },
-   { "--uid",                   OptionUInt64::OWNER_UID }
 };
 
 
@@ -307,19 +297,14 @@ const std::map<std::string, OptionString::Key> strOptions = {
    { "--bufferurl",             OptionString::BUFFERURL }, 
    { "--cartridge",             OptionString::CARTRIDGE },
    { "--comment",               OptionString::COMMENT },
-   { "--diskid",                OptionString::DISKID },
    { "--drive",                 OptionString::DRIVE },
    { "--encryptionkeyname",     OptionString::ENCRYPTION_KEY_NAME },
    { "--fxid",                  OptionString::FXID },
-   { "--file",                  OptionString::FILENAME },
-   { "--hostname",              OptionString::HOSTNAME },
-   { "--input",                 OptionString::INPUT },
    { "--instance",              OptionString::INSTANCE },
    { "--logicallibrary",        OptionString::LOGICAL_LIBRARY },
    { "--mediatype",             OptionString::MEDIA_TYPE },
    { "--mountpolicy",           OptionString::MOUNT_POLICY },
-   { "--output",                OptionString::OUTPUT },
-   { "--path",                  OptionString::PATH },
+   { "--objectid",              OptionString::OBJECTID },
    { "--storageclass",          OptionString::STORAGE_CLASS },
    { "--supply",                OptionString::SUPPLY },
    { "--tapepool",              OptionString::TAPE_POOL },
@@ -357,7 +342,7 @@ const std::map<AdminCmd::Cmd, CmdHelp> cmdHelp = {
                           "  --force option is not set, the drives will complete any running mount and\n"
                           "  drives must be in the down state before deleting.\n\n"
                                          }},
-   { AdminCmd::CMD_FAILEDREQUEST,        { "failedrequest",        "fr",  { "ls" } }},
+   { AdminCmd::CMD_FAILEDREQUEST,        { "failedrequest",        "fr",  { "ls", "rm" } }},
    { AdminCmd::CMD_GROUPMOUNTRULE,       { "groupmountrule",       "gmr", { "add", "ch", "rm", "ls" } }},
    { AdminCmd::CMD_LOGICALLIBRARY,       { "logicallibrary",       "ll",  { "add", "ch", "rm", "ls" } }},
    { AdminCmd::CMD_MEDIATYPE,            { "mediatype",            "mt",  { "add", "ch", "rm", "ls" } }},
@@ -417,16 +402,13 @@ const std::map<AdminCmd::Cmd, CmdHelp> cmdHelp = {
  * Enumerate options
  */
 const Option opt_all                  { Option::OPT_FLAG, "--all",                   "-a",   "" };
-const Option opt_archivefileid        { Option::OPT_UINT, "--id",                    "-I",   " <archive_file_id>" };
 const Option opt_archivepriority      { Option::OPT_UINT, "--archivepriority",       "--ap", " <priority_value>" };
 const Option opt_bufferurl            { Option::OPT_STR,  "--bufferurl",             "-b",   " <buffer URL>" };
 const Option opt_capacity             { Option::OPT_UINT, "--capacity",              "-c",   " <capacity_in_bytes>" };
 const Option opt_cartridge            { Option::OPT_STR,  "--cartridge",             "-t",   " <cartridge>" };
-const Option opt_checkchecksum        { Option::OPT_FLAG, "--checkchecksum",         "-c",   "" };
 const Option opt_comment              { Option::OPT_STR,  "--comment",               "-m",   " <\"comment\">" };
 const Option opt_copynb               { Option::OPT_UINT, "--copynb",                "-c",   " <copy_number>" };
 const Option opt_disabled             { Option::OPT_BOOL, "--disabled",              "-d",   " <\"true\" or \"false\">" };
-const Option opt_diskid               { Option::OPT_STR,  "--diskid",                "-d",   " <disk_id>" };
 const Option opt_drivename            { Option::OPT_STR,  "--drive",                 "-d",   " <drive_name>" };
 const Option opt_drivename_cmd        { Option::OPT_CMD,  "--drive",                 "",     "<drive_name>" };
 const Option opt_encrypted            { Option::OPT_BOOL, "--encrypted",             "-e",   " <\"true\" or \"false\">" };
@@ -434,18 +416,13 @@ const Option opt_encryptionkeyname    { Option::OPT_STR,  "--encryptionkeyname",
 const Option opt_fid                  { Option::OPT_STR,  "--fxid",                  "-f",   " <eos_fxid>" };
 const Option opt_fidfile              { Option::OPT_STR_LIST, "--fxidfile",          "-F",   " <filename>" };
 const Option opt_filename             { Option::OPT_STR,  "--file",                  "-f",   " <filename>" };
-const Option opt_firstfseq            { Option::OPT_UINT, "--firstfseq",             "-f",   " <first_fseq>" };
 const Option opt_force                { Option::OPT_BOOL, "--force",                 "-f",   " <\"true\" or \"false\">" };
 const Option opt_force_flag           { Option::OPT_FLAG, "--force",                 "-f",   "" };
-const Option opt_gid                  { Option::OPT_UINT, "--gid",                   "-g",   " <group_id>" };
-const Option opt_hostname_alias       { Option::OPT_STR,  "--name",                  "-n",   " <host_name>", "--hostname" };
-const Option opt_input                { Option::OPT_STR,  "--input",                 "-i",   " <\"zero\" or \"urandom\">" };
 const Option opt_instance             { Option::OPT_STR,  "--instance",              "-i",   " <disk_instance>" };
 const Option opt_justarchive          { Option::OPT_FLAG, "--justarchive",           "-a",   "" };
 const Option opt_justmove             { Option::OPT_FLAG, "--justmove",              "-m",   "" };
 const Option opt_justaddcopies        { Option::OPT_FLAG, "--justaddcopies",         "-a",   "" };
 const Option opt_justretrieve         { Option::OPT_FLAG, "--justretrieve",          "-r",   "" };
-const Option opt_lastfseq             { Option::OPT_UINT, "--lastfseq",              "-l",   " <last_fseq>" };
 const Option opt_log                  { Option::OPT_FLAG, "--log",                   "-l",   "" };
 const Option opt_logicallibrary       { Option::OPT_STR,  "--logicallibrary",        "-l",   " <logical_library_name>" };
 const Option opt_logicallibrary_alias { Option::OPT_STR,  "--name",                  "-n",   " <logical_library_name>", "--logicallibrary" };
@@ -459,18 +436,12 @@ const Option opt_minlpos              { Option::OPT_UINT, "--minlpos",
 const Option opt_minretrieverequestage{ Option::OPT_UINT, "--minretrieverequestage", "--ra", " <min_request_age>" };
 const Option opt_mountpolicy          { Option::OPT_STR,  "--mountpolicy",           "-u",   " <mount_policy_name>" };
 const Option opt_mountpolicy_alias    { Option::OPT_STR,  "--name",                  "-n",   " <mount_policy_name>", "--mountpolicy" };
-const Option opt_number_of_files      { Option::OPT_UINT, "--nbfiles",               "-n",   " <number_of_files_per_tape>" };
-const Option opt_number_of_files_alias{ Option::OPT_UINT, "--number",                "-n",   " <number_of_files>", "--nbfiles" };
 const Option opt_number_of_wraps      { Option::OPT_UINT, "--nbwraps",               "-w",   " <number_of_wraps>" };
-const Option opt_output               { Option::OPT_STR,  "--output",                "-o",   " <\"null\" or output_dir>" };
-const Option opt_owner_uid            { Option::OPT_UINT, "--uid",                   "-u",   " <owner_uid>" };
 const Option opt_partialfiles         { Option::OPT_UINT, "--partial",               "-p",   " <number_of_files_per_tape>" };
 const Option opt_partialtapes         { Option::OPT_UINT, "--partialtapesnumber",    "-p",   " <number_of_partial_tapes>" };
-const Option opt_path                 { Option::OPT_STR,  "--path",                  "-p",   " <full_path>" };
-const Option opt_primarydensitycode   { Option::OPT_UINT,  "--primarydensitycode",   "-p",   " <primary_density_code>" };
+const Option opt_primarydensitycode   { Option::OPT_UINT, "--primarydensitycode",    "-p",   " <primary_density_code>" };
 const Option opt_retrievepriority     { Option::OPT_UINT, "--retrievepriority",      "--rp", " <priority_value>" };
 const Option opt_secondarydensitycode { Option::OPT_UINT, "--secondarydensitycode",  "-s",   " <secondary_density_code>" };
-const Option opt_size                 { Option::OPT_UINT, "--size",                  "-s",   " <file_size>" };
 const Option opt_storageclass         { Option::OPT_STR,  "--storageclass",          "-s",   " <storage_class_name>" };
 const Option opt_storageclass_alias   { Option::OPT_STR,  "--name",                  "-n",   " <storage_class_name>", "--storageclass" };
 const Option opt_summary              { Option::OPT_FLAG, "--summary",               "-S",   "" };
@@ -495,6 +466,7 @@ const Option opt_sleep_time           { Option::OPT_UINT, "--sleeptime",
 const Option opt_reason               { Option::OPT_STR,  "--reason",                "-r",   " <reason_status_change>" };
 const Option opt_show_superseded      { Option::OPT_FLAG, "--showsuperseded",        "-s",   "" };
 const Option opt_no_recall            { Option::OPT_FLAG, "--no-recall",             "-nr",  "" };
+const Option opt_object_id            { Option::OPT_STR,  "--objectid",              "-o",   " <objectId>" };
 
 /*!
  * Map valid options to commands
@@ -521,6 +493,7 @@ const std::map<cmd_key_t, cmd_val_t> cmdOptions = {
    {{ AdminCmd::CMD_FAILEDREQUEST,        AdminCmd::SUBCMD_LS    },
       { opt_justarchive.optional(), opt_justretrieve.optional(), opt_tapepool.optional(),
         opt_vid.optional(), opt_log.optional(), opt_summary.optional() }},
+   {{ AdminCmd::CMD_FAILEDREQUEST,        AdminCmd::SUBCMD_RM    }, { opt_object_id }},
    /*----------------------------------------------------------------------------------------------------*/
    {{ AdminCmd::CMD_GROUPMOUNTRULE,       AdminCmd::SUBCMD_ADD   },
       { opt_instance, opt_username_alias, opt_mountpolicy, opt_comment }},

scheduler/OStoreDB/OStoreDB.cpp

@@ -1537,6 +1537,120 @@ void OStoreDB::cancelArchive(const common::dataStructures::DeleteArchiveRequest
   ar.remove();
 }
 
+//------------------------------------------------------------------------------
+// OStoreDB::deleteFailed()
+//------------------------------------------------------------------------------
+void OStoreDB::deleteFailed(const std::string &objectId, log::LogContext &lc)
+{
+  // Fetch the object
+  objectstore::GenericObject fr(objectId, m_objectStore);
+  objectstore::ScopedExclusiveLock sel;
+  try {
+    sel.lock(fr);
+    fr.fetch();
+  } catch (objectstore::Backend::NoSuchObject &) {
+    throw exception::Exception("Object " + objectId + " not found.");
+  }
+
+  // Validate that the object is an Archive or Retrieve request
+  if(fr.type() != objectstore::serializers::ArchiveRequest_t &&
+     fr.type() != objectstore::serializers::RetrieveRequest_t) {
+    throw exception::Exception("Object " + objectId + " is not an archive or retrieve request.");
+  }
+
+  // Make a list of all owners of the object
+  std::list<std::string> queueIds;
+  if(!fr.getOwner().empty()) queueIds.push_back(fr.getOwner());
+  if(!fr.getBackupOwner().empty()) queueIds.push_back(fr.getBackupOwner());
+  if(fr.type() == objectstore::serializers::ArchiveRequest_t) {
+    // Archive requests can be in two queues, so ownership is per job not per request
+    objectstore::ArchiveRequest ar(fr);
+    // We already hold a lock on the generic object
+    ar.fetchNoLock();
+    for(auto &job : ar.dumpJobs()) {
+      if(!job.owner.empty()) queueIds.push_back(job.owner);
+    }
+  }
+  if(queueIds.empty()) {
+    throw exception::Exception("Object " + objectId + " is not owned by any queue.");
+  }
+
+  // Make a set of failed archive or retrieve queues
+  std::set<std::string> failedQueueIds;
+  {
+    RootEntry re(m_objectStore);
+    ScopedExclusiveLock rel(re);
+    re.fetch();
+
+    switch(fr.type()) {
+      case objectstore::serializers::ArchiveRequest_t: {
+        auto archiveQueueList = re.dumpArchiveQueues(JobQueueType::FailedJobs);
+        for(auto &r : archiveQueueList) {
+          failedQueueIds.insert(r.address);
+        }
+        break;
+      }
+      case objectstore::serializers::RetrieveRequest_t: {
+        auto retrieveQueueList = re.dumpRetrieveQueues(JobQueueType::FailedJobs);
+        for(auto &r : retrieveQueueList) {
+          failedQueueIds.insert(r.address);
+        }
+        break;
+      }
+      default: ;
+    }
+  }
+
+  // Validate that all owners of the object are failed queues and therefore it is safe to delete the job
+  for(auto &qid : queueIds) {
+    if(failedQueueIds.find(qid) == failedQueueIds.end()) {
+      throw exception::Exception("Will not delete object " + objectId + "\nwhich is owned by " + qid);
+    }
+  }
+
+  // Checks passed, delete the request
+  log::ScopedParamContainer params(lc);
+  params.add("objectId", objectId);
+  int owner_no = 0;
+  for(auto &qid : queueIds) {
+    params.add("owner" + std::to_string(owner_no++), qid);
+  }
+
+  // Delete the references
+  lc.log(log::INFO, "OStoreDB::deleteFailed(): deleting references");
+  bool isQueueEmpty = false;
+  switch(fr.type()) {
+    case objectstore::serializers::ArchiveRequest_t: {
+      for(auto &arq_id : queueIds) {
+        ArchiveQueue arq(arq_id, m_objectStore);
+        ScopedExclusiveLock arq_el(arq);
+        arq.fetch();
+        arq.removeJobsAndCommit(std::list<std::string>(1, objectId));
+        if(arq.isEmpty()) isQueueEmpty = true;
+      }
+      break;
+    }
+    case objectstore::serializers::RetrieveRequest_t: {
+      for(auto &rrq_id : queueIds) {
+        RetrieveQueue rrq(rrq_id, m_objectStore);
+        ScopedExclusiveLock rrq_el(rrq);
+        rrq.fetch();
+        rrq.removeJobsAndCommit(std::list<std::string>(1, objectId));
+        if(rrq.isEmpty()) isQueueEmpty = true;
+      }
+      break;
+    }
+    default: ;
+  }
+
+  // Delete the request
+  lc.log(log::INFO, "OStoreDB::deleteFailed(): deleting failed request");
+  fr.remove();
+
+  // Trim empty queues
+  if(isQueueEmpty) trimEmptyQueues(lc);
+}
+
 //------------------------------------------------------------------------------
 // OStoreDB::getRetrieveJobs()
 //------------------------------------------------------------------------------

scheduler/OStoreDB/OStoreDB.hpp

@@ -362,6 +362,8 @@ public:
    */
   virtual void cancelArchive(const common::dataStructures::DeleteArchiveRequest& request, log::LogContext & lc) override;
 
+  virtual void deleteFailed(const std::string &objectId, log::LogContext &lc) override;
+
   std::list<cta::common::dataStructures::RetrieveJob> getRetrieveJobs(const std::string &vid) const override;
 
   std::map<std::string, std::list<common::dataStructures::RetrieveJob>> getRetrieveJobs() const override;

scheduler/OStoreDB/OStoreDBFactory.hpp

@@ -229,6 +229,10 @@ public:
     m_OStoreDB.cancelRetrieve(instanceName, rqst, lc);
   }
 
+  void deleteFailed(const std::string &objectId, log::LogContext & lc) override {
+    m_OStoreDB.deleteFailed(objectId, lc);
+  }
+
   void queueRepack(const SchedulerDatabase::QueueRepackRequest & repackRequest, log::LogContext& lc) override {
     m_OStoreDB.queueRepack(repackRequest, lc);
   }

scheduler/Scheduler.cpp

@@ -268,12 +268,16 @@ void Scheduler::deleteArchive(const std::string &instanceName, const common::dat
 }
 
 //------------------------------------------------------------------------------
-// cancelRetrieve
+// abortRetrieve
 //------------------------------------------------------------------------------
 void Scheduler::abortRetrieve(const std::string &instanceName, const common::dataStructures::CancelRetrieveRequest &request, log::LogContext & lc) {
   m_db.cancelRetrieve(instanceName, request, lc);
 }
 
+void Scheduler::deleteFailed(const std::string &objectId, log::LogContext & lc) {
+  m_db.deleteFailed(objectId, lc);
+}
+
 //------------------------------------------------------------------------------
 // updateFileInfo
 //------------------------------------------------------------------------------

scheduler/Scheduler.hpp

@@ -167,6 +167,11 @@ public:
   void abortRetrieve(const std::string &instanceName, 
     const cta::common::dataStructures::CancelRetrieveRequest &request, log::LogContext & lc);
   
+  /** 
+   * Delete a job from the failed queue.
+   */
+  void deleteFailed(const std::string &objectId, log::LogContext & lc);
+  
   /** 
    * Update the file information of an archived file.
    * Throws a UserError exception in case of wrong request parameters (ex. unknown file id)

scheduler/SchedulerDatabase.hpp

@@ -357,6 +357,13 @@ public:
    */
   virtual void cancelArchive(const common::dataStructures::DeleteArchiveRequest& request,  log::LogContext & lc) = 0;
   
+  /**
+   * Idempotently deletes the specified ArchiveRequest from the objectstore
+   * @param request, the ArchiveRequest to delete
+   * @param lc the LogContext
+   */
+  virtual void deleteFailed(const std::string &objectId, log::LogContext & lc) = 0;
+  
   /**
    * Returns all of the queued archive jobs.  The returned jobs are
    * grouped by tape pool and then sorted by creation time, oldest first.

xroot_plugins/XrdSsiCtaRequestMessage.cpp

@@ -137,6 +137,9 @@ void RequestMessage::process(const cta::xrd::Request &request, cta::xrd::Respons
             case cmd_pair(AdminCmd::CMD_FAILEDREQUEST, AdminCmd::SUBCMD_LS):
                processFailedRequest_Ls(response, stream);
                break;
+            case cmd_pair(AdminCmd::CMD_FAILEDREQUEST, AdminCmd::SUBCMD_RM):
+               processFailedRequest_Rm(response);
+               break;
             case cmd_pair(AdminCmd::CMD_GROUPMOUNTRULE, AdminCmd::SUBCMD_ADD):
                processGroupMountRule_Add(response);
                break;
@@ -1060,6 +1063,19 @@ void RequestMessage::processFailedRequest_Ls(cta::xrd::Response &response, XrdSs
 
 
 
+void RequestMessage::processFailedRequest_Rm(cta::xrd::Response &response)
+{
+   using namespace cta::admin;
+
+   auto &objectId = getRequired(OptionString::OBJECTID);
+
+   m_scheduler.deleteFailed(objectId, m_lc);
+
+   response.set_type(cta::xrd::Response::RSP_SUCCESS);
+}
+
+
+
 void RequestMessage::processGroupMountRule_Add(cta::xrd::Response &response)
 {
    using namespace cta::admin;

xroot_plugins/XrdSsiCtaRequestMessage.hpp

@@ -172,6 +172,7 @@ private:
   void processDrive_Down            (cta::xrd::Response &response);
   void processDrive_Ch              (cta::xrd::Response &response);
   void processDrive_Rm              (cta::xrd::Response &response);
+  void processFailedRequest_Rm      (cta::xrd::Response &response);
   void processGroupMountRule_Add    (cta::xrd::Response &response);
   void processGroupMountRule_Ch     (cta::xrd::Response &response);
   void processGroupMountRule_Rm     (cta::xrd::Response &response);

xrootd-ssi-protobuf-interface @ fe00c7bc

-Subproject commit 8f52c91184ee20987455ca82323937e9bda037c1
+Subproject commit fe00c7bc15bd5a95326bdeb2fe5f5fe2a9316ddc