From 56f6b5fb5ad4540d77bb24b356270d5c941c529c Mon Sep 17 00:00:00 2001
From: Joao Afonso <joao.afonso@cern.ch>
Date: Fri, 28 Mar 2025 15:44:56 +0100
Subject: [PATCH 1/3] Add option in cta-admin to identify dual tape files that
only have one copy - min age
---
catalogue/TapeSearchCriteria.hpp | 5 ++++
catalogue/rdbms/RdbmsTapeCatalogue.cpp | 7 +++++-
cmdline/CtaAdminCmdParse.hpp | 4 +--
.../configmaps/cta-frontend-xrootd.yaml | 5 ++++
.../orchestration/helm/frontend/values.yaml | 2 ++
frontend/common/AdminCmd.cpp | 3 ++-
frontend/common/AdminCmd.hpp | 25 +++++++++++++------
frontend/common/FrontendService.cpp | 14 +++++++++++
frontend/common/FrontendService.hpp | 6 +++++
frontend/grpc/RequestMessage.hpp | 11 +++++---
frontend/grpc/ServerTapeLsRequestHandler.cpp | 4 +--
frontend/grpc/cta-frontend-grpc.conf.example | 4 +++
xroot_plugins/XrdCtaTapeLs.hpp | 3 +++
.../cta-frontend-xrootd.conf.example | 5 ++++
14 files changed, 82 insertions(+), 16 deletions(-)
diff --git a/catalogue/TapeSearchCriteria.hpp b/catalogue/TapeSearchCriteria.hpp
index 05160b6b2e..61cc0101e6 100644
--- a/catalogue/TapeSearchCriteria.hpp
+++ b/catalogue/TapeSearchCriteria.hpp
@@ -100,6 +100,11 @@ struct TapeSearchCriteria {
*/
std::optional<bool> checkMissingFileCopies;
+ /**
+ * Missing tape file copies minimum age in secs
+ */
+ uint64_t missingFileCopiesMinAgeSecs;
+
/**
* The state of the tapes to look for
*/
diff --git a/catalogue/rdbms/RdbmsTapeCatalogue.cpp b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
index 308a297ca2..39856975be 100644
--- a/catalogue/rdbms/RdbmsTapeCatalogue.cpp
+++ b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
@@ -1443,7 +1443,7 @@ std::list<common::dataStructures::Tape> RdbmsTapeCatalogue::getTapes(rdbms::Conn
WHERE
SC.NB_COPIES > 1
GROUP BY
- AF.ARCHIVE_FILE_ID, SC.NB_COPIES
+ AF.ARCHIVE_FILE_ID, SC.NB_COPIES AND AF.CREATION_TIME < :MAX_CREATION_TIME
HAVING
SC.NB_COPIES <> COUNT(TF.ARCHIVE_FILE_ID)
) MISSING_COPIES
@@ -1474,6 +1474,11 @@ std::list<common::dataStructures::Tape> RdbmsTapeCatalogue::getTapes(rdbms::Conn
throw cta::exception::UserError(std::string("The state provided does not exist. Possible values are: ")
+ cta::common::dataStructures::Tape::getAllPossibleStates());
}
+ if (searchCriteria.checkMissingFileCopies.value_or(false)) {
+ uint64_t max_creation_time = time(nullptr);
+ max_creation_time -= searchCriteria.missingFileCopiesMinAgeSecs;
+ stmt.bindUint64(":MAX_CREATION_TIME", max_creation_time);
+ }
// Disk file ID lookup requires multiple queries
std::vector<std::string>::const_iterator diskFileId_it;
diff --git a/cmdline/CtaAdminCmdParse.hpp b/cmdline/CtaAdminCmdParse.hpp
index 26cfa749af..96fede545a 100644
--- a/cmdline/CtaAdminCmdParse.hpp
+++ b/cmdline/CtaAdminCmdParse.hpp
@@ -517,7 +517,7 @@ const Option opt_archive_route_type {Option::OPT_STR,
std::string(R"( <")") +
cta::common::dataStructures::toString(ArchiveRouteType::DEFAULT) + R"(" or ")" +
cta::common::dataStructures::toString(ArchiveRouteType::REPACK) + R"(">)"};
-const Option opt_missingfilecopes {Option::OPT_FLAG, "--missingfilecopies", "--mfc", ""};
+const Option opt_missingfilecopies {Option::OPT_FLAG, "--missingfilecopies", "--mfc", ""};
/*!
* Subset of commands that return streaming output
@@ -1037,7 +1037,7 @@ tape (ta)
opt_fromcastor.optional(),
opt_purchase_order.optional(),
opt_physical_library.optional(),
- opt_missingfilecopes.optional()} },
+ opt_missingfilecopies.optional()} },
/**md
tapefile (tf)
diff --git a/continuousintegration/orchestration/helm/frontend/templates/configmaps/cta-frontend-xrootd.yaml b/continuousintegration/orchestration/helm/frontend/templates/configmaps/cta-frontend-xrootd.yaml
index 8955217583..aac4a4ec24 100644
--- a/continuousintegration/orchestration/helm/frontend/templates/configmaps/cta-frontend-xrootd.yaml
+++ b/continuousintegration/orchestration/helm/frontend/templates/configmaps/cta-frontend-xrootd.yaml
@@ -31,6 +31,11 @@ data:
# CTA Catalogue options
cta.catalogue.numberofconnections {{ .Values.conf.catalogue.numberOfConnections }}
+ # Missing tape file copies minimum age.
+ # Set it to prevent false flagging of multi-copy files in the process of being archived tapes.
+ # Default 24 hours
+ cta.catalogue.missing_file_copies_min_age_secs {{ .Values.conf.catalogue.missingFileCopiesMinAgeSecs }}
+
# Maximum file size (in GB) that the CTA Frontend will accept for archiving
cta.archivefile.max_size_gb {{ .Values.conf.frontend.archiveFileMaxSizeGb }}
diff --git a/continuousintegration/orchestration/helm/frontend/values.yaml b/continuousintegration/orchestration/helm/frontend/values.yaml
index e320c0f09c..df9a0beaad 100644
--- a/continuousintegration/orchestration/helm/frontend/values.yaml
+++ b/continuousintegration/orchestration/helm/frontend/values.yaml
@@ -6,6 +6,8 @@ replicaCount: 1
conf:
catalogue:
numberOfConnections: 10
+ # Reduced value for test environment
+ missingFileCopiesMinAgeSecs: 5
frontend:
instanceName: "CI"
diff --git a/frontend/common/AdminCmd.cpp b/frontend/common/AdminCmd.cpp
index 67d7ee6ee6..6422100221 100644
--- a/frontend/common/AdminCmd.cpp
+++ b/frontend/common/AdminCmd.cpp
@@ -37,7 +37,8 @@ AdminCmd::AdminCmd(const frontend::FrontendService& frontendService,
m_cliIdentity(clientIdentity),
m_archiveFileMaxSize(frontendService.getArchiveFileMaxSize()),
m_repackBufferURL(frontendService.getRepackBufferURL()),
- m_repackMaxFilesToSelect(frontendService.getRepackMaxFilesToSelect())
+ m_repackMaxFilesToSelect(frontendService.getRepackMaxFilesToSelect()),
+ m_missingFileCopiesMinAgeSecs(frontendService.getMissingFileCopiesMinAgeSecs())
{
m_lc.pushOrReplace({"user", m_cliIdentity.username + "@" + m_cliIdentity.host});
diff --git a/frontend/common/AdminCmd.hpp b/frontend/common/AdminCmd.hpp
index cdacf4fd4d..bf97d5e228 100644
--- a/frontend/common/AdminCmd.hpp
+++ b/frontend/common/AdminCmd.hpp
@@ -102,14 +102,19 @@ public:
* or not. In the case of flags, they should always have the value true if the flag is
* present, but we do a redundant check anyway.
*
- * @param[in] option Optional command line option
+ * @param[in] option Optional command line option
+ * @param[out] has_option Set to true if the option exists, unmodified if it does not
*
* @retval true The flag is present in the options map, and its value is true
* @retval false The flag is either not present or is present and set to false
*/
- bool has_flag(admin::OptionBoolean::Key option) const {
+ bool has_flag(admin::OptionBoolean::Key option, bool *has_option = nullptr) const {
auto opt_it = m_option_bool.find(option);
- return opt_it != m_option_bool.end() && opt_it->second;
+ if (opt_it != m_option_bool.end()) {
+ if(has_option != nullptr) *has_option = true;
+ return opt_it->second;
+ }
+ return false;
}
/*!
@@ -125,6 +130,11 @@ public:
*/
std::optional<std::string> getAndValidateDiskFileIdOptional(bool* has_any = nullptr) const;
+ /*!
+ * @return The missing tape file copies minimum age.
+ */
+ uint64_t getMissingFileCopiesMinAgeSecs() const { return m_missingFileCopiesMinAgeSecs; }
+
protected:
/*!
* Convert AdminCmd <Cmd, SubCmd> pair to an integer so that it can be used in a switch statement
@@ -237,10 +247,11 @@ private:
void processRecycleTapeFile_Restore (xrd::Response& response);
void processModifyArchiveFile (xrd::Response& response);
- common::dataStructures::SecurityIdentity m_cliIdentity; //!< Client identity: username, host, authentication
- const uint64_t m_archiveFileMaxSize; //!< Maximum allowed file size for archive requests
- const std::optional<std::string> m_repackBufferURL; //!< Repack buffer URL
- const std::optional<std::uint64_t> m_repackMaxFilesToSelect;//!< Repack max files to expand
+ common::dataStructures::SecurityIdentity m_cliIdentity; //!< Client identity: username, host, authentication
+ const uint64_t m_archiveFileMaxSize; //!< Maximum allowed file size for archive requests
+ const std::optional<std::string> m_repackBufferURL; //!< Repack buffer URL
+ const std::optional<std::uint64_t> m_repackMaxFilesToSelect; //!< Repack max files to expand
+ const uint64_t m_missingFileCopiesMinAgeSecs; //!< Missing tape file copies minimum age
// Command options extracted from protobuf
std::map<admin::OptionBoolean::Key, bool> m_option_bool; //!< Boolean options
diff --git a/frontend/common/FrontendService.cpp b/frontend/common/FrontendService.cpp
index 1d04598ec6..6b16f0a5c8 100644
--- a/frontend/common/FrontendService.cpp
+++ b/frontend/common/FrontendService.cpp
@@ -116,6 +116,20 @@ FrontendService::FrontendService(const std::string& configFilename) : m_archiveF
log(log::INFO, std::string("Starting cta-frontend"), params);
}
+ auto missingFileCopiesMinAgeSecs = config.getOptionValueUInt("cta.catalogue.missing_file_copies_min_age_secs");
+ m_missingFileCopiesMinAgeSecs = missingFileCopiesMinAgeSecs.value_or(0);
+
+ {
+ // Log cta.archivefile.max_size_gb
+ std::list<log::Param> params;
+ params.push_back(log::Param("source", missingFileCopiesMinAgeSecs.has_value() ? configFilename : "Compile time default"));
+ params.push_back(log::Param("category", "cta.catalogue"));
+ params.push_back(log::Param("key", "missingFileCopiesMinAgeSecs"));
+ params.push_back(
+ log::Param("value", std::to_string(missingFileCopiesMinAgeSecs.value_or(0))));
+ log(log::INFO, "Configuration entry", params);
+ }
+
// Initialise the Catalogue
std::string catalogueConfigFile = "/etc/cta/cta-catalogue.conf";
const rdbms::Login catalogueLogin = rdbms::Login::parseFile(catalogueConfigFile);
diff --git a/frontend/common/FrontendService.hpp b/frontend/common/FrontendService.hpp
index 04d13f1dab..79de900c6c 100644
--- a/frontend/common/FrontendService.hpp
+++ b/frontend/common/FrontendService.hpp
@@ -69,6 +69,11 @@ public:
return m_schedulerBackendName;
}
+ /*!
+ * Get missing tape file copies minimum age
+ */
+ uint64_t getMissingFileCopiesMinAgeSecs() const { return m_missingFileCopiesMinAgeSecs; }
+
/*!
* Get a reference to the Scheduler
*/
@@ -186,6 +191,7 @@ private:
std::optional<std::string> m_tlsKey; //!< The TLS service key file
std::optional<std::string> m_tlsCert; //!< The TLS service certificate file
std::optional<std::string> m_tlsChain; //!< The TLS CA chain file
+ uint64_t m_missingFileCopiesMinAgeSecs; //!< Missing tape file copies minimum age.
// clang-format on
};
diff --git a/frontend/grpc/RequestMessage.hpp b/frontend/grpc/RequestMessage.hpp
index de5237b15f..69073e4b28 100644
--- a/frontend/grpc/RequestMessage.hpp
+++ b/frontend/grpc/RequestMessage.hpp
@@ -103,14 +103,19 @@ public:
* or not. In the case of flags, they should always have the value true if the flag is
* present, but we do a redundant check anyway.
*
- * @param[in] option Optional command line option
+ * @param[in] option Optional command line option
+ * @param[out] has_option Set to true if the option exists, unmodified if it does not
*
* @retval true The flag is present in the options map, and its value is true
* @retval false The flag is either not present or is present and set to false
*/
- bool hasFlag(cta::admin::OptionBoolean::Key option) const {
+ bool has_flag(cta::admin::OptionBoolean::Key option, bool *has_option = nullptr) const {
auto opt_it = m_option_bool.find(option);
- return opt_it != m_option_bool.end() && opt_it->second;
+ if (opt_it != m_option_bool.end()) {
+ if(has_option != nullptr) *has_option = true;
+ return opt_it->second;
+ }
+ return false;
}
private:
diff --git a/frontend/grpc/ServerTapeLsRequestHandler.cpp b/frontend/grpc/ServerTapeLsRequestHandler.cpp
index 47f6881704..89405187d1 100644
--- a/frontend/grpc/ServerTapeLsRequestHandler.cpp
+++ b/frontend/grpc/ServerTapeLsRequestHandler.cpp
@@ -100,13 +100,13 @@ bool cta::frontend::grpc::server::TapeLsRequestHandler::next(const bool bOk) {
m_searchCriteria.state = common::dataStructures::Tape::stringToState(stateOpt.value());
}
- if(!(requestMsg.hasFlag(cta::admin::OptionBoolean::ALL) || bHasAny)) {
+ if(!(requestMsg.has_flag(cta::admin::OptionBoolean::ALL) || bHasAny)) {
lc.log(cta::log::ERR, "In grpc::server::TapeLsRequestHandler::next(): Must specify at least one search option, or --all.");
m_response.mutable_header()->set_type(cta::xrd::Response::RSP_ERR_USER);
m_response.mutable_header()->set_show_header(cta::admin::HeaderType::NONE);
m_response.mutable_header()->set_message_txt("Must specify at least one search option, or --all.");
m_streamState = StreamState::ERROR;
- } else if(requestMsg.hasFlag(cta::admin::OptionBoolean::ALL) && bHasAny) {
+ } else if(requestMsg.has_flag(cta::admin::OptionBoolean::ALL) && bHasAny) {
lc.log(cta::log::ERR, "In grpc::server::TapeLsRequestHandler::next(): Cannot specify --all together with other search options.");
m_response.mutable_header()->set_type(cta::xrd::Response::RSP_ERR_USER);
m_response.mutable_header()->set_show_header(cta::admin::HeaderType::NONE);
diff --git a/frontend/grpc/cta-frontend-grpc.conf.example b/frontend/grpc/cta-frontend-grpc.conf.example
index bddb45e40e..73b778f532 100644
--- a/frontend/grpc/cta-frontend-grpc.conf.example
+++ b/frontend/grpc/cta-frontend-grpc.conf.example
@@ -100,6 +100,10 @@ cta.log.url file:/var/log/cta/cta-frontend.log
# CTA Catalogue options
cta.catalogue.numberofconnections 10
+# Missing tape file copies minimum age.
+# Set it to prevent false flagging of multi-copy files in the process of being archived tapes.
+# Default 24 hours
+cta.catalogue.missing_file_copies_min_age_secs 86400
####################################
# Variables used by cta-frontend-async-grpc
diff --git a/xroot_plugins/XrdCtaTapeLs.hpp b/xroot_plugins/XrdCtaTapeLs.hpp
index dfa5a9b5b4..4a5cedafef 100644
--- a/xroot_plugins/XrdCtaTapeLs.hpp
+++ b/xroot_plugins/XrdCtaTapeLs.hpp
@@ -81,6 +81,9 @@ TapeLsStream::TapeLsStream(const frontend::AdminCmdStream& requestMsg, cta::cata
searchCriteria.physicalLibraryName = requestMsg.getOptional(OptionString::PHYSICAL_LIBRARY, &has_any);
searchCriteria.diskFileIds = requestMsg.getOptional(OptionStrList::FILE_ID, &has_any);
searchCriteria.checkMissingFileCopies = requestMsg.getOptional(OptionBoolean::MISSING_FILE_COPIES, &has_any);
+ if (searchCriteria.checkMissingFileCopies.value_or(false)) {
+ searchCriteria.missingFileCopiesMinAgeSecs = requestMsg.getMissingFileCopiesMinAgeSecs();
+ }
auto stateOpt = requestMsg.getOptional(OptionString::STATE, &has_any);
if(stateOpt){
diff --git a/xroot_plugins/cta-frontend-xrootd.conf.example b/xroot_plugins/cta-frontend-xrootd.conf.example
index 1c2f62d647..71697bf240 100644
--- a/xroot_plugins/cta-frontend-xrootd.conf.example
+++ b/xroot_plugins/cta-frontend-xrootd.conf.example
@@ -29,6 +29,11 @@ cta.schedulerdb.threadstacksize_mb 1
# CTA Catalogue options
cta.catalogue.numberofconnections 10
+# Missing tape file copies minimum age.
+# Set it to prevent false flagging of multi-copy files in the process of being archived tapes.
+# Default 24 hours
+cta.catalogue.missing_file_copies_min_age_secs 86400
+
# Maximum file size (in GB) that the CTA Frontend will accept for archiving
cta.archivefile.max_size_gb 1000
--
GitLab
From 0b7e6937f7c4efe06b26863c938cf48e47a4bf8e Mon Sep 17 00:00:00 2001
From: Joao Afonso <joao.afonso@cern.ch>
Date: Fri, 28 Mar 2025 16:03:36 +0100
Subject: [PATCH 2/3] Minor fixes for unit-tests
---
catalogue/rdbms/RdbmsTapeCatalogue.cpp | 2 +-
catalogue/tests/modules/ArchiveFileCatalogueTest.cpp | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/catalogue/rdbms/RdbmsTapeCatalogue.cpp b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
index 39856975be..6117539fd0 100644
--- a/catalogue/rdbms/RdbmsTapeCatalogue.cpp
+++ b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
@@ -1443,7 +1443,7 @@ std::list<common::dataStructures::Tape> RdbmsTapeCatalogue::getTapes(rdbms::Conn
WHERE
SC.NB_COPIES > 1
GROUP BY
- AF.ARCHIVE_FILE_ID, SC.NB_COPIES AND AF.CREATION_TIME < :MAX_CREATION_TIME
+ AF.ARCHIVE_FILE_ID, SC.NB_COPIES AND AF.CREATION_TIME <= :MAX_CREATION_TIME
HAVING
SC.NB_COPIES <> COUNT(TF.ARCHIVE_FILE_ID)
) MISSING_COPIES
diff --git a/catalogue/tests/modules/ArchiveFileCatalogueTest.cpp b/catalogue/tests/modules/ArchiveFileCatalogueTest.cpp
index 7ad78e9e3b..bcf08dd2a8 100644
--- a/catalogue/tests/modules/ArchiveFileCatalogueTest.cpp
+++ b/catalogue/tests/modules/ArchiveFileCatalogueTest.cpp
@@ -5069,6 +5069,7 @@ TEST_P(cta_catalogue_ArchiveFileTest, getTapesWithMissingTapeFileCopies) {
{
cta::catalogue::TapeSearchCriteria searchCriteria;
searchCriteria.checkMissingFileCopies = true;
+ searchCriteria.missingFileCopiesMinAgeSecs = 0;
const std::list<cta::common::dataStructures::Tape> tapes = m_catalogue->Tape()->getTapes(searchCriteria);
ASSERT_EQ(2, tapes.size());
@@ -5110,6 +5111,7 @@ TEST_P(cta_catalogue_ArchiveFileTest, getTapesWithMissingTapeFileCopies) {
{
cta::catalogue::TapeSearchCriteria searchCriteria;
searchCriteria.checkMissingFileCopies = true;
+ searchCriteria.missingFileCopiesMinAgeSecs = 0;
const std::list<cta::common::dataStructures::Tape> tapes = m_catalogue->Tape()->getTapes(searchCriteria);
ASSERT_EQ(1, tapes.size());
@@ -5149,6 +5151,7 @@ TEST_P(cta_catalogue_ArchiveFileTest, getTapesWithMissingTapeFileCopies) {
{
cta::catalogue::TapeSearchCriteria searchCriteria;
searchCriteria.checkMissingFileCopies = true;
+ searchCriteria.missingFileCopiesMinAgeSecs = 0;
const std::list<cta::common::dataStructures::Tape> tapes = m_catalogue->Tape()->getTapes(searchCriteria);
ASSERT_TRUE(tapes.empty());
--
GitLab
From 63feee1c75ab138054c244ba9ca83ff9c53dfc3a Mon Sep 17 00:00:00 2001
From: Joao Afonso <joao.afonso@cern.ch>
Date: Fri, 28 Mar 2025 16:05:53 +0100
Subject: [PATCH 3/3] Review fixes
---
catalogue/rdbms/RdbmsTapeCatalogue.cpp | 4 ++--
frontend/common/FrontendService.cpp | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/catalogue/rdbms/RdbmsTapeCatalogue.cpp b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
index 6117539fd0..74863f636c 100644
--- a/catalogue/rdbms/RdbmsTapeCatalogue.cpp
+++ b/catalogue/rdbms/RdbmsTapeCatalogue.cpp
@@ -1441,9 +1441,9 @@ std::list<common::dataStructures::Tape> RdbmsTapeCatalogue::getTapes(rdbms::Conn
INNER JOIN STORAGE_CLASS SC ON AF.STORAGE_CLASS_ID = SC.STORAGE_CLASS_ID
INNER JOIN TAPE_FILE TF ON AF.ARCHIVE_FILE_ID = TF.ARCHIVE_FILE_ID
WHERE
- SC.NB_COPIES > 1
+ SC.NB_COPIES > 1 AND AF.CREATION_TIME <= :MAX_CREATION_TIME
GROUP BY
- AF.ARCHIVE_FILE_ID, SC.NB_COPIES AND AF.CREATION_TIME <= :MAX_CREATION_TIME
+ AF.ARCHIVE_FILE_ID, SC.NB_COPIES
HAVING
SC.NB_COPIES <> COUNT(TF.ARCHIVE_FILE_ID)
) MISSING_COPIES
diff --git a/frontend/common/FrontendService.cpp b/frontend/common/FrontendService.cpp
index 6b16f0a5c8..8b3043d03c 100644
--- a/frontend/common/FrontendService.cpp
+++ b/frontend/common/FrontendService.cpp
@@ -120,7 +120,7 @@ FrontendService::FrontendService(const std::string& configFilename) : m_archiveF
m_missingFileCopiesMinAgeSecs = missingFileCopiesMinAgeSecs.value_or(0);
{
- // Log cta.archivefile.max_size_gb
+ // Log cta.catalogue.missing_file_copies_min_age_secs
std::list<log::Param> params;
params.push_back(log::Param("source", missingFileCopiesMinAgeSecs.has_value() ? configFilename : "Compile time default"));
params.push_back(log::Param("category", "cta.catalogue"));
--
GitLab