From a0d7ba2366c307cf2ccffdf50e63b96781cc5daa Mon Sep 17 00:00:00 2001
From: mvelosob <miguel.veloso.barros@cern.ch>
Date: Fri, 12 Nov 2021 11:04:49 +0100
Subject: [PATCH] cta-admin tf rm stores the diskFilePath in the recycle bin
(#1062)
---
catalogue/Catalogue.hpp | 11 ++++--
catalogue/CatalogueRetryWrapper.hpp | 8 +++--
catalogue/CatalogueTest.cpp | 42 +++++++++++++----------
catalogue/DummyCatalogue.hpp | 3 +-
catalogue/RdbmsCatalogue.cpp | 19 +++++++---
catalogue/RdbmsCatalogue.hpp | 13 +++++--
xroot_plugins/XrdSsiCtaRequestMessage.cpp | 10 ++++--
7 files changed, 73 insertions(+), 33 deletions(-)
diff --git a/catalogue/Catalogue.hpp b/catalogue/Catalogue.hpp
index 42af652fe5..71bc1c64e3 100644
--- a/catalogue/Catalogue.hpp
+++ b/catalogue/Catalogue.hpp
@@ -905,6 +905,13 @@ public:
virtual ArchiveFileItor getArchiveFilesItor(
const TapeFileSearchCriteria &searchCriteria = TapeFileSearchCriteria()) const = 0;
+ /**
+ * Returns the specified archive file. If the search criteria result in more than one tape file being returned
+ * an exception is thrown.
+ * @param searchCriteria The search criteria.
+ * @return The archive file.
+ */
+ virtual common::dataStructures::ArchiveFile getArchiveFileForDeletion(const TapeFileSearchCriteria &searchCriteria = TapeFileSearchCriteria()) const = 0;
typedef CatalogueItor<common::dataStructures::FileRecycleLog> FileRecycleLogItor;
@@ -965,10 +972,10 @@ public:
/**
* Deletes a tape file copy
*
- * @param criteria The search criteria of the tape file to delete
+ * @param file The tape file to delete
* @param reason The reason for deleting the tape file copy
*/
- virtual void deleteTapeFileCopy(const TapeFileSearchCriteria &criteria, const std::string &reason) = 0;
+ virtual void deleteTapeFileCopy(common::dataStructures::ArchiveFile &file, const std::string &reason) = 0;
/**
* Returns the archive file with the specified unique identifier.
diff --git a/catalogue/CatalogueRetryWrapper.hpp b/catalogue/CatalogueRetryWrapper.hpp
index 50120a93e3..012decde67 100644
--- a/catalogue/CatalogueRetryWrapper.hpp
+++ b/catalogue/CatalogueRetryWrapper.hpp
@@ -574,8 +574,12 @@ public:
return retryOnLostConnection(m_log, [&]{return m_catalogue->getTapeFileSummary(searchCriteria);}, m_maxTriesToConnect);
}
- void deleteTapeFileCopy(const TapeFileSearchCriteria &criteria, const std::string &reason) override {
- return retryOnLostConnection(m_log, [&]{return m_catalogue->deleteTapeFileCopy(criteria, reason);}, m_maxTriesToConnect);
+ common::dataStructures::ArchiveFile getArchiveFileForDeletion(const TapeFileSearchCriteria &searchCriteria = TapeFileSearchCriteria()) const override {
+ return retryOnLostConnection(m_log, [&]{return m_catalogue->getArchiveFileForDeletion(searchCriteria);}, m_maxTriesToConnect);
+ }
+
+ void deleteTapeFileCopy(common::dataStructures::ArchiveFile &file, const std::string &reason) override {
+ return retryOnLostConnection(m_log, [&]{return m_catalogue->deleteTapeFileCopy(file, reason);}, m_maxTriesToConnect);
}
common::dataStructures::ArchiveFile getArchiveFileById(const uint64_t id) const override {
diff --git a/catalogue/CatalogueTest.cpp b/catalogue/CatalogueTest.cpp
index f60e294cd3..4d96b5ae1a 100644
--- a/catalogue/CatalogueTest.cpp
+++ b/catalogue/CatalogueTest.cpp
@@ -16384,7 +16384,9 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyUsingArchiveID) {
cta::catalogue::TapeFileSearchCriteria criteria;
criteria.vid = tape1.vid;
criteria.archiveFileId = 1;
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(1, archiveFile.tapeFiles.size());
auto fileRecycleLogItor = m_catalogue->getFileRecycleLogItor();
@@ -16397,16 +16399,15 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyUsingArchiveID) {
ASSERT_EQ(1, recycleFileLog.copyNb);
ASSERT_EQ(1 * 100, recycleFileLog.blockId);
ASSERT_EQ("(Deleted using cta-admin tf rm) " + reason, recycleFileLog.reasonLog);
- ASSERT_EQ(std::string("Not applicable for copies deleted with cta-admin tf rm"), recycleFileLog.diskFilePath.value());
+ ASSERT_EQ(std::string("/test/file1"), recycleFileLog.diskFilePath.value());
}
{
- //delete last copy of file should fail
+ //get last archive file copy for deletions should fail
cta::catalogue::TapeFileSearchCriteria criteria;
criteria.vid = tape2.vid;
criteria.archiveFileId = 1;
- ASSERT_THROW(m_catalogue->deleteTapeFileCopy(criteria, reason), exception::UserError);
-
+ ASSERT_THROW(m_catalogue->getArchiveFileForDeletion(criteria), exception::UserError);
}
}
@@ -16443,8 +16444,7 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyDoesNotExist) {
cta::catalogue::TapeFileSearchCriteria criteria;
criteria.vid = tape2.vid;
criteria.archiveFileId = 1;
- ASSERT_THROW(m_catalogue->deleteTapeFileCopy(criteria, reason), exception::UserError);
-
+ ASSERT_THROW(m_catalogue->getArchiveFileForDeletion(criteria), exception::UserError);
}
}
@@ -16555,7 +16555,9 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyUsingFXID) {
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(1, archiveFile.tapeFiles.size());
auto fileRecycleLogItor = m_catalogue->getFileRecycleLogItor();
@@ -16568,7 +16570,7 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyUsingFXID) {
ASSERT_EQ(1, recycleFileLog.copyNb);
ASSERT_EQ(1 * 100, recycleFileLog.blockId);
ASSERT_EQ("(Deleted using cta-admin tf rm) " + reason, recycleFileLog.reasonLog);
- ASSERT_EQ(std::string("Not applicable for copies deleted with cta-admin tf rm"), recycleFileLog.diskFilePath.value());
+ ASSERT_EQ(std::string("/test/file1"), recycleFileLog.diskFilePath.value());
}
{
@@ -16579,7 +16581,7 @@ TEST_P(cta_catalogue_CatalogueTest, DeleteTapeFileCopyUsingFXID) {
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- ASSERT_THROW(m_catalogue->deleteTapeFileCopy(criteria, reason), exception::UserError);
+ ASSERT_THROW(m_catalogue->getArchiveFileForDeletion(criteria), exception::UserError);
}
}
@@ -16690,7 +16692,9 @@ TEST_P(cta_catalogue_CatalogueTest, RestoreTapeFileCopy) {
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(1, archiveFile.tapeFiles.size());
}
@@ -16823,7 +16827,9 @@ TEST_P(cta_catalogue_CatalogueTest, RestoreRewrittenTapeFileCopyFails) {
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(1, archiveFile.tapeFiles.size());
}
@@ -17013,8 +17019,6 @@ TEST_P(cta_catalogue_CatalogueTest, RestoreVariousDeletedTapeFileCopies) {
}
{
- //delete copy of file on tape1
-
//delete copy of file on tape1
cta::catalogue::TapeFileSearchCriteria criteria;
criteria.vid = tape1.vid;
@@ -17022,22 +17026,24 @@ TEST_P(cta_catalogue_CatalogueTest, RestoreVariousDeletedTapeFileCopies) {
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(2, archiveFile.tapeFiles.size());
}
{
//delete copy of file on tape2
-
- //delete copy of file on tape1
cta::catalogue::TapeFileSearchCriteria criteria;
criteria.vid = tape2.vid;
criteria.diskInstance = diskInstance;
criteria.diskFileIds = std::vector<std::string>();
auto fid = std::to_string(strtol("BC614D", nullptr, 16));
criteria.diskFileIds.value().push_back(fid);
- m_catalogue->deleteTapeFileCopy(criteria, reason);
+ auto archiveFileForDeletion = m_catalogue->getArchiveFileForDeletion(criteria);
+ archiveFileForDeletion.diskFileInfo.path = "/test/file1";
+ m_catalogue->deleteTapeFileCopy(archiveFileForDeletion, reason);
auto archiveFile = m_catalogue->getArchiveFileById(1);
ASSERT_EQ(1, archiveFile.tapeFiles.size());
}
diff --git a/catalogue/DummyCatalogue.hpp b/catalogue/DummyCatalogue.hpp
index 9f7a1d2d17..592aa35db4 100644
--- a/catalogue/DummyCatalogue.hpp
+++ b/catalogue/DummyCatalogue.hpp
@@ -105,7 +105,8 @@ public:
std::list<common::dataStructures::StorageClass> getStorageClasses() const { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
common::dataStructures::StorageClass getStorageClass(const std::string &name) const { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
common::dataStructures::ArchiveFileSummary getTapeFileSummary(const TapeFileSearchCriteria& searchCriteria) const { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
- void deleteTapeFileCopy(const TapeFileSearchCriteria &criteria, const std::string &reason) override {throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented");}
+ common::dataStructures::ArchiveFile getArchiveFileForDeletion(const TapeFileSearchCriteria &searchCriteria = TapeFileSearchCriteria()) const override {throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented");}
+ void deleteTapeFileCopy(common::dataStructures::ArchiveFile &file, const std::string &reason) override {throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented");}
std::list<TapePool> getTapePools(const TapePoolSearchCriteria &searchCriteria) const { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
cta::optional<TapePool> getTapePool(const std::string &tapePoolName) const override { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
std::list<common::dataStructures::Tape> getTapes(const TapeSearchCriteria& searchCriteria) const { throw exception::Exception(std::string("In ")+__PRETTY_FUNCTION__+": not implemented"); }
diff --git a/catalogue/RdbmsCatalogue.cpp b/catalogue/RdbmsCatalogue.cpp
index 81e629b638..c6cbbc9ee5 100644
--- a/catalogue/RdbmsCatalogue.cpp
+++ b/catalogue/RdbmsCatalogue.cpp
@@ -7381,16 +7381,18 @@ common::dataStructures::ArchiveFileSummary RdbmsCatalogue::getTapeFileSummary(
}
//------------------------------------------------------------------------------
-// deleteTapeFileCopy
+// getArchiveFileForDeletion
//------------------------------------------------------------------------------
-void RdbmsCatalogue::deleteTapeFileCopy(const TapeFileSearchCriteria &criteria, const std::string &reason) {
-
+common::dataStructures::ArchiveFile RdbmsCatalogue::getArchiveFileForDeletion(const TapeFileSearchCriteria &criteria) const {
if (!criteria.diskFileIds && !criteria.archiveFileId) {
throw exception::UserError("To delete a file copy either the diskFileId+diskInstanceName or archiveFileId must be specified");
}
if (criteria.diskFileIds && !criteria.diskInstance) {
throw exception::UserError("DiskFileId makes no sense without disk instance");
}
+ if (!criteria.vid) {
+ throw exception::UserError("Vid must be specified");
+ }
auto vid = criteria.vid.value();
TapeFileSearchCriteria searchCriteria = criteria;
@@ -7446,11 +7448,18 @@ void RdbmsCatalogue::deleteTapeFileCopy(const TapeFileSearchCriteria &criteria,
criteria.diskInstance.value() + " on vid " + vid);
}
}
+ return af;
+}
+
+
+//------------------------------------------------------------------------------
+// deleteTapeFileCopy
+//------------------------------------------------------------------------------
+void RdbmsCatalogue::deleteTapeFileCopy(common::dataStructures::ArchiveFile &file, const std::string &reason) {
log::LogContext lc(m_log);
auto conn = m_connPool.getConn();
- af.diskFileInfo.path = "Not applicable for copies deleted with cta-admin tf rm"; // will go into the diskFilePath column of the File Recycle log
- copyTapeFileToFileRecyleLogAndDelete(conn, af, reason, lc);
+ copyTapeFileToFileRecyleLogAndDelete(conn, file, reason, lc);
}
diff --git a/catalogue/RdbmsCatalogue.hpp b/catalogue/RdbmsCatalogue.hpp
index 663c4338e8..e72b0382a9 100644
--- a/catalogue/RdbmsCatalogue.hpp
+++ b/catalogue/RdbmsCatalogue.hpp
@@ -941,14 +941,21 @@ public:
common::dataStructures::ArchiveFileSummary getTapeFileSummary(
const TapeFileSearchCriteria &searchCriteria) const override;
+ /**
+ * Returns the specified archive file. If the search criteria result in more than one tape file being returned
+ * an exception is thrown.
+ * @param searchCriteria The search criteria.
+ * @return The archive file.
+ */
+ common::dataStructures::ArchiveFile getArchiveFileForDeletion(const TapeFileSearchCriteria &searchCriteria = TapeFileSearchCriteria()) const override;
+
/**
* Deletes a tape file copy
*
- * @param criteria The search criteria of the tape file to delete
+ * @param file The tape file to delete
* @param reason The reason for deleting the tape file copy
*/
- void deleteTapeFileCopy(const TapeFileSearchCriteria &criteria, const std::string &reason) override;
-
+ void deleteTapeFileCopy(common::dataStructures::ArchiveFile &file, const std::string &reason) override;
/**
* Returns the archive file with the specified unique identifier.
diff --git a/xroot_plugins/XrdSsiCtaRequestMessage.cpp b/xroot_plugins/XrdSsiCtaRequestMessage.cpp
index 2a001f4f2d..3329cba3c0 100644
--- a/xroot_plugins/XrdSsiCtaRequestMessage.cpp
+++ b/xroot_plugins/XrdSsiCtaRequestMessage.cpp
@@ -1913,8 +1913,14 @@ void RequestMessage::processTapeFile_Rm(cta::xrd::Response &response)
if (instance) {
searchCriteria.diskInstance = instance.value();
}
- m_catalogue.deleteTapeFileCopy(searchCriteria, reason);
- response.set_type(cta::xrd::Response::RSP_SUCCESS);
+
+ auto archiveFile = m_catalogue.getArchiveFileForDeletion(searchCriteria);
+ grpc::EndpointMap endpoints(m_namespaceMap);
+ auto diskFilePath = endpoints.getPath(archiveFile.diskInstance, archiveFile.diskFileId);
+ archiveFile.diskFileInfo.path = diskFilePath;
+
+ m_catalogue.deleteTapeFileCopy(archiveFile, reason);
+ response.set_type(cta::xrd::Response::RSP_SUCCESS);
}
--
GitLab