From 316f9cbdc93526f9fa73b5e5ed6cd9cbd92248b2 Mon Sep 17 00:00:00 2001
From: Eric Cano <Eric.Cano@cern.ch>
Date: Thu, 13 Jun 2019 17:02:50 +0200
Subject: [PATCH] #471 Fixed various bugs.
---
catalogue/CatalogueTest.cpp | 116 --------------------------------
objectstore/DriveRegister.hpp | 2 +-
objectstore/DriveState.cpp | 37 +++++++++-
objectstore/DriveState.hpp | 6 ++
objectstore/GenericObject.cpp | 3 +
objectstore/Helpers.cpp | 2 +-
objectstore/RetrieveRequest.cpp | 4 +-
scheduler/OStoreDB/OStoreDB.cpp | 18 ++++-
scheduler/SchedulerDatabase.hpp | 4 +-
scheduler/SchedulerTest.cpp | 3 +-
10 files changed, 68 insertions(+), 127 deletions(-)
diff --git a/catalogue/CatalogueTest.cpp b/catalogue/CatalogueTest.cpp
index 1b01310b6c..e1989f6bd4 100644
--- a/catalogue/CatalogueTest.cpp
+++ b/catalogue/CatalogueTest.cpp
@@ -12151,122 +12151,6 @@ TEST_P(cta_catalogue_CatalogueTest, reclaimTape_full_lastFSeq_1_one_tape_file_su
}
}
-TEST_P(cta_catalogue_CatalogueTest, exist_non_superseded_files_after_fseq) {
- using namespace cta;
-
- const std::string diskInstanceName1 = "disk_instance_1";
-
- ASSERT_TRUE(m_catalogue->getTapes().empty());
-
- const std::string vid1 = "VID123";
- const std::string vid2 = "VID234";
- const std::string mediaType = "media_type";
- const std::string vendor = "vendor";
- const std::string logicalLibraryName = "logical_library_name";
- const bool logicalLibraryIsDisabled= false;
- const std::string tapePoolName = "tape_pool_name";
- const std::string vo = "vo";
- const uint64_t nbPartialTapes = 2;
- const bool isEncrypted = true;
- const cta::optional<std::string> supply("value for the supply pool mechanism");
- const uint64_t capacityInBytes = (uint64_t)10 * 1000 * 1000 * 1000 * 1000;
- const bool disabledValue = true;
- const bool fullValue = false;
- const std::string createTapeComment = "Create tape";
-
- m_catalogue->createLogicalLibrary(m_admin, logicalLibraryName, logicalLibraryIsDisabled, "Create logical library");
- m_catalogue->createTapePool(m_admin, tapePoolName, vo, nbPartialTapes, isEncrypted, supply, "Create tape pool");
- m_catalogue->createTape(m_admin, vid1, mediaType, vendor, logicalLibraryName, tapePoolName, capacityInBytes,
- disabledValue, fullValue, createTapeComment);
-
- //A tape with no tape file have no files after FSeq 0
- ASSERT_FALSE(m_catalogue->existNonSupersededFilesAfterFSeqAndDeleteTapeFilesForWriting(vid1,0));
-
- const uint64_t archiveFileId = 1234;
-
- ASSERT_FALSE(m_catalogue->getArchiveFilesItor().hasMore());
- ASSERT_THROW(m_catalogue->getArchiveFileById(archiveFileId), exception::Exception);
-
- common::dataStructures::StorageClass storageClass;
- storageClass.diskInstance = diskInstanceName1;
- storageClass.name = "storage_class";
- storageClass.nbCopies = 1;
- storageClass.comment = "Create storage class";
- m_catalogue->createStorageClass(m_admin, storageClass);
-
- /*
- * Insert a file in the tape vid1
- */
- {
- const uint64_t archiveFileSize = 1;
- const std::string tapeDrive = "tape_drive";
- const std::string checksumType = "checksum_type";
- const std::string checksumValue = "checksum_value";
-
- auto file1WrittenUP=cta::make_unique<cta::catalogue::TapeFileWritten>();
- auto & file1Written = *file1WrittenUP;
- std::set<cta::catalogue::TapeItemWrittenPointer> file1WrittenSet;
- file1WrittenSet.insert(file1WrittenUP.release());
- file1Written.archiveFileId = archiveFileId;
- file1Written.diskInstance = storageClass.diskInstance;
- file1Written.diskFileId = "5678";
- file1Written.diskFilePath = "/public_dir/public_file";
- file1Written.diskFileUser = "public_disk_user";
- file1Written.diskFileGroup = "public_disk_group";
- file1Written.size = archiveFileSize;
- file1Written.checksumType = checksumType;
- file1Written.checksumValue = checksumValue;
- file1Written.storageClassName = storageClass.name;
- file1Written.vid = vid1;
- file1Written.fSeq = 1;
- file1Written.blockId = 4321;
- file1Written.compressedSize = 1;
- file1Written.copyNb = 1;
- file1Written.tapeDrive = tapeDrive;
- m_catalogue->filesWrittenToTape(file1WrittenSet);
- }
- //One file written : this file is not superseded by another one, existNonSupersededFilesAfterFSeq = true
- ASSERT_TRUE(m_catalogue->existNonSupersededFilesAfterFSeqAndDeleteTapeFilesForWriting(vid1,0));
- //No file after the only file inserted, existNonSupersededFilesAfterFseq = false
- ASSERT_FALSE(m_catalogue->existNonSupersededFilesAfterFSeqAndDeleteTapeFilesForWriting(vid1,1));
-
- //Insert another file in another tape that will supersed the first one in vid1
- {
- m_catalogue->createTape(m_admin, vid2, mediaType, vendor, logicalLibraryName, tapePoolName, capacityInBytes,
- disabledValue, fullValue, createTapeComment);
- const uint64_t archiveFileSize = 1;
- const std::string tapeDrive = "tape_drive";
- const std::string checksumType = "checksum_type";
- const std::string checksumValue = "checksum_value";
-
- auto file1WrittenUP=cta::make_unique<cta::catalogue::TapeFileWritten>();
- auto & file1Written = *file1WrittenUP;
- std::set<cta::catalogue::TapeItemWrittenPointer> file1WrittenSet;
- file1WrittenSet.insert(file1WrittenUP.release());
- file1Written.archiveFileId = archiveFileId;
- file1Written.diskInstance = storageClass.diskInstance;
- file1Written.diskFileId = "5678";
- file1Written.diskFilePath = "/public_dir/public_file";
- file1Written.diskFileUser = "public_disk_user";
- file1Written.diskFileGroup = "public_disk_group";
- file1Written.size = archiveFileSize;
- file1Written.checksumType = checksumType;
- file1Written.checksumValue = checksumValue;
- file1Written.storageClassName = storageClass.name;
- file1Written.vid = vid2;
- file1Written.fSeq = 1;
- file1Written.blockId = 4321;
- file1Written.compressedSize = 1;
- file1Written.copyNb = 1;
- file1Written.tapeDrive = tapeDrive;
- m_catalogue->filesWrittenToTape(file1WrittenSet);
- }
- //The tape files written to tape vid2 are not superseded by any file, but the tape files in vid1
- //are superseded by the tape files in vid2
- ASSERT_FALSE(m_catalogue->existNonSupersededFilesAfterFSeqAndDeleteTapeFilesForWriting(vid1,0));
- ASSERT_TRUE(m_catalogue->existNonSupersededFilesAfterFSeqAndDeleteTapeFilesForWriting(vid2,0));
-}
-
TEST_P(cta_catalogue_CatalogueTest, createModifyDeleteActivityWeight) {
using namespace cta;
diff --git a/objectstore/DriveRegister.hpp b/objectstore/DriveRegister.hpp
index 1754d81e1f..58cbe002e8 100644
--- a/objectstore/DriveRegister.hpp
+++ b/objectstore/DriveRegister.hpp
@@ -77,7 +77,7 @@ public:
void removeDrive(const std::string & driveName);
/**
- * JSON dump of the drive
+ * JSON dump of the drive register
* @return
*/
std::string dump();
diff --git a/objectstore/DriveState.cpp b/objectstore/DriveState.cpp
index fa47f28e77..634d9c71ae 100644
--- a/objectstore/DriveState.cpp
+++ b/objectstore/DriveState.cpp
@@ -18,9 +18,13 @@
#include "DriveState.hpp"
#include "GenericObject.hpp"
+#include <google/protobuf/util/json_util.h>
namespace cta { namespace objectstore {
+//------------------------------------------------------------------------------
+// DriveState::DriveState())
+//------------------------------------------------------------------------------
DriveState::DriveState(GenericObject& go):
ObjectOps<serializers::DriveState, serializers::DriveState_t>(go.objectStore()) {
// Here we transplant the generic object into the new object
@@ -29,6 +33,9 @@ ObjectOps<serializers::DriveState, serializers::DriveState_t>(go.objectStore())
getPayloadFromHeader();
}
+//------------------------------------------------------------------------------
+// DriveState::garbageCollect())
+//------------------------------------------------------------------------------
void DriveState::garbageCollect(const std::string& presumedOwner, AgentReference& agentReference, log::LogContext& lc, cta::catalogue::Catalogue& catalogue) {
// The drive state is easily replaceable. We just delete it on garbage collection.
checkPayloadWritable();
@@ -40,6 +47,9 @@ void DriveState::garbageCollect(const std::string& presumedOwner, AgentReference
lc.log(log::INFO, "In DriveState::garbageCollect(): Garbage collected and removed drive state object.");
}
+//------------------------------------------------------------------------------
+// DriveState::initialize())
+//------------------------------------------------------------------------------
void DriveState::initialize(const std::string & driveName) {
// Setup underlying object with defaults from dataStructures::DriveState
ObjectOps<serializers::DriveState, serializers::DriveState_t>::initialize();
@@ -52,14 +62,21 @@ void DriveState::initialize(const std::string & driveName) {
m_payloadInterpreted = true;
}
-
+//------------------------------------------------------------------------------
+// DriveState::DriveState())
+//------------------------------------------------------------------------------
DriveState::DriveState(const std::string& address, Backend& os):
ObjectOps<serializers::DriveState, serializers::DriveState_t>(os, address) { }
+//------------------------------------------------------------------------------
+// DriveState::DriveState())
+//------------------------------------------------------------------------------
DriveState::DriveState(Backend& os):
ObjectOps<serializers::DriveState, serializers::DriveState_t>(os) { }
-
+//------------------------------------------------------------------------------
+// DriveState::getState())
+//------------------------------------------------------------------------------
cta::common::dataStructures::DriveState DriveState::getState() {
cta::common::dataStructures::DriveState ret;
ret.driveName = m_payload.drivename();
@@ -107,6 +124,9 @@ cta::common::dataStructures::DriveState DriveState::getState() {
return ret;
}
+//------------------------------------------------------------------------------
+// DriveState::setState())
+//------------------------------------------------------------------------------
void DriveState::setState(cta::common::dataStructures::DriveState& state) {
// There should be no need to set the drive name.
m_payload.set_host(state.host);
@@ -154,6 +174,19 @@ void DriveState::setState(cta::common::dataStructures::DriveState& state) {
}
}
+//------------------------------------------------------------------------------
+// DriveState::dump())
+//------------------------------------------------------------------------------
+std::string DriveState::dump() {
+ checkPayloadReadable();
+ google::protobuf::util::JsonPrintOptions options;
+ options.add_whitespace = true;
+ options.always_print_primitive_fields = true;
+ std::string headerDump;
+ google::protobuf::util::MessageToJsonString(m_payload, &headerDump, options);
+ return headerDump;
+}
+
}} // namespace cta::objectstore
diff --git a/objectstore/DriveState.hpp b/objectstore/DriveState.hpp
index 83fb27263b..4c7d8da089 100644
--- a/objectstore/DriveState.hpp
+++ b/objectstore/DriveState.hpp
@@ -45,6 +45,12 @@ public:
// Data access
cta::common::dataStructures::DriveState getState();
void setState(cta::common::dataStructures::DriveState & state);
+
+ /**
+ * JSON dump of the drive state
+ * @return
+ */
+ std::string dump();
};
}} // namespace cta::objectstore
\ No newline at end of file
diff --git a/objectstore/GenericObject.cpp b/objectstore/GenericObject.cpp
index b0cd9a98d0..c0f6806a2f 100644
--- a/objectstore/GenericObject.cpp
+++ b/objectstore/GenericObject.cpp
@@ -187,6 +187,9 @@ std::string GenericObject::dump() {
case serializers::DriveRegister_t:
bodyDump = dumpWithType<DriveRegister>(this);
break;
+ case serializers::DriveState_t:
+ bodyDump = dumpWithType<DriveState>(this);
+ break;
case serializers::ArchiveQueue_t:
bodyDump = dumpWithType<cta::objectstore::ArchiveQueue>(this);
break;
diff --git a/objectstore/Helpers.cpp b/objectstore/Helpers.cpp
index 6eff98a143..a8d1489f67 100644
--- a/objectstore/Helpers.cpp
+++ b/objectstore/Helpers.cpp
@@ -613,7 +613,7 @@ void Helpers::getLockedAndFetchedDriveState(DriveState& driveState, ScopedExclus
}
} catch (DriveRegister::NoSuchDrive &) {
// OK, we do need to create the drive status.
- driveState.setAddress(agentReference.nextId(std::string ("DriveStatus-")+driveName));
+ driveState.setAddress(agentReference.nextId(std::string ("DriveState-")+driveName));
driveState.initialize(driveName);
agentReference.addToOwnership(driveState.getAddressIfSet(), be);
driveState.setOwner(agentReference.getAgentAddress());
diff --git a/objectstore/RetrieveRequest.cpp b/objectstore/RetrieveRequest.cpp
index 14efe9a7cd..f1a5be8b1f 100644
--- a/objectstore/RetrieveRequest.cpp
+++ b/objectstore/RetrieveRequest.cpp
@@ -475,9 +475,9 @@ optional<RetrieveActivityDescription> RetrieveRequest::getActivity() {
if (m_payload.has_activity_weight()) {
RetrieveActivityDescription activity;
activity.priority = m_payload.activity_weight().priority();
- activity.diskInstanceName = m_payload.activity_weight().activity();
+ activity.diskInstanceName = m_payload.activity_weight().disk_instance_name();
activity.activity = m_payload.activity_weight().activity();
- activity.priority = m_payload.activity_weight().weight();
+ activity.weight = m_payload.activity_weight().weight();
activity.creationTime = m_payload.activity_weight().creation_time();
ret = activity;
}
diff --git a/scheduler/OStoreDB/OStoreDB.cpp b/scheduler/OStoreDB/OStoreDB.cpp
index 15d1a11198..15f98fa664 100644
--- a/scheduler/OStoreDB/OStoreDB.cpp
+++ b/scheduler/OStoreDB/OStoreDB.cpp
@@ -394,6 +394,8 @@ void OStoreDB::fetchMountInfo(SchedulerDatabase::TapeMountDecisionInfo& tmdi, Ro
tmdi.existingOrNextMounts.back().bytesTransferred = d.bytesTransferredInSession;
tmdi.existingOrNextMounts.back().filesTransferred = d.filesTransferredInSession;
tmdi.existingOrNextMounts.back().latestBandwidth = d.latestBandwidth;
+ if (d.currentActivityAndWeight)
+ tmdi.existingOrNextMounts.back().activity = d.currentActivityAndWeight.value().activity;
}
if (activeMountTypes.count((int)d.nextMountType)) {
tmdi.existingOrNextMounts.push_back(ExistingMount());
@@ -405,6 +407,8 @@ void OStoreDB::fetchMountInfo(SchedulerDatabase::TapeMountDecisionInfo& tmdi, Ro
tmdi.existingOrNextMounts.back().bytesTransferred = 0;
tmdi.existingOrNextMounts.back().filesTransferred = 0;
tmdi.existingOrNextMounts.back().latestBandwidth = 0;
+ if (d.nextActivityAndWeight)
+ tmdi.existingOrNextMounts.back().activity = d.currentActivityAndWeight.value().activity;
}
}
auto registerProcessingTime = t.secs(utils::Timer::resetCounter);
@@ -2808,6 +2812,7 @@ void OStoreDB::setDriveDown(common::dataStructures::DriveState & driveState,
driveState.desiredDriveState.forceDown=false;
driveState.currentVid="";
driveState.currentTapePool="";
+ driveState.currentActivityAndWeight = nullopt;
}
//------------------------------------------------------------------------------
@@ -2846,6 +2851,7 @@ void OStoreDB::setDriveUpOrMaybeDown(common::dataStructures::DriveState & driveS
driveState.driveStatus=targetStatus;
driveState.currentVid="";
driveState.currentTapePool="";
+ driveState.currentActivityAndWeight = nullopt;
}
//------------------------------------------------------------------------------
@@ -2879,6 +2885,7 @@ void OStoreDB::setDriveProbing(common::dataStructures::DriveState & driveState,
driveState.driveStatus=inputs.status;
driveState.currentVid="";
driveState.currentTapePool="";
+ driveState.currentActivityAndWeight = nullopt;
}
//------------------------------------------------------------------------------
@@ -2891,8 +2898,7 @@ void OStoreDB::setDriveStarting(common::dataStructures::DriveState & driveState,
driveState.lastUpdateTime = inputs.reportTime;
return;
}
- // If we are changing state, then all should be reset. We are not supposed to
- // know the direction yet.
+ // If we are changing state, then all should be reset.
driveState.sessionId=inputs.mountSessionId;
driveState.bytesTransferredInSession=0;
driveState.filesTransferredInSession=0;
@@ -2913,6 +2919,12 @@ void OStoreDB::setDriveStarting(common::dataStructures::DriveState & driveState,
driveState.driveStatus=common::dataStructures::DriveStatus::Starting;
driveState.currentVid=inputs.vid;
driveState.currentTapePool=inputs.tapepool;
+ if (inputs.activityAndWeigh) {
+ common::dataStructures::DriveState::ActivityAndWeight aaw;
+ aaw.activity = inputs.activityAndWeigh.value().activity;
+ aaw.weight = inputs.activityAndWeigh.value().weight;
+ driveState.currentActivityAndWeight = aaw;
+ }
}
//------------------------------------------------------------------------------
@@ -3108,6 +3120,7 @@ void OStoreDB::setDriveCleaningUp(common::dataStructures::DriveState & driveStat
driveState.driveStatus=common::dataStructures::DriveStatus::CleaningUp;
driveState.currentVid=inputs.vid;
driveState.currentTapePool=inputs.tapepool;
+ driveState.currentActivityAndWeight = nullopt;
}
//------------------------------------------------------------------------------
@@ -3140,6 +3153,7 @@ void OStoreDB::setDriveShutdown(common::dataStructures::DriveState & driveState,
driveState.driveStatus=common::dataStructures::DriveStatus::CleaningUp;
driveState.currentVid=inputs.vid;
driveState.currentTapePool=inputs.tapepool;
+ driveState.currentActivityAndWeight = nullopt;
}
//------------------------------------------------------------------------------
// OStoreDB::TapeMountDecisionInfo::createArchiveMount()
diff --git a/scheduler/SchedulerDatabase.hpp b/scheduler/SchedulerDatabase.hpp
index 9b95e00dac..414b3ee460 100644
--- a/scheduler/SchedulerDatabase.hpp
+++ b/scheduler/SchedulerDatabase.hpp
@@ -575,9 +575,9 @@ public:
// the tapepool. So for different tape pools, we do not order. Likewise, both mounts should have an activity to
// be comparable
if (activityNameAndWeightedMountCount && other.activityNameAndWeightedMountCount && tapePool == other.tapePool) {
- if (activityNameAndWeightedMountCount.value().weightedMountCount > other.activityNameAndWeightedMountCount.value().weight)
+ if (activityNameAndWeightedMountCount.value().weightedMountCount > other.activityNameAndWeightedMountCount.value().weightedMountCount)
return true;
- if (activityNameAndWeightedMountCount.value().weightedMountCount < other.activityNameAndWeightedMountCount.value().weight)
+ if (activityNameAndWeightedMountCount.value().weightedMountCount < other.activityNameAndWeightedMountCount.value().weightedMountCount)
return false;
}
if(minRequestAge < other.minRequestAge)
diff --git a/scheduler/SchedulerTest.cpp b/scheduler/SchedulerTest.cpp
index ddd44dff77..64d185f510 100644
--- a/scheduler/SchedulerTest.cpp
+++ b/scheduler/SchedulerTest.cpp
@@ -162,7 +162,7 @@ public:
const uint64_t minArchiveRequestAge = 2;
const uint64_t retrievePriority = 3;
const uint64_t minRetrieveRequestAge = 4;
- const uint64_t maxDrivesAllowed = 5;
+ const uint64_t maxDrivesAllowed = 50;
const std::string mountPolicyComment = "create mount group";
ASSERT_TRUE(catalogue.getMountPolicies().empty());
@@ -2846,6 +2846,7 @@ TEST_P(SchedulerTest, archiveReportMultipleAndQueueRetrievesWithActivities) {
ASSERT_NE(nullptr, mount.get());
ASSERT_EQ(cta::common::dataStructures::MountType::Retrieve, mount.get()->getMountType());
ASSERT_TRUE((bool)mount.get()->getActivity());
+ std::cout << i << ": " << mount.get()->getActivity().value() << std::endl;
if (ea != Unknown) {
std::string expectedActivity(ea==A?"A":"B"), activity(mount.get()->getActivity().value());
ASSERT_EQ(expectedActivity, activity);
--
GitLab