From 2ca3ae102a386c84322b1e94dbc9f0cce0f077f9 Mon Sep 17 00:00:00 2001 From: Eric Cano <Eric.Cano@cern.ch> Date: Thu, 17 Mar 2016 17:13:12 +0100 Subject: [PATCH] Competed the configuration file support with all cta.conf parameters and integrated tpconfig support as well. Fixed exception losing its initial content after appending with getMessage() << --- common/exception/Exception.cpp | 2 +- .../daemon/FetchReportOrFlushLimits.hpp | 32 +++++++++++ tapeserver/daemon/SourcedParameter.cpp | 56 +++++++++++++++++-- tapeserver/daemon/SourcedParameter.hpp | 1 + tapeserver/daemon/TapedConfiguration.cpp | 15 +++++ tapeserver/daemon/TapedConfiguration.hpp | 37 +++++++++++- tapeserver/daemon/TapedConfigurationTests.cpp | 39 ++++++++++++- tapeserver/daemon/Tpconfig.cpp | 44 ++++++++++++++- tapeserver/daemon/Tpconfig.hpp | 8 ++- tapeserver/daemon/TpconfigTests.cpp | 39 ++++++++++--- 10 files changed, 250 insertions(+), 23 deletions(-) create mode 100644 tapeserver/daemon/FetchReportOrFlushLimits.hpp diff --git a/common/exception/Exception.cpp b/common/exception/Exception.cpp index d6fb17ed2c..f355a22e05 100644 --- a/common/exception/Exception.cpp +++ b/common/exception/Exception.cpp @@ -23,8 +23,8 @@ //------------------------------------------------------------------------------ cta::exception::Exception::Exception(const std::string &context, const bool embedBacktrace) : - m_message(context), m_backtrace(!embedBacktrace) { + m_message << context; } //------------------------------------------------------------------------------ diff --git a/tapeserver/daemon/FetchReportOrFlushLimits.hpp b/tapeserver/daemon/FetchReportOrFlushLimits.hpp new file mode 100644 index 0000000000..0ad099a05c --- /dev/null +++ b/tapeserver/daemon/FetchReportOrFlushLimits.hpp @@ -0,0 +1,32 @@ +/* + * The CERN Tape Archive (CTA) project + * Copyright (C) 2015 CERN + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#pragma once + +namespace cta { namespace tape { namespace daemon { + +/** The structure representing the maximum number of bytes and files + cta-taped will fetch or report in one access to the object store*/ +struct FetchReportOrFlushLimits { + uint64_t maxBytes; + uint64_t maxFiles; + FetchReportOrFlushLimits(): maxBytes(0), maxFiles(0) {} + FetchReportOrFlushLimits(uint64_t bytes, uint64_t files): maxBytes(bytes), maxFiles(files) {} +}; + +}}} // namespace cta::tape::daemon \ No newline at end of file diff --git a/tapeserver/daemon/SourcedParameter.cpp b/tapeserver/daemon/SourcedParameter.cpp index e10a4664cd..aecd7c3fa3 100644 --- a/tapeserver/daemon/SourcedParameter.cpp +++ b/tapeserver/daemon/SourcedParameter.cpp @@ -17,6 +17,8 @@ */ #include "tapeserver/daemon/SourcedParameter.hpp" +#include "tapeserver/daemon/FetchReportOrFlushLimits.hpp" +#include <algorithm> namespace cta { @@ -26,17 +28,63 @@ namespace daemon { template<> void SourcedParameter<time_t>::set(const std::string & value, const std::string & source) { if (!utils::isValidUInt(value)) { - std::stringstream err; - err << "In SourcedParameter::set: badly formatted integer" + BadlyFormattedInteger ex; + ex.getMessage() << "In SourcedParameter<time_t>::set() : badly formatted integer" << " for category=" << m_category << " key=" << m_key - << " value=\'" << value << "'"; - throw BadlyFormattedInteger(err.str()); + << " value=\'" << value << "' at:" << source; + throw ex; } std::istringstream(value) >> m_value; m_source = source; m_set = true; } +template<> +void SourcedParameter<uint64_t>::set(const std::string & value, const std::string & source) { + if (!utils::isValidUInt(value)) { + BadlyFormattedInteger ex; + ex.getMessage() << "In SourcedParameter<uint64_t>::set() : badly formatted integer" + << " for category=" << m_category << " key=" << m_key + << " value=\'" << value << "' at:" << source; + throw ex; + } + std::istringstream(value) >> m_value; + m_source = source; + m_set = true; +} + +template<> +void SourcedParameter<FetchReportOrFlushLimits>::set(const std::string & value, + const std::string & source) { + // We expect an entry in the form "<size limit>, <file limit>" + // There should be one and only one comma in the parameter. + if (1 != std::count(value.begin(), value.end(), ',')) { + BadlyFormattedSizeFileLimit ex; + ex.getMessage() << "In SourcedParameter<FetchReportOrFlushLimits>::set() : badly formatted entry: one (and only one) comma expected" + << " for category=" << m_category << " key=" << m_key + << " value=\'" << value << "' at:" << source; + throw ex; + } + // We can now split the entry + std::string bytes, files; + size_t commaPos=value.find(','); + bytes=value.substr(0, commaPos); + files=value.substr(commaPos+1); + bytes=utils::trimString(bytes); + files=utils::trimString(files); + if (!(utils::isValidUInt(bytes)&&utils::isValidUInt(files))) { + BadlyFormattedInteger ex; + ex.getMessage() << "In SourcedParameter<FetchReportOrFlushLimits>::set() : badly formatted integer" + << " for category=" << m_category << " key=" << m_key + << " value=\'" << value << "' at:" << source; + throw ex; + } + std::istringstream(bytes) >> m_value.maxBytes; + std::istringstream(files) >> m_value.maxFiles; + m_source = source; + m_set = true; +} + template<> void SourcedParameter<std::string>::set(const std::string & value, const std::string & source) { m_value = value; diff --git a/tapeserver/daemon/SourcedParameter.hpp b/tapeserver/daemon/SourcedParameter.hpp index 1874e8fe74..9313781eeb 100644 --- a/tapeserver/daemon/SourcedParameter.hpp +++ b/tapeserver/daemon/SourcedParameter.hpp @@ -38,6 +38,7 @@ public: CTA_GENERATE_EXCEPTION_CLASS(MandatoryParameterNotDefined); CTA_GENERATE_EXCEPTION_CLASS(UnsupportedParameterType); CTA_GENERATE_EXCEPTION_CLASS(BadlyFormattedInteger); + CTA_GENERATE_EXCEPTION_CLASS(BadlyFormattedSizeFileLimit); /// Constructor for mandatory options (they do not have default values) SourcedParameter(const std::string & category, const std::string & key): diff --git a/tapeserver/daemon/TapedConfiguration.cpp b/tapeserver/daemon/TapedConfiguration.cpp index 68026bfa20..513b5107f4 100644 --- a/tapeserver/daemon/TapedConfiguration.cpp +++ b/tapeserver/daemon/TapedConfiguration.cpp @@ -18,6 +18,7 @@ #include "TapedConfiguration.hpp" #include "ConfigurationFile.hpp" +#include "Tpconfig.hpp" namespace cta { namespace tape { namespace daemon { //------------------------------------------------------------------------------ @@ -36,13 +37,27 @@ TapedConfiguration TapedConfiguration::createFromCtaConf( // Parse config file ConfigurationFile cf(generalConfigPath); // Extract configuration from parsed config file + // TpConfig ret.tpConfigPath.setFromConfigurationFile(cf, generalConfigPath); + // Memory management + ret.bufferSize.setFromConfigurationFile(cf, generalConfigPath); + ret.bufferCount.setFromConfigurationFile(cf, generalConfigPath); + // Batched metadata access and tape write flush parameters + ret.archiveFetchBytesFiles.setFromConfigurationFile(cf, generalConfigPath); + ret.archiveFlushBytesFiles.setFromConfigurationFile(cf, generalConfigPath); + ret.retrieveFetchBytesFiles.setFromConfigurationFile(cf, generalConfigPath); + // Disk file access parameters + ret.nbDiskThreads.setFromConfigurationFile(cf, generalConfigPath); + // Watchdog: parameters for timeouts in various situations. ret.wdIdleSessionTimer.setFromConfigurationFile(cf, generalConfigPath); ret.wdMountMaxSecs.setFromConfigurationFile(cf, generalConfigPath); ret.wdNoBlockMoveMaxSecs.setFromConfigurationFile(cf, generalConfigPath); ret.wdScheduleMaxSecs.setFromConfigurationFile(cf, generalConfigPath); + // The central storage access configuration ret.objectStoreURL.setFromConfigurationFile(cf, generalConfigPath); ret.fileCatalogURL.setFromConfigurationFile(cf, generalConfigPath); + // Extract drive list from tpconfig + parsed config file + ret.driveConfigs = Tpconfig::parseFile(ret.tpConfigPath.value()); return ret; } diff --git a/tapeserver/daemon/TapedConfiguration.hpp b/tapeserver/daemon/TapedConfiguration.hpp index 5c05b88638..51697faa9c 100644 --- a/tapeserver/daemon/TapedConfiguration.hpp +++ b/tapeserver/daemon/TapedConfiguration.hpp @@ -21,10 +21,11 @@ #include <map> #include <type_traits> #include <limits> -#include "DriveConfiguration.hpp" #include "common/log/DummyLogger.hpp" #include "common/exception/Exception.hpp" #include "SourcedParameter.hpp" +#include "FetchReportOrFlushLimits.hpp" +#include "Tpconfig.hpp" namespace cta { namespace tape { @@ -47,9 +48,39 @@ struct TapedConfiguration { //---------------------------------------------------------------------------- /// Path to the file describing the tape drives (TPCONFIG) SourcedParameter<std::string> tpConfigPath = decltype(tpConfigPath) - ("taped" , "tpConfigPath", "/etc/cta/TPCONFIG", "Compile time default"); + ("taped" , "TpConfigPath", "/etc/cta/TPCONFIG", "Compile time default"); /// Extracted drives configuration. - std::map<std::string, DriveConfiguration> driveConfigs; + Tpconfig driveConfigs; + //---------------------------------------------------------------------------- + // Memory management + //---------------------------------------------------------------------------- + /// Memory buffer size (with a default of 5MB). TODO-switch to 32MB once validated in CASTOR. + SourcedParameter<uint64_t> bufferSize = decltype(bufferSize) + ("taped", "BufferSize", 5*1025*1024, "Compile time default"); + /// Memory buffer count per drive. There is no default to this one. + SourcedParameter<uint64_t> bufferCount = decltype(bufferCount) + ("taped", "BufferCount"); + //---------------------------------------------------------------------------- + // Batched metadata access and tape write flush parameters + //---------------------------------------------------------------------------- + /// The fetch size for archive requests + SourcedParameter<FetchReportOrFlushLimits> archiveFetchBytesFiles = + decltype(archiveFetchBytesFiles) + ("taped", "ArchiveFetchBytesFiles", {80L*1000*1000*1000, 500}, "Compile time default"); + /// The flush to tape criteria for archiving + SourcedParameter<FetchReportOrFlushLimits> archiveFlushBytesFiles = + decltype(archiveFlushBytesFiles) + ("taped", "ArchiveFlushBytesFiles", {32L*1000*1000*1000, 200}, "Compile time default"); + /// The fetch and report size for retrieve requests + SourcedParameter<FetchReportOrFlushLimits> retrieveFetchBytesFiles = + decltype(retrieveFetchBytesFiles) + ("taped", "RetrieveFetchBytesFiles", {80L*1000*1000*1000, 500}, "Compile time default"); + //---------------------------------------------------------------------------- + // Disk file access parameters + //---------------------------------------------------------------------------- + /// Number of disk threads. This is the number of parallel file transfers. + SourcedParameter<uint64_t> nbDiskThreads = decltype(nbDiskThreads) + ("taped", "NbDiskThreads", 10, "Compile time default"); //---------------------------------------------------------------------------- // Watchdog: parameters for timeouts in various situations. //---------------------------------------------------------------------------- diff --git a/tapeserver/daemon/TapedConfigurationTests.cpp b/tapeserver/daemon/TapedConfigurationTests.cpp index 9fdba64e06..fd6dbbfde8 100644 --- a/tapeserver/daemon/TapedConfigurationTests.cpp +++ b/tapeserver/daemon/TapedConfigurationTests.cpp @@ -33,15 +33,50 @@ TEST(cta_Daemon, TapedConfiguration) { "#A good enough configuration file for taped\n" "general ObjectStoreURL vfsObjectStore:///tmp/dir\n" "general FileCatalogURL sqliteFileCatalog:///tmp/dir2\n" - ); + "taped BufferCount 1\n" + "taped TpConfigPath "); + TempFile emptyTpConfig; + completeConfFile.stringAppend(emptyTpConfig.path()); ASSERT_THROW(cta::tape::daemon::TapedConfiguration::createFromCtaConf(incompleteConfFile.path()), - cta::tape::daemon::SourcedParameter<std::string>::MandatoryParameterNotDefined); + cta::tape::daemon::SourcedParameter<uint64_t>::MandatoryParameterNotDefined); + auto completeConfig = + cta::tape::daemon::TapedConfiguration::createFromCtaConf(completeConfFile.path()); + ASSERT_EQ(completeConfFile.path()+":2", completeConfig.objectStoreURL.source()); + ASSERT_EQ("vfsObjectStore:///tmp/dir", completeConfig.objectStoreURL.value()); + ASSERT_EQ(completeConfFile.path()+":3", completeConfig.fileCatalogURL.source()); + ASSERT_EQ("sqliteFileCatalog:///tmp/dir2", completeConfig.fileCatalogURL.value()); +} + +TEST(cta_Daemon, TapedConfigurationFull) { + TempFile completeConfFile; + completeConfFile.stringFill( + "#A good enough configuration file for taped\n" + "general ObjectStoreURL vfsObjectStore:///tmp/dir\n" + "general FileCatalogURL sqliteFileCatalog:///tmp/dir2\n" + "taped ArchiveFetchBytesFiles 1,2\n" + "taped ArchiveFlushBytesFiles 3 , 4 \n" + "taped RetrieveFetchBytesFiles 5, 6\n" + "taped BufferCount 1 \n" + "taped TpConfigPath "); + TempFile TpConfig; + TpConfig.stringFill("drive0 lib0 /dev/tape0 lib0slot0\n" + "drive1 lib0 /dev/tape1 lib0slot1\n" + "drive2 lib0 /dev/tape2 lib0slot2"); + completeConfFile.stringAppend(TpConfig.path()); auto completeConfig = cta::tape::daemon::TapedConfiguration::createFromCtaConf(completeConfFile.path()); ASSERT_EQ(completeConfFile.path()+":2", completeConfig.objectStoreURL.source()); ASSERT_EQ("vfsObjectStore:///tmp/dir", completeConfig.objectStoreURL.value()); ASSERT_EQ(completeConfFile.path()+":3", completeConfig.fileCatalogURL.source()); ASSERT_EQ("sqliteFileCatalog:///tmp/dir2", completeConfig.fileCatalogURL.value()); + ASSERT_EQ(1, completeConfig.archiveFetchBytesFiles.value().maxBytes); + ASSERT_EQ(2, completeConfig.archiveFetchBytesFiles.value().maxFiles); + ASSERT_EQ(3, completeConfig.archiveFlushBytesFiles.value().maxBytes); + ASSERT_EQ(4, completeConfig.archiveFlushBytesFiles.value().maxFiles); + ASSERT_EQ(5, completeConfig.retrieveFetchBytesFiles.value().maxBytes); + ASSERT_EQ(6, completeConfig.retrieveFetchBytesFiles.value().maxFiles); + ASSERT_EQ(3, completeConfig.driveConfigs.size()); + ASSERT_EQ("/dev/tape1", completeConfig.driveConfigs.at("drive1").value().devFilename); } } // namespace unitTests \ No newline at end of file diff --git a/tapeserver/daemon/Tpconfig.cpp b/tapeserver/daemon/Tpconfig.cpp index cdda65ea91..abfd9fdfbc 100644 --- a/tapeserver/daemon/Tpconfig.cpp +++ b/tapeserver/daemon/Tpconfig.cpp @@ -25,11 +25,12 @@ #include <fstream> #include <algorithm> +namespace cta { namespace tape { namespace daemon { + //------------------------------------------------------------------------------ // parseTpconfigFile //------------------------------------------------------------------------------ -cta::tape::daemon::Tpconfig -cta::tape::daemon::Tpconfig::parseFile(const std::string &filename) { +Tpconfig Tpconfig::parseFile(const std::string &filename) { Tpconfig ret; // Try to open the configuration file, throwing an exception if there is a // failure @@ -75,7 +76,44 @@ cta::tape::daemon::Tpconfig::parseFile(const std::string &filename) { // The constructor implicitly validates the lengths const TpconfigLine configLine(unitName, logicalLibarry, devFilePath, librarySlot); // Store the value of the data-columns in the output list parameter - ret.push_back(TpconfigLine(configLine)); + // Check for duplication + if (ret.count(configLine.unitName)) { + DuplicateEntry ex("In Tpconfig::parseFile(): duplicate entry for unit "); + ex.getMessage() << configLine.unitName << " at " << filename << ":" << lineNumber + << " previous at " << ret.at(configLine.unitName).source(); + throw ex; + } + // Check there is not duplicate of the path + { + auto i = std::find_if(ret.begin(), ret.end(), + [&](decltype(*ret.begin()) i) + {return i.second.value().devFilename == configLine.devFilename;}); + if (ret.end() != i) { + DuplicateEntry ex("In Tpconfig::parseFile(): duplicate path for unit "); + ex.getMessage() << configLine.unitName << " at " << filename << ":" << lineNumber + << " previous at " << i->second.source(); + throw ex; + } + } + // Check there is not duplicate of the library slot + { + auto i = std::find_if(ret.begin(), ret.end(), + [&](decltype(*ret.begin()) i) + {return i.second.value().librarySlot == configLine.librarySlot;}); + if (ret.end() != i) { + DuplicateEntry ex("In Tpconfig::parseFile(): duplicate library slot for unit "); + ex.getMessage() << configLine.unitName << " at " << filename << ":" << lineNumber + << " previous at " << i->second.source(); + throw ex; + } + } + // This is a re-use of the container for the cta.conf representation. + // There is no category nor key here. + std::stringstream linestr; + linestr << filename << ":" << lineNumber; + ret.emplace(configLine.unitName, decltype(ret.begin()->second)("", "", configLine, linestr.str())); } return ret; } + +}}} // namespace cta::tape::daemon \ No newline at end of file diff --git a/tapeserver/daemon/Tpconfig.hpp b/tapeserver/daemon/Tpconfig.hpp index ed497be9d9..885069cb81 100644 --- a/tapeserver/daemon/Tpconfig.hpp +++ b/tapeserver/daemon/Tpconfig.hpp @@ -19,19 +19,21 @@ #pragma once #include "tapeserver/daemon/TpconfigLine.hpp" +#include "tapeserver/daemon/SourcedParameter.hpp" #include "common/exception/Exception.hpp" -#include <list> +#include <map> namespace cta { namespace tape { namespace daemon { /** - * A list of lines parsed from a TPCONFIG file. + * A map of lines parsed from a TPCONFIG file (key is the drive name) */ -class Tpconfig: public std::list<TpconfigLine> { +class Tpconfig: public std::map<std::string, SourcedParameter<TpconfigLine>> { public: CTA_GENERATE_EXCEPTION_CLASS(InvalidArgument); + CTA_GENERATE_EXCEPTION_CLASS(DuplicateEntry); /** * Parses the specified TPCONFIG file. * diff --git a/tapeserver/daemon/TpconfigTests.cpp b/tapeserver/daemon/TpconfigTests.cpp index b79eea25ac..53395f7428 100644 --- a/tapeserver/daemon/TpconfigTests.cpp +++ b/tapeserver/daemon/TpconfigTests.cpp @@ -75,15 +75,40 @@ TEST(cta_Daemon, Tpconfig_base) { ASSERT_EQ(3, tpc.size()); int i=0; for(auto & t: tpc) { - ASSERT_EQ("drive", t.unitName.substr(0,5)); - ASSERT_EQ("lib0", t.logicalLibrary); - ASSERT_EQ("/dev/tape", t.devFilename.substr(0,9)); - ASSERT_EQ("lib0slot", t.librarySlot.substr(0,8)); - ASSERT_EQ('0'+i, t.unitName.back()); - ASSERT_EQ('0'+i, t.devFilename.back()); - ASSERT_EQ('0'+i, t.librarySlot.back()); + ASSERT_EQ("drive", t.second.value().unitName.substr(0,5)); + ASSERT_EQ("lib0", t.second.value().logicalLibrary); + ASSERT_EQ("/dev/tape", t.second.value().devFilename.substr(0,9)); + ASSERT_EQ("lib0slot", t.second.value().librarySlot.substr(0,8)); + ASSERT_EQ('0'+i, t.second.value().unitName.back()); + ASSERT_EQ('0'+i, t.second.value().devFilename.back()); + ASSERT_EQ('0'+i, t.second.value().librarySlot.back()); i++; } } +TEST(cta_Daemon, Tpconfig_duplicates) { + TempFile tf; + // Test duplicate unit name + tf.stringFill("drive0 lib0 /dev/tape0 lib0slot0\n" + "drive1 lib0 /dev/tape1 lib0slot1\n" + "drive0 lib0 /dev/tape2 lib0slot2"); + ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile(tf.path()), cta::exception::Exception); + // Test duplicate path + tf.stringFill("drive0 lib0 /dev/tape0 lib0slot0\n" + "drive1 lib0 /dev/tape1 lib0slot1\n" + "drive2 lib0 /dev/tape0 lib0slot2"); + ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile(tf.path()), cta::exception::Exception); + // Test duplicate slot + tf.stringFill("drive0 lib0 /dev/tape0 lib0slot0\n" + "drive1 lib0 /dev/tape1 lib0slot1\n" + "drive2 lib0 /dev/tape2 lib0slot0"); + ASSERT_THROW(cta::tape::daemon::Tpconfig::parseFile(tf.path()), cta::exception::Exception); + // No duplication. + // Test duplicate slot + tf.stringFill("drive0 lib0 /dev/tape0 lib0slot0\n" + "drive1 lib0 /dev/tape1 lib0slot1\n" + "drive2 lib0 /dev/tape2 lib0slot2"); + cta::tape::daemon::Tpconfig::parseFile(tf.path()); +} + } // namespace unitTests -- GitLab