diff --git a/CMakeLists.txt b/CMakeLists.txt index 0585b7fb2..fbc8de420 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -6,10 +6,15 @@ project( fdb5 LANGUAGES C CXX ) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -# set(CMAKE_COMPILE_WARNING_AS_ERROR ON) + # add_compile_options(-fsanitize=address) # add_compile_options(-fsanitize=address,undefined -fno-omit-frame-pointer) # add_link_options(-fsanitize=address) + +# add_compile_options(-fsanitize=thread) +# add_compile_options(-fsanitize=thread,undefined -fno-omit-frame-pointer) +# add_link_options(-fsanitize=thread) + # set(CMAKE_CXX_FLAGS "-Wall -Wextra -Wno-unused-parameter -Wno-unused-variable -Wno-sign-compare") # set(CMAKE_CXX_FLAGS "-Wno-unused-parameter -Wno-unused-variable -Wno-reorder -Wno-sign-compare -Wvla-cxx-extension") diff --git a/src/fdb5/api/FDBFactory.cc b/src/fdb5/api/FDBFactory.cc index 07260054f..ee1cc2643 100644 --- a/src/fdb5/api/FDBFactory.cc +++ b/src/fdb5/api/FDBFactory.cc @@ -28,22 +28,8 @@ namespace fdb5 { //---------------------------------------------------------------------------------------------------------------------- -FDBBase::FDBBase(const Config& config, const std::string& name) : name_(name), config_(config) { - - bool writable = config.getBool("writable", true); - bool visitable = config.getBool("visitable", true); - if (!config.getBool("list", visitable)) { - controlIdentifiers_ |= ControlIdentifier::List; - } - if (!config.getBool("retrieve", visitable)) { - controlIdentifiers_ |= ControlIdentifier::Retrieve; - } - if (!config.getBool("archive", writable)) { - controlIdentifiers_ |= ControlIdentifier::Archive; - } - if (!config.getBool("wipe", writable)) { - controlIdentifiers_ |= ControlIdentifier::Wipe; - } +FDBBase::FDBBase(const Config& config, const std::string& name) : + name_(name), config_(config), controlIdentifiers_(ControlIdentifiers::parse(config)) { LOG_DEBUG_LIB(LibFdb5) << "FDBBase: " << config << std::endl; } diff --git a/src/fdb5/api/RemoteFDB.cc b/src/fdb5/api/RemoteFDB.cc index a85bc5ffa..2d801bb0a 100644 --- a/src/fdb5/api/RemoteFDB.cc +++ b/src/fdb5/api/RemoteFDB.cc @@ -1,5 +1,6 @@ #include #include +#include #include "eckit/config/Resource.h" #include "eckit/io/Buffer.h" @@ -139,7 +140,9 @@ const net::Endpoint& RemoteFDB::storeEndpoint() const { if (storesLocalFields_.empty()) { throw SeriousBug("Unable to find a store to serve local data"); } - return storesLocalFields_.at(std::rand() % storesLocalFields_.size()); + static std::mt19937 rd; + static std::uniform_int_distribution dist(0, storesLocalFields_.size() - 1); + return storesLocalFields_.at(dist(rd)); } const net::Endpoint& RemoteFDB::storeEndpoint(const net::Endpoint& fieldLocationEndpoint) const { // looking for an alias for the given endpoint @@ -158,7 +161,7 @@ const net::Endpoint& RemoteFDB::storeEndpoint(const net::Endpoint& fieldLocation RemoteFDB::RemoteFDB(const Configuration& config, const std::string& name) : LocalFDB(config, name), Client(config) { - Buffer buf = controlWriteReadResponse(remote::Message::Stores, generateRequestID()); + Buffer buf = controlWriteReadResponse(Message::Stores, generateRequestID()); MemoryStream s(buf); size_t numStores; s >> numStores; @@ -209,7 +212,7 @@ RemoteFDB::RemoteFDB(const Configuration& config, const std::string& name) : Loc fieldLocationEndpoints.push_back(""); } - Buffer buf2 = controlWriteReadResponse(remote::Message::Schema, generateRequestID()); + Buffer buf2 = controlWriteReadResponse(Message::Schema, generateRequestID()); MemoryStream s2(buf2); Schema* schema = Reanimator::reanimate(s2); @@ -217,13 +220,10 @@ RemoteFDB::RemoteFDB(const Configuration& config, const std::string& name) : Loc config_.set("stores", stores); config_.set("fieldLocationEndpoints", fieldLocationEndpoints); config_.overrideSchema(static_cast(controlEndpoint()) + "/schema", schema); +} - /// @note: We must instantiate the ReadLimiter before any RemoteStores due to their static initialisation. - /// @todo: this may change in future. - static size_t memoryLimit = - Resource("$FDB_READ_LIMIT;fdbReadLimit", - config_.userConfig().getUnsigned("limits.read", size_t(1) * 1024 * 1024 * 1024)); // 1GiB - ReadLimiter::init(memoryLimit); +RemoteFDB::~RemoteFDB() { + deregister(); } // ----------------------------------------------------------------------------------------------------- @@ -248,14 +248,16 @@ auto RemoteFDB::forwardApiCall(const HelperClass& helper, const FDBToolRequest& // Ensure we have an entry in the message queue before we trigger anything that // will result in return messages - uint32_t id = generateRequestID(); - auto entry = messageQueues_.emplace(id, std::make_shared(HelperClass::queueSize())); - ASSERT(entry.second); - std::shared_ptr messageQueue(entry.first->second); + std::shared_ptr messageQueue; + { + std::lock_guard lock(messageMutex_); + auto entry = messageQueues_.emplace(id, std::make_shared(HelperClass::queueSize())); + ASSERT(entry.second); + messageQueue = entry.first->second; + } // Encode the request and send it to the server - Buffer encodeBuffer(HelperClass::bufferSize()); MemoryStream s(encodeBuffer); s << request; @@ -313,64 +315,69 @@ const Configuration& RemoteFDB::clientConfig() const { return config(); } -bool RemoteFDB::handle(remote::Message message, uint32_t requestID) { +bool RemoteFDB::handle(Message message, uint32_t requestID) { switch (message) { case Message::Complete: { - + std::lock_guard lock(messageMutex_); auto it = messageQueues_.find(requestID); if (it == messageQueues_.end()) { return false; } it->second->close(); - // Remove entry (shared_ptr --> message queue will be destroyed when it - // goes out of scope in the worker thread). + // Remove entry (shared_ptr --> message queue will be destroyed when it goes out of scope in the worker + // thread). messageQueues_.erase(it); return true; } case Message::Error: { - - std::ostringstream ss; - ss << "RemoteFDB - client id: " << clientId() - << " - received an error without error description for requestID " << requestID << std::endl; - throw RemoteFDBException(ss.str(), controlEndpoint()); - - return false; + std::lock_guard lock(messageMutex_); + // Received Error message without error description. Remove the corresponding entry from the message queue + // and let the caller know & complain + auto it = messageQueues_.find(requestID); + if (it != messageQueues_.end()) { + it->second->interrupt( + std::make_exception_ptr(RemoteFDBException("no error description provided", controlEndpoint()))); + // Remove entry (shared_ptr --> message queue will be destroyed when it goes out of scope in the worker + // thread). + messageQueues_.erase(it); + } + return true; } default: + Log::error() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID << "]" + << std::endl; return false; } } -bool RemoteFDB::handle(remote::Message message, uint32_t requestID, Buffer&& payload) { +bool RemoteFDB::handle(Message message, uint32_t requestID, Buffer&& payload) { switch (message) { case Message::Blob: { + std::lock_guard lock(messageMutex_); auto it = messageQueues_.find(requestID); if (it == messageQueues_.end()) { return false; } - it->second->emplace(std::move(payload)); return true; } - case Message::Error: { - + std::lock_guard lock(messageMutex_); auto it = messageQueues_.find(requestID); - if (it == messageQueues_.end()) { - return false; + if (it != messageQueues_.end()) { + std::string errmsg{static_cast(payload.data()), payload.size()}; + it->second->interrupt(std::make_exception_ptr(RemoteFDBException(errmsg, controlEndpoint()))); + // Remove entry (shared_ptr --> message queue will be destroyed when it goes out of scope in the worker + // thread). + messageQueues_.erase(it); } - std::string msg; - msg.resize(payload.size(), ' '); - payload.copy(&msg[0], payload.size()); - it->second->interrupt(std::make_exception_ptr(RemoteFDBException(msg, controlEndpoint()))); - // Remove entry (shared_ptr --> message queue will be destroyed when it - // goes out of scope in the worker thread). - messageQueues_.erase(it); return true; } default: + Log::warning() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID + << ",payloadSize=" << payload.size() << "]" << std::endl; return false; } } diff --git a/src/fdb5/api/RemoteFDB.h b/src/fdb5/api/RemoteFDB.h index a741cd89e..d6129dd21 100644 --- a/src/fdb5/api/RemoteFDB.h +++ b/src/fdb5/api/RemoteFDB.h @@ -41,7 +41,7 @@ class RemoteFDB : public LocalFDB, public Client { public: // method RemoteFDB(const eckit::Configuration& config, const std::string& name); - ~RemoteFDB() override {} + ~RemoteFDB() override; ListIterator inspect(const metkit::mars::MarsRequest& request) override; @@ -94,6 +94,7 @@ class RemoteFDB : public LocalFDB, public Client { // The shared_ptr allows this removal to be asynchronous with the actual task // cleaning up and returning to the client. std::unordered_map> messageQueues_; + std::mutex messageMutex_; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/fdb5/api/helpers/ControlIterator.cc b/src/fdb5/api/helpers/ControlIterator.cc index d3509b1e3..194be19dd 100644 --- a/src/fdb5/api/helpers/ControlIterator.cc +++ b/src/fdb5/api/helpers/ControlIterator.cc @@ -10,6 +10,8 @@ #include "fdb5/api/helpers/ControlIterator.h" +#include + #include "eckit/serialisation/Stream.h" #include "fdb5/database/Catalogue.h" @@ -32,6 +34,36 @@ eckit::Stream& operator>>(eckit::Stream& s, ControlAction& a) { //---------------------------------------------------------------------------------------------------------------------- +std::ostream& operator<<(std::ostream& s, const ControlIdentifier& i) { + switch (i) { + case ControlIdentifier::None: + s << "None"; + break; + case ControlIdentifier::List: + s << "List"; + break; + case ControlIdentifier::Retrieve: + s << "Retrieve"; + break; + case ControlIdentifier::Archive: + s << "Archive"; + break; + case ControlIdentifier::Wipe: + s << "Wipe"; + break; + case ControlIdentifier::UniqueRoot: + s << "UniqueRoot"; + break; + case ControlIdentifier::UnsafeWipeAll: + s << "UnsafeWipeAll"; + break; + } + s << "(" << static_cast::type>(i) << ")"; + return s; +} + +//---------------------------------------------------------------------------------------------------------------------- + ControlIdentifierIterator::ControlIdentifierIterator(const ControlIdentifiers& identifiers) : value_(0), remaining_(identifiers.value_) { @@ -86,6 +118,61 @@ ControlIdentifiers::ControlIdentifiers(eckit::Stream& s) { s >> value_; } +ControlIdentifiers ControlIdentifiers::parse(const eckit::LocalConfiguration& config, bool unsafeWipeAllDefault) { + ControlIdentifiers identifiers; + + bool writable = config.getBool("writable", true); + bool visitable = config.getBool("visitable", true); + if (!config.getBool("list", visitable)) { + identifiers.value_ |= static_cast(ControlIdentifier::List); + } + if (!config.getBool("retrieve", visitable)) { + identifiers.value_ |= static_cast(ControlIdentifier::Retrieve); + } + if (!config.getBool("archive", writable)) { + identifiers.value_ |= static_cast(ControlIdentifier::Archive); + } + if (!config.getBool("wipe", writable)) { + identifiers.value_ |= static_cast(ControlIdentifier::Wipe); + } + // Unsafe Wipe all is disabled by default, unless explicitly enabled in the configuration file + if (!config.getBool("unsafeWipeAll", identifiers.enabled(ControlIdentifier::Wipe) && unsafeWipeAllDefault)) { + identifiers.value_ |= static_cast(ControlIdentifier::UnsafeWipeAll); + } + return identifiers; +} + +ControlIdentifiers ControlIdentifiers::parse(const eckit::LocalConfiguration& config, ControlIdentifiers defaultValue) { + ControlIdentifiers identifiers = defaultValue; + + std::optional writable; + if (config.has("writable")) { + writable = config.getBool("writable"); + } + std::optional visitable; + if (config.has("visitable")) { + visitable = config.getBool("visitable"); + } + if (!config.getBool("list", visitable ? *visitable : defaultValue.enabled(ControlIdentifier::List))) { + identifiers.value_ |= static_cast(ControlIdentifier::List); + } + if (!config.getBool("retrieve", visitable ? *visitable : defaultValue.enabled(ControlIdentifier::Retrieve))) { + identifiers.value_ |= static_cast(ControlIdentifier::Retrieve); + } + if (!config.getBool("archive", writable ? *writable : defaultValue.enabled(ControlIdentifier::Archive))) { + identifiers.value_ |= static_cast(ControlIdentifier::Archive); + } + if (!config.getBool("wipe", writable ? *writable : defaultValue.enabled(ControlIdentifier::Wipe))) { + identifiers.value_ |= static_cast(ControlIdentifier::Wipe); + } + // Unsafe Wipe all is disabled by default, unless explicitly enabled in the configuration file + if (!config.getBool("unsafeWipeAll", identifiers.enabled(ControlIdentifier::Wipe) && + defaultValue.enabled(ControlIdentifier::UnsafeWipeAll))) { + identifiers.value_ |= static_cast(ControlIdentifier::UnsafeWipeAll); + } + return identifiers; +} + ControlIdentifiers& ControlIdentifiers::operator|=(const ControlIdentifier& val) { value_ |= static_cast(val); return *this; diff --git a/src/fdb5/api/helpers/ControlIterator.h b/src/fdb5/api/helpers/ControlIterator.h index 72ea30933..980eda5b8 100644 --- a/src/fdb5/api/helpers/ControlIterator.h +++ b/src/fdb5/api/helpers/ControlIterator.h @@ -15,6 +15,7 @@ #include +#include "eckit/config/LocalConfiguration.h" #include "eckit/filesystem/URI.h" #include "fdb5/api/helpers/APIIterator.h" @@ -49,12 +50,15 @@ enum class ControlIdentifier : uint16_t { Retrieve = 1 << 1, Archive = 1 << 2, Wipe = 1 << 3, - UniqueRoot = 1 << 4 + UniqueRoot = 1 << 4, + UnsafeWipeAll = 1 << 5 }; +std::ostream& operator<<(std::ostream& s, const ControlIdentifier& m); + static const std::initializer_list ControlIdentifierList{ - ControlIdentifier::List, ControlIdentifier::Retrieve, ControlIdentifier::Archive, ControlIdentifier::Wipe, - ControlIdentifier::UniqueRoot}; + ControlIdentifier::List, ControlIdentifier::Retrieve, ControlIdentifier::Archive, + ControlIdentifier::Wipe, ControlIdentifier::UniqueRoot, ControlIdentifier::UnsafeWipeAll}; //---------------------------------------------------------------------------------------------------------------------- // An iterator to facilitate working with the ControlIdentifiers structure @@ -96,6 +100,9 @@ class ControlIdentifiers { ControlIdentifiers(const ControlIdentifier& val); ControlIdentifiers(eckit::Stream& s); + static ControlIdentifiers parse(const eckit::LocalConfiguration& config, bool unsafeWipeAllDefault = true); + static ControlIdentifiers parse(const eckit::LocalConfiguration& config, ControlIdentifiers defaultValue); + ControlIdentifiers& operator|=(const ControlIdentifier& val); ControlIdentifiers operator|(const ControlIdentifier& val); diff --git a/src/fdb5/api/local/QueryVisitor.h b/src/fdb5/api/local/QueryVisitor.h index c2a03fa04..88c8714ee 100644 --- a/src/fdb5/api/local/QueryVisitor.h +++ b/src/fdb5/api/local/QueryVisitor.h @@ -16,9 +16,9 @@ /// @author Simon Smart /// @date November 2018 -#ifndef fdb5_api_local_QueryVisitor_H -#define fdb5_api_local_QueryVisitor_H +#pragma once +#include #include #include @@ -49,6 +49,7 @@ class QueryVisitor : public EntryVisitor { const metkit::mars::MarsRequest& canonicalise(const Rule& rule) const { bool success; + std::lock_guard lock(canonicalisedMutex_); auto it = canonicalised_.find(&rule.registry()); if (it == canonicalised_.end()) { std::tie(it, success) = canonicalised_.emplace(&rule.registry(), rule.registry().canonicalise(request_)); @@ -67,11 +68,10 @@ class QueryVisitor : public EntryVisitor { /// Cache of canonicalised requests mutable std::unordered_map canonicalised_; + mutable std::mutex canonicalisedMutex_; ///< Protects canonicalised_ map }; //---------------------------------------------------------------------------------------------------------------------- } // namespace fdb5::api::local - -#endif diff --git a/src/fdb5/database/WipeState.cc b/src/fdb5/database/WipeState.cc index e656da2fb..698f87190 100644 --- a/src/fdb5/database/WipeState.cc +++ b/src/fdb5/database/WipeState.cc @@ -1,8 +1,12 @@ #include "fdb5/database/WipeState.h" + #include + +#include "eckit/config/Resource.h" #include "eckit/exception/Exceptions.h" #include "eckit/filesystem/URI.h" #include "eckit/log/Log.h" + #include "fdb5/LibFdb5.h" #include "fdb5/api/helpers/WipeIterator.h" #include "fdb5/database/Store.h" @@ -241,7 +245,10 @@ Store& StoreWipeState::store(const Config& config) const { void StoreWipeState::encode(eckit::Stream& s) const { - if (!signature_.isSigned()) { + static bool acceptUnsigned = + eckit::Resource("$FDB_ACCEPT_UNSIGNED_WIPE_STATE;fdbAcceptUnsignedWipeState", false); + + if (!signature_.isSigned() && !acceptUnsigned) { throw eckit::SeriousBug("StoreWipeState must be signed before encoding"); } diff --git a/src/fdb5/remote/Connection.cc b/src/fdb5/remote/Connection.cc index 4d7b2d846..cd1a8891e 100644 --- a/src/fdb5/remote/Connection.cc +++ b/src/fdb5/remote/Connection.cc @@ -15,7 +15,7 @@ namespace fdb5::remote { Connection::Connection() : single_(false) {} void Connection::teardown() { - closingSocket_ = true; + closingSocket_.store(true); if (!valid()) { return; @@ -91,11 +91,29 @@ eckit::Buffer Connection::read(bool control, MessageHeader& hdr) const { && readUnsafe(socket, &tail, sizeof(tail))) { ASSERT(tail == MessageHeader::EndMarker); + + if (hdr.message == Message::Exit) { + closingSocket_.store(true); + } + if (hdr.message == Message::Error) { + eckit::net::Endpoint remoteEndpoint{socket.remoteHost(), socket.remotePort()}; + std::ostringstream ss; + if (payload.size() == 0) { + ss << "Received an error without error description for clientID " << hdr.clientID() << " requestID " + << hdr.requestID << std::endl; + throw RemoteFDBException(ss.str(), remoteEndpoint); + } + std::string errmsg{static_cast(payload.data()), payload.size()}; + ss << "Received error message: \"" << errmsg << "\" from " << remoteEndpoint << " for clientID " + << hdr.clientID() << " requestID " << hdr.requestID << std::endl; + eckit::Log::warning() << ss.str(); + } return payload; } } hdr.message = Message::Exit; + closingSocket_.store(true); return eckit::Buffer{0}; } @@ -130,7 +148,7 @@ void Connection::write(const Message msg, const bool control, const uint32_t cli void Connection::error(std::string_view msg, uint32_t clientID, uint32_t requestID) const { eckit::Log::error() << "[clientID=" << clientID << ",requestID=" << requestID << "] " << msg << std::endl; - write(Message::Error, false, clientID, requestID, msg.data(), msg.length()); + write(Message::Error, true, clientID, requestID, msg.data(), msg.length()); } eckit::Buffer Connection::readControl(MessageHeader& hdr) const { diff --git a/src/fdb5/remote/Connection.h b/src/fdb5/remote/Connection.h index f851b8eaf..221473752 100644 --- a/src/fdb5/remote/Connection.h +++ b/src/fdb5/remote/Connection.h @@ -19,6 +19,7 @@ #include #include "eckit/exception/Exceptions.h" +#include "eckit/net/Endpoint.h" #include "eckit/net/TCPSocket.h" #include "eckit/os/BackTrace.h" #include "eckit/serialisation/MemoryStream.h" @@ -50,6 +51,15 @@ class TCPException : public eckit::Exception { //---------------------------------------------------------------------------------------------------------------------- +class RemoteFDBException : public eckit::RemoteException { +public: + + RemoteFDBException(const std::string& msg, const eckit::net::Endpoint& endpoint) : + eckit::RemoteException(msg, endpoint) {} +}; + +//---------------------------------------------------------------------------------------------------------------------- + class Connection { public: // types @@ -106,7 +116,7 @@ class Connection { private: // members - bool closingSocket_ = false; + mutable std::atomic closingSocket_{false}; mutable std::mutex controlMutex_; mutable std::mutex dataMutex_; diff --git a/src/fdb5/remote/FdbServer.cc b/src/fdb5/remote/FdbServer.cc index 20eb87239..db8f90835 100644 --- a/src/fdb5/remote/FdbServer.cc +++ b/src/fdb5/remote/FdbServer.cc @@ -44,10 +44,6 @@ void FDBForker::run() { eckit::Monitor::instance().reset(); // needed to the monitor to work on forked (but not execed process) - // Ensure random state is reset after fork - ::srand(::getpid() + ::time(nullptr)); - ::srandom(::getpid() + ::time(nullptr)); - eckit::Log::info() << "FDB forked pid " << ::getpid() << " -- connection: " << socket_.localHost() << ":" << socket_.localPort() << "-->" << socket_.remoteHost() << ":" << socket_.remotePort() << std::endl; @@ -86,7 +82,7 @@ class FDBServerThread : public eckit::Thread { private: // members eckit::net::TCPSocket socket_; - eckit::LocalConfiguration config_; + Config config_; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/fdb5/remote/FdbServer.h b/src/fdb5/remote/FdbServer.h index 0776cb8da..c5bf824d2 100644 --- a/src/fdb5/remote/FdbServer.h +++ b/src/fdb5/remote/FdbServer.h @@ -45,7 +45,7 @@ class FDBForker : public eckit::ProcessControler { void run() override; eckit::net::TCPSocket socket_; - eckit::LocalConfiguration config_; + Config config_; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/fdb5/remote/Messages.cc b/src/fdb5/remote/Messages.cc index 2621e39ae..072668fe7 100644 --- a/src/fdb5/remote/Messages.cc +++ b/src/fdb5/remote/Messages.cc @@ -115,6 +115,9 @@ std::ostream& operator<<(std::ostream& s, const Message& m) { case Message::Complete: s << "Complete"; break; + case Message::Unauthorised: + s << "Unauthorised"; + break; // Data communication case Message::Blob: diff --git a/src/fdb5/remote/Messages.h b/src/fdb5/remote/Messages.h index a19472b43..0ad588762 100644 --- a/src/fdb5/remote/Messages.h +++ b/src/fdb5/remote/Messages.h @@ -78,6 +78,7 @@ enum class Message : uint16_t { // Responses Received = 200, Complete, + Unauthorised, // Data communication Blob = 300, diff --git a/src/fdb5/remote/RemoteFieldLocation.cc b/src/fdb5/remote/RemoteFieldLocation.cc index b6c9d8e6e..3387c79b7 100644 --- a/src/fdb5/remote/RemoteFieldLocation.cc +++ b/src/fdb5/remote/RemoteFieldLocation.cc @@ -104,13 +104,11 @@ eckit::DataHandle* RemoteFieldLocation::dataHandle() const { eckit::Log::debug() << std::endl; } - RemoteStore& store = RemoteStore::get(uri_); - eckit::URI remote = RemoteFieldLocation::internalURI(uri_); std::unique_ptr loc( FieldLocationFactory::instance().build(remote.scheme(), remote, offset_, length_, remapKey_)); - return store.dataHandle(*loc); + return RemoteStore::get(uri_).dataHandle(*loc); } void RemoteFieldLocation::visit(FieldLocationVisitor& visitor) const { diff --git a/src/fdb5/remote/RemoteFieldLocation.h b/src/fdb5/remote/RemoteFieldLocation.h index ecbb5a8c5..87b70157c 100644 --- a/src/fdb5/remote/RemoteFieldLocation.h +++ b/src/fdb5/remote/RemoteFieldLocation.h @@ -16,15 +16,12 @@ /// @author Simon Smart /// @date Nov 2016 -#ifndef fdb5_RemoteFieldLocation_H -#define fdb5_RemoteFieldLocation_H +#pragma once #include "fdb5/database/FieldLocation.h" namespace fdb5::remote { -class RemoteStore; - //---------------------------------------------------------------------------------------------------------------------- class RemoteFieldLocation : public FieldLocation { @@ -69,5 +66,3 @@ class RemoteFieldLocation : public FieldLocation { //---------------------------------------------------------------------------------------------------------------------- } // namespace fdb5::remote - -#endif // fdb5_RemoteFieldLocation_H diff --git a/src/fdb5/remote/client/Client.cc b/src/fdb5/remote/client/Client.cc index e3e51e784..a096bc723 100644 --- a/src/fdb5/remote/client/Client.cc +++ b/src/fdb5/remote/client/Client.cc @@ -59,7 +59,7 @@ Client::Client(const eckit::Configuration& config, } void Client::refreshConnection() { - if (connection_->valid()) { + if (connection_->valid()) { // Connection is still valid, no need to refresh return; } eckit::Log::warning() << "Connection to " << connection_->controlEndpoint() @@ -69,8 +69,14 @@ void Client::refreshConnection() { connection_->add(*this); } +void Client::deregister() { + if (!deregistered_.exchange(true)) { + connection_->remove(id_); + } +} + Client::~Client() { - connection_->remove(id_); + deregister(); } void Client::controlWriteCheckResponse(const Message msg, const uint32_t requestID, const bool dataListener, @@ -86,8 +92,16 @@ void Client::controlWriteCheckResponse(const Message msg, const uint32_t request } auto f = connection_->controlWrite(*this, msg, requestID, dataListener, payloads); - f.wait(); - ASSERT(f.get().size() == 0); + try { + f.wait(); + ASSERT(f.get().size() == 0); + } + catch (const std::exception& e) { + std::ostringstream ss; + ss << "Error while waiting for response to control message " << msg << " with requestID " << requestID << ": " + << e.what(); + throw RemoteFDBException(ss.str(), connection_->controlEndpoint()); + } } eckit::Buffer Client::controlWriteReadResponse(const Message msg, const uint32_t requestID, const void* const payload, @@ -103,8 +117,16 @@ eckit::Buffer Client::controlWriteReadResponse(const Message msg, const uint32_t } auto f = connection_->controlWrite(*this, msg, requestID, false, payloads); - f.wait(); - return eckit::Buffer{f.get()}; + try { + f.wait(); + return eckit::Buffer{f.get()}; + } + catch (const std::exception& e) { + std::ostringstream ss; + ss << "Error while waiting for response to control message " << msg << " with requestID " << requestID << ": " + << e.what(); + throw RemoteFDBException(ss.str(), connection_->controlEndpoint()); + } } void Client::dataWrite(Message msg, uint32_t requestID, PayloadList payloads) { diff --git a/src/fdb5/remote/client/Client.h b/src/fdb5/remote/client/Client.h index 320a6a62c..7bf09ad1f 100644 --- a/src/fdb5/remote/client/Client.h +++ b/src/fdb5/remote/client/Client.h @@ -17,6 +17,7 @@ #include "fdb5/remote/Messages.h" #include "fdb5/remote/client/ClientConnection.h" +#include #include #include // std::pair #include @@ -27,15 +28,6 @@ namespace fdb5::remote { //---------------------------------------------------------------------------------------------------------------------- -class RemoteFDBException : public eckit::RemoteException { -public: - - RemoteFDBException(const std::string& msg, const eckit::net::Endpoint& endpoint) : - eckit::RemoteException(msg, endpoint) {} -}; - -//---------------------------------------------------------------------------------------------------------------------- - class Client { public: // types @@ -91,6 +83,11 @@ class Client { protected: + /// Deregister this client from its connection. Idempotent. + /// Derived classes with state accessed by handle() should call this + /// in their destructor, before that state is destroyed. + void deregister(); + std::shared_ptr connection_; private: @@ -100,6 +97,7 @@ class Client { private: uint32_t id_; + std::atomic deregistered_{false}; mutable std::mutex blockingRequestMutex_; }; diff --git a/src/fdb5/remote/client/ClientConnection.cc b/src/fdb5/remote/client/ClientConnection.cc index 466717b00..191a59715 100644 --- a/src/fdb5/remote/client/ClientConnection.cc +++ b/src/fdb5/remote/client/ClientConnection.cc @@ -330,6 +330,13 @@ SessionID ClientConnection::verifyServerStartupResponse() { return serverSession; } +std::string msgHeader(MessageHeader& hdr, net::Endpoint& endpoint) { + std::ostringstream ss; + ss << (hdr.control() ? "CONTROL" : "DATA") << " connection=" << endpoint << " [message=" << hdr.message + << ",clientID=" << hdr.clientID() << ",requestID=" << hdr.requestID << ",payload=" << hdr.payloadSize << "]"; + return ss.str(); +} + void ClientConnection::listeningControlThreadLoop() { try { @@ -339,90 +346,104 @@ void ClientConnection::listeningControlThreadLoop() { while (true) { Buffer payload = Connection::readControl(hdr); - - LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::listeningControlThreadLoop - got [message=" << hdr.message - << ",clientID=" << hdr.clientID() << ",control=" << hdr.control() - << ",requestID=" << hdr.requestID << ",payload=" << hdr.payloadSize << "]" - << std::endl; + LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::listeningControlThreadLoop - " + << msgHeader(hdr, controlEndpoint_) << std::endl; if (hdr.message == Message::Exit) { - LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::listeningControlThreadLoop() -- Control thread stopping" + LOG_DEBUG_LIB(LibFdb5) << "CONTROL connection=" << controlEndpoint_ << " - thread stopping" << std::endl; return; } - else { - if (hdr.clientID()) { - bool handled = false; - ASSERT(hdr.control() || single_); + if (hdr.clientID()) { + ASSERT(hdr.control() || single_); + bool found = false; + bool handled = false; + { + // is the message a response to a blocking request? + // acquire the mutex and look for the request ID in the promises map + // only hold the mutex for the promise lookup/fulfillment, then release before calling handle(). std::lock_guard lock(promisesMutex_); - auto pp = promises_.find(hdr.requestID); if (pp != promises_.end()) { - if (hdr.payloadSize == 0) { - ASSERT(hdr.message == Message::Received); - pp->second.set_value(Buffer(0)); + found = true; + if (hdr.message == Message::Error) { // this is an error response to a blocking request, + // set the exception on the promise + std::string errmsg = + (hdr.payloadSize == 0) + ? "remote error - no error message provided" + : std::string{static_cast(payload.data()), payload.size()}; + try { + pp->second.set_exception( + std::make_exception_ptr(RemoteFDBException(errmsg, controlEndpoint()))); + } + catch (const std::exception& e) { + Log::error() + << "ERROR: " << msgHeader(hdr, controlEndpoint_) << " - received error \"" << errmsg + << "\" for blocking request - unable to set the exception on the promise: " + << e.what() << std::endl; + } + handled = true; } else { - pp->second.set_value(std::move(payload)); + if (hdr.payloadSize == 0) { + ASSERT(hdr.message == Message::Received); + pp->second.set_value(Buffer(0)); + } + else { + pp->second.set_value(std::move(payload)); + } + handled = true; } promises_.erase(pp); - handled = true; } - else { - Client* client = nullptr; - { - std::lock_guard lock(clientsMutex_); - - auto it = clients_.find(hdr.clientID()); - if (it == clients_.end()) { - std::ostringstream ss; - ss << "ERROR: CONTROL connection=" << controlEndpoint_ - << " received [clientID=" << hdr.clientID() << ",requestID=" << hdr.requestID - << ",message=" << hdr.message << ",payload=" << hdr.payloadSize << "]" << std::endl; - ss << "ClientID (" << hdr.clientID() << ") not found. ABORTING"; - Log::status() << ss.str() << std::endl; - Log::error() << "Retrieving... " << ss.str() << std::endl; - throw SeriousBug(ss.str(), Here()); - } - client = it->second; - } + } + if (!found) { + // if not a response to a blocking request, then it must be a message for a client, look up the + // client and call handle() + std::lock_guard lock(clientsMutex_); + auto it = clients_.find(hdr.clientID()); + if (it == clients_.end()) { + std::ostringstream ss; + ss << "ERROR: " << msgHeader(hdr, controlEndpoint_) << " - ClientID not found. ABORTING"; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw SeriousBug(ss.str(), Here()); + } - if (hdr.payloadSize == 0) { - handled = client->handle(hdr.message, hdr.requestID); - } - else { - handled = client->handle(hdr.message, hdr.requestID, std::move(payload)); - } + auto* client = it->second; + if (hdr.payloadSize == 0) { + handled = client->handle(hdr.message, hdr.requestID); + } + else { + handled = client->handle(hdr.message, hdr.requestID, std::move(payload)); } + } - if (!handled) { - std::ostringstream ss; - if (hdr.message == Message::Error) { - ss << "RemoteFDB received an unhandled error on CONTROL connection. [clientID=" - << hdr.clientID() << ",requestID=" << hdr.requestID << "]"; - if (hdr.payloadSize) { - std::string msg; - msg.resize(payload.size(), ' '); - payload.copy(msg.data(), payload.size()); - ss << ": " << msg; - } - throw RemoteFDBException(ss.str(), controlEndpoint_); - } - else { - ss << "ERROR: CONTROL connection=" << controlEndpoint_ - << "Unexpected message recieved [message=" << hdr.message - << ",clientID=" << hdr.clientID() << ",requestID=" << hdr.requestID << "]. ABORTING"; - Log::status() << ss.str() << std::endl; - Log::error() << "Client Retrieving... " << ss.str() << std::endl; - throw SeriousBug(ss.str(), Here()); + if (!handled) { + std::ostringstream ss; + ss << "ERROR: " << msgHeader(hdr, controlEndpoint_); + + if (hdr.message == Message::Error) { + ss << " - received an unhandled error"; + if (hdr.payloadSize) { + std::string errmsg{static_cast(payload.data()), payload.size()}; + ss << ": \"" << errmsg << "\""; } + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw RemoteFDBException(ss.str(), controlEndpoint_); + } + else { + ss << " - received unexpected message. ABORTING"; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw SeriousBug(ss.str(), Here()); } } } } - // We don't want to let exceptions escape inside a worker thread. } catch (const std::exception& e) { @@ -433,14 +454,6 @@ void ClientConnection::listeningControlThreadLoop() { } } -void ClientConnection::closeConnection() { - LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::closeConnection() -- Data thread stopping" << std::endl; - std::lock_guard lock(clientsMutex_); - for (auto& [id, client] : clients_) { - client->closeConnection(); - } -} - void ClientConnection::listeningDataThreadLoop() { try { @@ -452,65 +465,63 @@ void ClientConnection::listeningDataThreadLoop() { while (true) { Buffer payload = Connection::readData(hdr); - - LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::listeningDataThreadLoop - got [message=" << hdr.message - << ",requestID=" << hdr.requestID << ",payload=" << hdr.payloadSize << "]" + LOG_DEBUG_LIB(LibFdb5) << "ClientConnection::listeningDataThreadLoop - " << msgHeader(hdr, dataEndpoint_) << std::endl; if (hdr.message == Message::Exit) { - closeConnection(); + LOG_DEBUG_LIB(LibFdb5) << "DATA connection=" << dataEndpoint_ << " - thread stopping" << std::endl; + std::lock_guard lock(clientsMutex_); + for (auto& [id, client] : clients_) { + client->closeConnection(); + } return; } - else { - if (hdr.clientID()) { - bool handled = false; - Client* client = nullptr; - { - std::lock_guard lock(clientsMutex_); - - auto it = clients_.find(hdr.clientID()); - if (it == clients_.end()) { - std::ostringstream ss; - ss << "ERROR: DATA connection=" << dataEndpoint_ << " received [clientID=" << hdr.clientID() - << ",requestID=" << hdr.requestID << ",message=" << hdr.message - << ",payload=" << hdr.payloadSize << "]" << std::endl; - ss << "ClientID (" << hdr.clientID() << ") not found. ABORTING"; - Log::status() << ss.str() << std::endl; - Log::error() << "Retrieving... " << ss.str() << std::endl; - throw SeriousBug(ss.str(), Here()); - } - client = it->second; - } - ASSERT(client); - ASSERT(!hdr.control()); - if (hdr.payloadSize == 0) { - handled = client->handle(hdr.message, hdr.requestID); - } - else { - handled = client->handle(hdr.message, hdr.requestID, std::move(payload)); - } + if (hdr.clientID()) { + ASSERT(!hdr.control()); - if (!handled) { - std::ostringstream ss; - if (hdr.message == Message::Error) { - ss << "RemoteFDB received an unhandled error on DATA connection. [clientID=" - << hdr.clientID() << ",requestID=" << hdr.requestID << "]"; - if (hdr.payloadSize) { - std::string msg; - msg.resize(payload.size(), ' '); - payload.copy(msg.data(), payload.size()); - ss << ": " << msg; - } - throw RemoteFDBException(ss.str(), dataEndpoint_); - } - else { - ss << "ERROR: DATA connection=" << dataEndpoint_ << " Unexpected message recieved (" - << hdr.message << "). ABORTING"; - Log::status() << ss.str() << std::endl; - Log::error() << "Client Retrieving... " << ss.str() << std::endl; - throw SeriousBug(ss.str(), Here()); + bool handled = false; + + // Hold clientsMutex_ across handle() to prevent the Client + // from being destroyed (via remove()) while handle() is in flight. + std::lock_guard lock(clientsMutex_); + + auto it = clients_.find(hdr.clientID()); + if (it == clients_.end()) { + std::ostringstream ss; + ss << "ERROR: " << msgHeader(hdr, dataEndpoint_) << " - ClientID not found. ABORTING"; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw SeriousBug(ss.str(), Here()); + } + + auto* client = it->second; + if (hdr.payloadSize == 0) { + handled = client->handle(hdr.message, hdr.requestID); + } + else { + handled = client->handle(hdr.message, hdr.requestID, std::move(payload)); + } + + if (!handled) { + std::ostringstream ss; + ss << "ERROR: " << msgHeader(hdr, dataEndpoint_); + + if (hdr.message == Message::Error) { + ss << " - received an unhandled error"; + if (hdr.payloadSize) { + std::string errmsg{static_cast(payload.data()), payload.size()}; + ss << ": \"" << errmsg << "\""; } + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw RemoteFDBException(ss.str(), dataEndpoint_); + } + else { + ss << " - received unexpected message. ABORTING"; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + throw SeriousBug(ss.str(), Here()); } } } diff --git a/src/fdb5/remote/client/ClientConnection.h b/src/fdb5/remote/client/ClientConnection.h index d0ffb102b..2c5baf89b 100644 --- a/src/fdb5/remote/client/ClientConnection.h +++ b/src/fdb5/remote/client/ClientConnection.h @@ -81,7 +81,6 @@ class ClientConnection : protected Connection { void listeningControlThreadLoop(); void listeningDataThreadLoop(); void dataWriteThreadLoop(); - void closeConnection(); const eckit::net::TCPSocket& controlSocket() const override { return controlClient_; } @@ -105,8 +104,6 @@ class ClientConnection : protected Connection { std::thread listeningControlThread_; std::thread listeningDataThread_; - std::mutex requestMutex_; - // requestID std::mutex idMutex_; uint32_t id_; diff --git a/src/fdb5/remote/client/ClientConnectionRouter.cc b/src/fdb5/remote/client/ClientConnectionRouter.cc index 01e861c43..a08c76718 100644 --- a/src/fdb5/remote/client/ClientConnectionRouter.cc +++ b/src/fdb5/remote/client/ClientConnectionRouter.cc @@ -1,7 +1,11 @@ #include "fdb5/remote/client/ClientConnectionRouter.h" -namespace { +#include +#include +#include +#include +namespace { class ConnectionError : public eckit::Exception { public: @@ -24,7 +28,11 @@ ConnectionError::ConnectionError(const eckit::net::Endpoint& endpoint) { reason(s.str()); eckit::Log::status() << what() << std::endl; } + +std::mutex initMutex; +std::unique_ptr instance_{nullptr}; } // namespace + namespace fdb5::remote { //---------------------------------------------------------------------------------------------------------------------- @@ -57,7 +65,8 @@ std::shared_ptr ClientConnectionRouter::connection( while (fullEndpoints.size() > 0) { // select a random endpoint - size_t idx = std::rand() % fullEndpoints.size(); + std::mt19937 rd; + size_t idx = std::uniform_int_distribution(0, fullEndpoints.size() - 1)(rd); eckit::net::Endpoint endpoint = fullEndpoints.at(idx).first; // look for the selected endpoint @@ -86,7 +95,7 @@ std::shared_ptr ClientConnectionRouter::connection( std::shared_ptr ClientConnectionRouter::refresh(const eckit::Configuration& config, const std::shared_ptr& connection) { - std::lock_guard lock(connectionMutex_); + std::lock_guard lock(connectionMutex_); const auto iter = connections_.find(connection->controlEndpoint()); if (iter == connections_.end() || !iter->second->valid()) { auto newConnection = @@ -109,9 +118,14 @@ void ClientConnectionRouter::deregister(ClientConnection& connection) { } } +ClientConnectionRouter::ClientConnectionRouter() {} + ClientConnectionRouter& ClientConnectionRouter::instance() { - static ClientConnectionRouter router; - return router; + std::lock_guard lock(initMutex); + if (!instance_) { + instance_.reset(new ClientConnectionRouter()); + } + return *instance_; } void ClientConnectionRouter::teardown(std::exception_ptr e) { diff --git a/src/fdb5/remote/client/ClientConnectionRouter.h b/src/fdb5/remote/client/ClientConnectionRouter.h index 43096b73d..9bfeb913b 100644 --- a/src/fdb5/remote/client/ClientConnectionRouter.h +++ b/src/fdb5/remote/client/ClientConnectionRouter.h @@ -47,7 +47,7 @@ class ClientConnectionRouter { private: - ClientConnectionRouter() {} ///< private constructor only used by singleton + ClientConnectionRouter(); ///< private constructor only used by singleton std::mutex connectionMutex_; diff --git a/src/fdb5/remote/client/ReadLimiter.cc b/src/fdb5/remote/client/ReadLimiter.cc index 51e34eae3..32787b4d9 100644 --- a/src/fdb5/remote/client/ReadLimiter.cc +++ b/src/fdb5/remote/client/ReadLimiter.cc @@ -9,6 +9,7 @@ */ #include "fdb5/remote/client/ReadLimiter.h" +#include #include #include "eckit/config/Resource.h" #include "fdb5/remote/client/RemoteStore.h" @@ -17,22 +18,30 @@ namespace fdb5::remote { //---------------------------------------------------------------------------------------------------------------------- namespace { -ReadLimiter* instance_ = nullptr; +std::mutex instanceMutex_; +std::unique_ptr instance_{nullptr}; } // namespace -bool ReadLimiter::isInitialised() { - return instance_ != nullptr; -} ReadLimiter& ReadLimiter::instance() { - ASSERT(instance_); + // the instance cannot be a static ReadLimiter, which is causing the following error on exit, + // when the instance is destroyed and the mutex is destroyed before the instance: + // libc++abi: terminating due to uncaught exception of type std::__1::system_error: mutex lock failed: Invalid + // argument + std::lock_guard lock(instanceMutex_); + if (instance_ == nullptr) { + instance_.reset(new ReadLimiter(defaultReadLimit())); + } return *instance_; } -void ReadLimiter::init(size_t memoryLimit) { - if (!instance_) { - instance_ = new ReadLimiter(memoryLimit); - } +size_t ReadLimiter::defaultReadLimit() { + static size_t limit = eckit::Resource("$FDB_READ_LIMIT;fdbReadLimit", size_t{1_GiB}); // 1 GiB default + return limit; +} + +void ReadLimiter::setMemoryLimit(size_t memoryLimit) { + memoryLimit_ = memoryLimit; } ReadLimiter::ReadLimiter(size_t memoryLimit) : memoryUsed_{0}, memoryLimit_{memoryLimit} {} @@ -61,7 +70,6 @@ void ReadLimiter::add(RemoteStore* client, uint32_t id, const FieldLocation& fie } bool ReadLimiter::tryNextRequest() { - std::lock_guard lock(mutex_); if (requests_.empty()) { return false; } @@ -104,33 +112,38 @@ void ReadLimiter::finishRequest(uint32_t clientID, uint32_t requestID) { /// @note: Only called when a RemoteStore is destroyed, which is currently on exit. void ReadLimiter::evictClient(size_t clientID) { - { - std::lock_guard lock(mutex_); + std::lock_guard lock(instanceMutex_); + if (instance_ != nullptr) { + std::lock_guard lock(instance_->mutex_); // Remove the client's active requests - auto it = activeRequests_.find(clientID); + auto it = instance_->activeRequests_.find(clientID); - if (it != activeRequests_.end()) { + if (it != instance_->activeRequests_.end()) { for (auto requestID : it->second) { - memoryUsed_ -= resultSizes_[{clientID, requestID}]; - resultSizes_.erase({clientID, requestID}); + instance_->memoryUsed_ -= instance_->resultSizes_[{clientID, requestID}]; + instance_->resultSizes_.erase({clientID, requestID}); } - activeRequests_.erase(it); + instance_->activeRequests_.erase(it); } // Clean up any pending requests attributed to this client ///@note O(n), room for optimisation. - auto it2 = requests_.begin(); - while (it2 != requests_.end()) { + auto it2 = instance_->requests_.begin(); + while (it2 != instance_->requests_.end()) { if (it2->client->id() == clientID) { - it2 = requests_.erase(it2); + it2 = instance_->requests_.erase(it2); } else { ++it2; // Only increment if we didn't erase } } - } + instance_->tryNextRequest(); - tryNextRequest(); + if (instance_->activeRequests_.empty() && instance_->requests_.empty()) { + // If there are no more active or pending requests, we can reset the instance to free memory. + instance_.reset(); + } + } } void ReadLimiter::print(std::ostream& out) const { diff --git a/src/fdb5/remote/client/ReadLimiter.h b/src/fdb5/remote/client/ReadLimiter.h index bcacc403a..3f9e73acc 100644 --- a/src/fdb5/remote/client/ReadLimiter.h +++ b/src/fdb5/remote/client/ReadLimiter.h @@ -21,8 +21,11 @@ #include #include + namespace fdb5::remote { +class RemoteStore; + //---------------------------------------------------------------------------------------------------------------------- struct RequestInfo { @@ -40,25 +43,18 @@ struct RequestInfo { class ReadLimiter { public: - static bool isInitialised(); - static ReadLimiter& instance(); + void setMemoryLimit(size_t memoryLimit); ReadLimiter(const ReadLimiter&) = delete; ReadLimiter& operator=(const ReadLimiter&) = delete; ReadLimiter(ReadLimiter&&) = delete; ReadLimiter& operator=(ReadLimiter&&) = delete; - static void init(size_t memoryLimit); - // Add a new request to the queue of requests to be sent. Will not be sent until we know we have buffer space. void add(RemoteStore* client, uint32_t id, const FieldLocation& fieldLocation, const Key& remapKey); // use const *? - // Attempt to send the next request in the queue. Returns true if a request was sent. - // If not enough memory is available, or there is no next request, returns false. - bool tryNextRequest(); - void finishRequest(uint32_t clientID, uint32_t requestID); // When a RemoteStore is destroyed, it must evict any unconsumed requests. @@ -66,7 +62,7 @@ class ReadLimiter { // request). /// @todo: This is somewhat pointless right now because the RemoteStores appear to be infinitely long lived... /// Revisit if this changes. - void evictClient(size_t clientID); + static void evictClient(size_t clientID); // Debugging void print(std::ostream& out) const; @@ -75,6 +71,12 @@ class ReadLimiter { ReadLimiter(size_t memoryLimit); + static size_t defaultReadLimit(); + + // Attempt to send the next request in the queue. Returns true if a request was sent. + // If not enough memory is available, or there is no next request, returns false. + bool tryNextRequest(); + // Send the request to the server void sendRequest(const RequestInfo& request) const; @@ -83,7 +85,7 @@ class ReadLimiter { mutable std::mutex mutex_; size_t memoryUsed_; - const size_t memoryLimit_; + size_t memoryLimit_; // Enqueued requests std::deque requests_; diff --git a/src/fdb5/remote/client/RemoteCatalogue.cc b/src/fdb5/remote/client/RemoteCatalogue.cc index a6b5c3713..81bbf79d4 100644 --- a/src/fdb5/remote/client/RemoteCatalogue.cc +++ b/src/fdb5/remote/client/RemoteCatalogue.cc @@ -196,14 +196,14 @@ const eckit::Configuration& RemoteCatalogue::clientConfig() const { } bool RemoteCatalogue::handle(Message message, uint32_t requestID) { - Log::warning() << *this << " - Received [message=" << ((uint)message) << ",requestID=" << requestID << "]" + Log::warning() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID << "]" << std::endl; return false; } bool RemoteCatalogue::handle(Message message, uint32_t requestID, eckit::Buffer&& payload) { - LOG_DEBUG_LIB(LibFdb5) << *this << " - Received [message=" << ((uint)message) << ",requestID=" << requestID - << ",payloadSize=" << payload.size() << "]" << std::endl; + Log::warning() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID + << ",payloadSize=" << payload.size() << "]" << std::endl; return false; } diff --git a/src/fdb5/remote/client/RemoteCatalogue.h b/src/fdb5/remote/client/RemoteCatalogue.h index b71a23bae..943e70b0f 100644 --- a/src/fdb5/remote/client/RemoteCatalogue.h +++ b/src/fdb5/remote/client/RemoteCatalogue.h @@ -106,10 +106,6 @@ class RemoteCatalogue : public CatalogueReader, public CatalogueWriter, public C // not implemented since the catalogue traversal is performed on the remote side std::optional computeAxis(const std::string& keyword) const override { NOTIMP; } -protected: - - ControlIdentifiers controlIdentifiers_; - private: Key currentIndexKey_; diff --git a/src/fdb5/remote/client/RemoteStore.cc b/src/fdb5/remote/client/RemoteStore.cc index d4836d26e..f91b141c2 100644 --- a/src/fdb5/remote/client/RemoteStore.cc +++ b/src/fdb5/remote/client/RemoteStore.cc @@ -24,8 +24,10 @@ #include "fdb5/remote/client/ReadLimiter.h" #include "fdb5/rules/Rule.h" +#include "eckit/config/Resource.h" #include "eckit/exception/Exceptions.h" #include "eckit/filesystem/URI.h" +#include "eckit/io/Buffer.h" #include "eckit/io/Length.h" #include "eckit/io/Offset.h" #include "eckit/log/Log.h" @@ -33,6 +35,7 @@ #include "eckit/runtime/Main.h" #include "eckit/serialisation/MemoryStream.h" #include "eckit/serialisation/Reanimator.h" +#include "eckit/utils/Literals.h" #include #include @@ -127,9 +130,7 @@ class FDBRemoteDataHandle : public DataHandle { // If we are in the DataHandle, then there MUST be data to read RemoteStore::StoredMessage msg = std::make_pair(remote::Message{}, eckit::Buffer{0}); - // eckit::Log::info() << "RemoteDataHandle::read() -- popping next" << std::endl; ASSERT(queue_->pop(msg) != -1); - // eckit::Log::info() << "RemoteDataHandle::read() -- popped next" << std::endl; // Handle any remote errors communicated from the server if (msg.first == Message::Error) { @@ -205,9 +206,13 @@ class FDBRemoteDataHandle : public DataHandle { Client::EndpointList storeEndpoints(const Config& config) { ASSERT(config.has("stores")); - ASSERT(config.has("fieldLocationEndpoints")); const auto stores = config.getStringVector("stores"); - const auto fieldLocationEndpoints = config.getStringVector("fieldLocationEndpoints"); + + // endpoints used in RemoteFieldLocations can differ from the store endpoints (e.g. different networks; canonical + // store names) if so, the canonical names must be provided in the fieldLocationEndpoints config, otherwise the + // store endpoints are used. + const auto fieldLocationEndpoints = + config.has("fieldLocationEndpoints") ? config.getStringVector("fieldLocationEndpoints") : stores; ASSERT(stores.size() == fieldLocationEndpoints.size()); @@ -218,7 +223,6 @@ Client::EndpointList storeEndpoints(const Config& config) { } return out; } - } // namespace //---------------------------------------------------------------------------------------------------------------------- @@ -234,6 +238,8 @@ RemoteStore::RemoteStore(const eckit::URI& uri, const Config& config) : } RemoteStore::~RemoteStore() { + deregister(); + // If we have launched a thread with an async and we manage to get here, this is // an error. n.b. if we don't do something, we will block in the destructor // of std::future. @@ -242,9 +248,7 @@ RemoteStore::~RemoteStore() { eckit::Main::instance().terminate(); } - if (ReadLimiter::isInitialised()) { - ReadLimiter::instance().evictClient(id()); - } + ReadLimiter::evictClient(id()); } eckit::URI RemoteStore::uri() const { @@ -351,16 +355,12 @@ void RemoteStore::print(std::ostream& out) const { } void RemoteStore::closeConnection() { + std::lock_guard lock(messageMutex_); for (auto& kv : messageQueues_) { if (!kv.second->closed()) { kv.second->interrupt(std::make_exception_ptr(eckit::Exception("Unexpected closure of store", Here()))); } } - for (auto& kv : retrieveMessageQueues_) { - if (!kv.second->closed()) { - kv.second->interrupt(std::make_exception_ptr(eckit::Exception("Unexpected closure of store", Here()))); - } - } } const eckit::Configuration& RemoteStore::clientConfig() const { @@ -371,38 +371,31 @@ bool RemoteStore::handle(Message message, uint32_t requestID) { switch (message) { case Message::Complete: { - // eckit::Log::info() << "RemoteStore::handle COMPLETE" << std::endl; - auto it = messageQueues_.find(requestID); - if (it != messageQueues_.end()) { - // eckit::Log::info() << "RemoteStore::handle COMPLETE close and erase queue" << std::endl; - it->second->close(); - - // Remove entry (shared_ptr --> message queue will be destroyed when it - // goes out of scope in the worker thread). - messageQueues_.erase(it); - // eckit::Log::info() << "RemoteStore::handle COMPLETE closed and erased queue" << std::endl; + std::lock_guard lock(messageMutex_); + auto id = messageQueues_.find(requestID); + if (id == messageQueues_.end()) { + return false; } - else { - std::lock_guard lock(retrieveMessageMutex_); - auto id = retrieveMessageQueues_.find(requestID); - ASSERT(id != retrieveMessageQueues_.end()); - id->second->emplace(std::make_pair(message, Buffer(0))); - - retrieveMessageQueues_.erase(id); - } + id->second->emplace(std::make_pair(message, Buffer(0))); + messageQueues_.erase(id); return true; } case Message::Error: { - - std::ostringstream ss; - ss << "RemoteStore client id: " << id() << " - received an error without error description for requestID " - << requestID << std::endl; - throw RemoteFDBException(ss.str(), controlEndpoint()); - - return false; + // Received Error message without error description. Remove the corresponding entry from the message queue + // and let the caller know & complain + std::lock_guard lock(messageMutex_); + auto it = messageQueues_.find(requestID); + if (it != messageQueues_.end()) { + it->second->interrupt( + std::make_exception_ptr(RemoteFDBException("no error description provided", controlEndpoint()))); + messageQueues_.erase(it); + } + return true; } default: + Log::warning() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID + << "]" << std::endl; return false; } } @@ -410,8 +403,8 @@ bool RemoteStore::handle(Message message, uint32_t requestID, eckit::Buffer&& pa switch (message) { - case Message::Store: { // received a Field location from the remote store, can forward to the archiver for the - // indexing + case Message::Store: { + // received a FieldLocation from the remote store, can forward to the archiver for the indexing MemoryStream s(payload); std::unique_ptr location(eckit::Reanimator::reanimate(s)); if (defaultEndpoint().empty()) { @@ -424,34 +417,25 @@ bool RemoteStore::handle(Message message, uint32_t requestID, eckit::Buffer&& pa } } case Message::Blob: { - auto it = messageQueues_.find(requestID); - if (it != messageQueues_.end()) { - it->second->emplace(message, std::move(payload)); - } - else { - std::lock_guard lock(retrieveMessageMutex_); - auto id = retrieveMessageQueues_.find(requestID); - ASSERT(id != retrieveMessageQueues_.end()); - id->second->emplace(std::make_pair(message, std::move(payload))); - } + std::lock_guard lock(messageMutex_); + auto id = messageQueues_.find(requestID); + ASSERT(id != messageQueues_.end()); + id->second->emplace(std::make_pair(message, std::move(payload))); return true; } case Message::Error: { - + std::lock_guard lock(messageMutex_); auto it = messageQueues_.find(requestID); if (it != messageQueues_.end()) { - std::string msg; - msg.resize(payload.size(), ' '); - payload.copy(&msg[0], payload.size()); - it->second->interrupt(std::make_exception_ptr(RemoteFDBException(msg, controlEndpoint()))); - - // Remove entry (shared_ptr --> message queue will be destroyed when it - // goes out of scope in the worker thread). + std::string errmsg{static_cast(payload.data()), payload.size()}; + it->second->interrupt(std::make_exception_ptr(RemoteFDBException(errmsg, controlEndpoint()))); messageQueues_.erase(it); } return true; } default: + Log::warning() << *this << " - Received unexpected [message=" << message << ",requestID=" << requestID + << ",payloadSize=" << payload.size() << "]" << std::endl; return false; } } @@ -467,9 +451,8 @@ eckit::DataHandle* RemoteStore::dataHandle(const FieldLocation& fieldLocation, c static size_t queueSize = 320; std::shared_ptr queue = nullptr; { - std::lock_guard lock(retrieveMessageMutex_); - - auto entry = retrieveMessageQueues_.emplace(id, std::make_shared(queueSize)); + std::lock_guard lock(messageMutex_); + auto entry = messageQueues_.emplace(id, std::make_shared(queueSize)); ASSERT(entry.second); queue = entry.first->second; @@ -557,7 +540,10 @@ bool RemoteStore::doWipeUnknowns(const std::set& unknownURIs) const eckit::Buffer sendBuf(1_KiB * unknownURIs.size() + 100); eckit::ResizableMemoryStream stream(sendBuf); stream << dbKey_; - stream << unknownURIs; + stream << unknownURIs.size(); + for (const auto& uri : unknownURIs) { + stream << uri; + } controlWriteCheckResponse(Message::DoWipeUnknowns, generateRequestID(), true, sendBuf, stream.position()); return true; } diff --git a/src/fdb5/remote/client/RemoteStore.h b/src/fdb5/remote/client/RemoteStore.h index 9b6eb27e2..b040b88b7 100644 --- a/src/fdb5/remote/client/RemoteStore.h +++ b/src/fdb5/remote/client/RemoteStore.h @@ -187,9 +187,8 @@ class RemoteStore : public Store, public Client { // The shared_ptr allows this removal to be asynchronous with the actual task // cleaning up and returning to the client. std::map> messageQueues_; - std::map> retrieveMessageQueues_; - std::mutex retrieveMessageMutex_; + std::mutex messageMutex_; Locations locations_; }; diff --git a/src/fdb5/remote/server/CatalogueHandler.cc b/src/fdb5/remote/server/CatalogueHandler.cc index dc4e416fe..cec7aa685 100644 --- a/src/fdb5/remote/server/CatalogueHandler.cc +++ b/src/fdb5/remote/server/CatalogueHandler.cc @@ -9,7 +9,7 @@ */ #include "fdb5/remote/server/CatalogueHandler.h" -#include "eckit/serialisation/ResizableMemoryStream.h" + #include "fdb5/LibFdb5.h" #include "fdb5/api/FDBFactory.h" #include "fdb5/api/helpers/FDBToolRequest.h" @@ -24,6 +24,7 @@ #include "eckit/net/NetMask.h" #include "eckit/net/TCPSocket.h" #include "eckit/serialisation/MemoryStream.h" +#include "eckit/serialisation/ResizableMemoryStream.h" #include "eckit/utils/Literals.h" #include @@ -44,7 +45,26 @@ namespace fdb5::remote { // *************************************************************************************** CatalogueHandler::CatalogueHandler(eckit::net::TCPSocket& socket, const Config& config) : - ServerConnection(socket, config), fdbControlConnection_(false), fdbDataConnection_(false) {} + ServerConnection(socket, config), fdbControlConnection_(false), fdbDataConnection_(false) { + + eckit::net::IPAddress clientIPaddress{controlSocket_.remoteAddr()}; + + clientNetwork_ = ""; + if (config_.has("networks")) { + for (const auto& net : config_.getSubConfigurations("networks")) { + if (net.has("name") && net.has("netmask")) { + eckit::net::NetMask netmask{net.getString("netmask")}; + if (netmask.contains(clientIPaddress)) { + clientNetwork_ = net.getString("name"); + controlIdentifiers_ = ControlIdentifiers::parse(net, controlIdentifiers_); + break; + } + } + } + } + + LOG_DEBUG_LIB(LibFdb5) << "Client " << clientIPaddress << " from network '" << clientNetwork_ << "'" << std::endl; +} CatalogueHandler::~CatalogueHandler() {} @@ -57,7 +77,7 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint std::lock_guard lock(handlerMutex_); auto it = fdbs_.find(clientID); if (it == fdbs_.end()) { - fdbs_[clientID]; + fdbs_.emplace(clientID, innerConfig_); fdbControlConnection_ = true; fdbDataConnection_ = !single_; numControlConnection_++; @@ -73,7 +93,6 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint stores(clientID, requestID); return Handled::Replied; - case Message::Archive: // notification that the client is starting to send data locations for archival archiver(); return Handled::YesAddArchiveListener; @@ -87,6 +106,13 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint } } } + catch (UnauthorisedException& e) { + std::ostringstream ss; + ss << "ERROR: Operation " << e.controlIdentifier() << " is not enabled for client " << clientID; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + error(ss.str(), clientID, requestID); + } catch (std::exception& e) { // n.b. more general than eckit::Exception error(e.what(), clientID, requestID); @@ -132,29 +158,36 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint return Handled::Replied; case Message::Wipe: // Initial wipe request + isEnabled(ControlIdentifier::Wipe); wipe(clientID, requestID, std::move(payload)); return Handled::Yes; case Message::DoMaskIndexEntries: - // doit! We expect DoMaskIndexEntries, doWipeURIs, DoWipeUnknowns and doWipeEmptyDatabase in succession + isEnabled(ControlIdentifier::Wipe); + // doit! We expect DoMaskIndexEntries, doWipeURIs, DoWipeUnknowns and doWipeEmptyDatabase in + // succession doMaskIndexEntries(clientID, requestID, std::move(payload)); return Handled::Yes; case Message::DoWipeURIs: // Do the wipe on our currentWipeState + isEnabled(ControlIdentifier::Wipe); doWipeURIs(clientID, requestID, std::move(payload)); return Handled::Yes; case Message::DoWipeFinish: // Finish wipe by deleting empty DBs + isEnabled(ControlIdentifier::Wipe); doWipeEmptyDatabase(clientID, requestID, std::move(payload)); return Handled::Yes; case Message::DoWipeUnknowns: // Wipe a set of unknown URIs + isEnabled(ControlIdentifier::Wipe); doWipeUnknowns(clientID, requestID, std::move(payload)); return Handled::Yes; case Message::DoUnsafeFullWipe: // wipe a full database including its content + isEnabled(ControlIdentifier::Wipe); doUnsafeFullWipe(clientID, requestID, std::move(payload)); - return Handled::Replied; + return Handled::Yes; default: { std::ostringstream ss; @@ -165,6 +198,13 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint } } } + catch (UnauthorisedException& e) { + std::ostringstream ss; + ss << "ERROR: Operation " << e.controlIdentifier() << " is not enabled for client " << clientID; + Log::status() << ss.str() << std::endl; + Log::error() << ss.str() << std::endl; + error(ss.str(), clientID, requestID); + } catch (std::exception& e) { // n.b. more general than eckit::Exception error(e.what(), clientID, requestID); @@ -175,7 +215,6 @@ Handled CatalogueHandler::handleControl(Message message, uint32_t clientID, uint return Handled::No; } - // API forwarding logic, adapted from original remoteHandler // Used for Inspect and List // *************************************************************************************** @@ -234,10 +273,11 @@ struct WipeHelper : public BaseHelper { eckit::Buffer encodeBuffer(encodeBufferSize(state)); ResizableMemoryStream s(encodeBuffer); - const std::string dummy_secret = eckit::Resource("$FDB_WIPE_SECRET;fdbWipeSecret", ""); - ASSERT(!dummy_secret.empty()); - - state.signStoreStates(dummy_secret); + static const std::string wipeSecret = eckit::Resource("$FDB_WIPE_SECRET;fdbWipeSecret", ""); + if (wipeSecret.empty()) { + throw(Exception("Unable to sign the wipe details")); + } + state.signStoreStates(wipeSecret); s << state; @@ -249,8 +289,11 @@ struct WipeHelper : public BaseHelper { // Expect this dbKey not to already be in progress ASSERT(handler.wipesInProgress_.find(dbKey) == handler.wipesInProgress_.end()); + if (unsafeWipeAll_) { + handler.isEnabled(ControlIdentifier::UnsafeWipeAll); + } handler.wipesInProgress_[dbKey] = { - unsafeWipeAll_, CatalogueReaderFactory::instance().build(dbKey, handler.config_), + unsafeWipeAll_, CatalogueReaderFactory::instance().build(dbKey, handler.innerConfig_), CatalogueWipeState(dbKey, state.safeURIs(), state.deleteMap(), state.indexesToMask())}; } else { @@ -265,8 +308,6 @@ struct WipeHelper : public BaseHelper { } WipeStateIterator apiCall(FDB& fdb, const FDBToolRequest& request) const { - // XXX: I'm inclined to say that in a multi-server scenario, unsafe wipe all is a bad idea. - ASSERT(!unsafeWipeAll_); return fdb.internal_->wipe(request, doit_, false, unsafeWipeAll_); } @@ -300,7 +341,7 @@ void CatalogueHandler::handleApiCall(uint32_t clientID, uint32_t requestID, ecki std::lock_guard lock(fdbMutex_); auto it = fdbs_.find(clientID); if (it == fdbs_.end()) { - fdbs_[clientID]; + fdbs_.emplace(clientID, innerConfig_); } } @@ -335,22 +376,27 @@ void CatalogueHandler::handleApiCall(uint32_t clientID, uint32_t requestID, ecki } void CatalogueHandler::list(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::List); handleApiCall(clientID, requestID, std::move(payload)); } void CatalogueHandler::axes(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::List); handleApiCall(clientID, requestID, std::move(payload)); } void CatalogueHandler::wipe(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::Wipe); handleApiCall(clientID, requestID, std::move(payload)); } void CatalogueHandler::inspect(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::Retrieve); handleApiCall(clientID, requestID, std::move(payload)); } void CatalogueHandler::stats(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::List); handleApiCall(clientID, requestID, std::move(payload)); } @@ -360,7 +406,7 @@ void CatalogueHandler::schema(uint32_t clientID, uint32_t requestID, eckit::Buff eckit::MemoryStream stream(schemaBuffer); if (payload.size() == 0) { // client requesting the top-level schema - stream << config_.schema(); + stream << innerConfig_.schema(); } else { // 1. Read dbkey to select catalogue @@ -369,6 +415,7 @@ void CatalogueHandler::schema(uint32_t clientID, uint32_t requestID, eckit::Buff // 2. Get catalogue Catalogue& cat = catalogue(clientID, dbKey); + cat.controlIdentifiers().enabled(ControlIdentifier::List); const Schema& schema = cat.schema(); stream << schema; } @@ -378,32 +425,15 @@ void CatalogueHandler::schema(uint32_t clientID, uint32_t requestID, eckit::Buff void CatalogueHandler::stores(uint32_t clientID, uint32_t requestID) { - eckit::net::IPAddress clientIPaddress{controlSocket_.remoteAddr()}; - - std::string clientNetwork = ""; - if (config_.has("networks")) { - for (const auto& net : config_.getSubConfigurations("networks")) { - if (net.has("name") && net.has("netmask")) { - eckit::net::NetMask netmask{net.getString("netmask")}; - if (netmask.contains(clientIPaddress)) { - clientNetwork = net.getString("name"); - break; - } - } - } - } - - LOG_DEBUG_LIB(LibFdb5) << "Client " << clientIPaddress << " from network '" << clientNetwork << "'" << std::endl; - ASSERT(config_.has("stores")); std::map> stores; for (const auto& configStore : config_.getSubConfigurations("stores")) { ASSERT(configStore.has("default")); eckit::net::Endpoint fieldLocationEndpoint{configStore.getString("default")}; eckit::net::Endpoint storeEndpoint{fieldLocationEndpoint}; - if (!clientNetwork.empty()) { - if (configStore.has(clientNetwork)) { - storeEndpoint = eckit::net::Endpoint{configStore.getString(clientNetwork)}; + if (!clientNetwork_.empty()) { + if (configStore.has(clientNetwork_)) { + storeEndpoint = eckit::net::Endpoint{configStore.getString(clientNetwork_)}; } } @@ -450,7 +480,7 @@ void CatalogueHandler::exists(uint32_t clientID, uint32_t requestID, eckit::Buff { eckit::MemoryStream stream(payload); const Key dbKey(stream); - exists = CatalogueReaderFactory::instance().build(dbKey, config_)->exists(); + exists = CatalogueReaderFactory::instance().build(dbKey, innerConfig_)->exists(); } eckit::Buffer existBuf(5); @@ -462,6 +492,8 @@ void CatalogueHandler::exists(uint32_t clientID, uint32_t requestID, eckit::Buff void CatalogueHandler::flush(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) { + isEnabled(ControlIdentifier::Archive); + ASSERT(payload.size() > 0); size_t numArchived = 0; @@ -519,7 +551,6 @@ void CatalogueHandler::archiveBlob(const uint32_t clientID, const uint32_t reque if (it == catalogues_.end()) { std::string what("Requested unknown catalogue id: " + std::to_string(clientID)); error(what, 0, 0); - throw; } } @@ -579,7 +610,7 @@ CatalogueWriter& CatalogueHandler::catalogue(uint32_t id, const Key& dbKey) { if (!single_) { numDataConnection_++; } - return *((catalogues_.emplace(id, CatalogueArchiver(!single_, dbKey, config_)).first)->second.catalogue); + return *((catalogues_.emplace(id, CatalogueArchiver(!single_, dbKey, innerConfig_)).first)->second.catalogue); } const CatalogueHandler::WipeInProgress& CatalogueHandler::cachedWipeState(Key dbKey) const { diff --git a/src/fdb5/remote/server/CatalogueHandler.h b/src/fdb5/remote/server/CatalogueHandler.h index 6fa7083d7..89c0bc549 100644 --- a/src/fdb5/remote/server/CatalogueHandler.h +++ b/src/fdb5/remote/server/CatalogueHandler.h @@ -56,6 +56,8 @@ class CatalogueHandler : public ServerConnection { private: // methods + friend struct WipeHelper; + Handled handleControl(Message message, uint32_t clientID, uint32_t requestID) override; Handled handleControl(Message message, uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload) override; @@ -85,8 +87,6 @@ class CatalogueHandler : public ServerConnection { void doWipeEmptyDatabase(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload); void doUnsafeFullWipe(uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload); - const Config& config() const { return config_; } - const WipeInProgress& cachedWipeState(Key dbKey) const; private: // member @@ -101,6 +101,8 @@ class CatalogueHandler : public ServerConnection { bool fdbControlConnection_; bool fdbDataConnection_; + std::string clientNetwork_; + struct WipeInProgress { bool unsafeWipeAll = false; std::unique_ptr catalogue; diff --git a/src/fdb5/remote/server/ServerConnection.cc b/src/fdb5/remote/server/ServerConnection.cc index 6655c2f2d..f1e627ebf 100644 --- a/src/fdb5/remote/server/ServerConnection.cc +++ b/src/fdb5/remote/server/ServerConnection.cc @@ -72,6 +72,9 @@ ServerConnection::ServerConnection(eckit::net::TCPSocket& socket, const Config& archiveQueue_(eckit::Resource("fdbServerMaxQueueSize", defaultArchiveQueueSize)), controlSocket_(socket) { + controlIdentifiers_ = ControlIdentifiers::parse(config, false); + + innerConfig_ = config_.has("fdb") ? Config{config_.getSubConfiguration("fdb"), config_.userConfig()} : config_; LOG_DEBUG_LIB(LibFdb5) << "ServerConnection::ServerConnection initialized" << std::endl; } @@ -518,4 +521,10 @@ void ServerConnection::waitForWorkers() { } } +void ServerConnection::isEnabled(ControlIdentifier identifier) { + if (!controlIdentifiers_.enabled(identifier)) { + throw UnauthorisedException(identifier); + } +} + } // namespace fdb5::remote diff --git a/src/fdb5/remote/server/ServerConnection.h b/src/fdb5/remote/server/ServerConnection.h index 903d3fac4..cde5a3882 100644 --- a/src/fdb5/remote/server/ServerConnection.h +++ b/src/fdb5/remote/server/ServerConnection.h @@ -21,12 +21,14 @@ #include #include "eckit/container/Queue.h" +#include "eckit/exception/Exceptions.h" #include "eckit/io/Buffer.h" #include "eckit/io/DataHandle.h" #include "eckit/net/TCPServer.h" #include "eckit/net/TCPSocket.h" #include "eckit/runtime/SessionID.h" +#include "fdb5/api/helpers/ControlIterator.h" #include "fdb5/config/Config.h" #include "fdb5/remote/Connection.h" #include "fdb5/remote/Messages.h" @@ -44,6 +46,19 @@ enum class Handled { Replied, }; +class UnauthorisedException : public eckit::Exception { +public: + + UnauthorisedException(ControlIdentifier ci) : + eckit::Exception("UnauthorisedException: " + std::to_string(static_cast(ci))), ci_(ci) {} + + ControlIdentifier controlIdentifier() const { return ci_; } + +private: + + ControlIdentifier ci_; +}; + //---------------------------------------------------------------------------------------------------------------------- class Handler { @@ -121,6 +136,8 @@ class ServerConnection : public Connection, public Handler { void archiver(); void queue(Message message, uint32_t clientID, uint32_t requestID, eckit::Buffer&& payload); + void isEnabled(ControlIdentifier identifier); + void handleException(std::exception_ptr e) override; private: @@ -139,6 +156,10 @@ class ServerConnection : public Connection, public Handler { virtual bool remove(bool control, uint32_t clientID) = 0; Config config_; + Config innerConfig_; + + ControlIdentifiers controlIdentifiers_; + std::string dataListenHostname_; eckit::Queue readLocationQueue_; diff --git a/src/fdb5/remote/server/StoreHandler.cc b/src/fdb5/remote/server/StoreHandler.cc index 54efa7c2d..45f81d867 100644 --- a/src/fdb5/remote/server/StoreHandler.cc +++ b/src/fdb5/remote/server/StoreHandler.cc @@ -296,8 +296,7 @@ Store& StoreHandler::getStore(uint32_t clientID) { for (const auto& kv : stores_) { Log::error() << " clientID: " << kv.first << ", store: " << *(kv.second.store) << std::endl; } - write(Message::Error, true, 0, 0, what.c_str(), what.length()); - throw; + error(what, 0, 0); } return *(it->second.store); @@ -315,7 +314,7 @@ Store& StoreHandler::store(uint32_t clientID, const Key& dbKey) { if (!single_) { numDataConnection_++; } - return *((stores_.emplace(clientID, StoreHelper(!single_, dbKey, config_)).first)->second.store); + return *((stores_.emplace(clientID, StoreHelper(!single_, dbKey, innerConfig_)).first)->second.store); } Store& StoreHandler::getStore(uint32_t clientID, const eckit::URI& uri) { @@ -330,7 +329,7 @@ Store& StoreHandler::getStore(uint32_t clientID, const eckit::URI& uri) { if (!single_) { numDataConnection_++; } - return *((stores_.emplace(clientID, StoreHelper(!single_, uri, config_)).first)->second.store); + return *((stores_.emplace(clientID, StoreHelper(!single_, uri, innerConfig_)).first)->second.store); } void StoreHandler::exists(const uint32_t clientID, const uint32_t requestID, const eckit::Buffer& payload) const { @@ -342,7 +341,7 @@ void StoreHandler::exists(const uint32_t clientID, const uint32_t requestID, con { eckit::MemoryStream stream(payload); const Key dbKey(stream); - exists = StoreFactory::instance().build(dbKey, config_)->exists(); + exists = StoreFactory::instance().build(dbKey, innerConfig_)->exists(); } eckit::Buffer existBuf(5); @@ -358,7 +357,12 @@ void StoreHandler::doWipeUnknowns(const uint32_t clientID, const uint32_t reques const WipeInProgress& currentWipe = cachedWipeState(key); - std::set uris{s}; + size_t numUnknowns = 0; + s >> numUnknowns; + std::set uris; + for (size_t i = 0; i < numUnknowns; ++i) { + uris.emplace(s); + } // Only proceed if unsafeWipeAll is set. ASSERT(currentWipe.unsafeWipeAll); @@ -430,6 +434,9 @@ const StoreHandler::WipeInProgress& StoreHandler::cachedWipeState(const Key& uri void StoreHandler::finaliseWipeState(const uint32_t clientID, const uint32_t requestID, const eckit::Buffer& payload) { + static bool acceptUnsigned = + eckit::Resource("$FDB_ACCEPT_UNSIGNED_WIPE_STATE;fdbAcceptUnsignedWipeState", false); + bool unsafeAll = false; bool doit = false; eckit::MemoryStream inStream(payload); @@ -441,11 +448,12 @@ void StoreHandler::finaliseWipeState(const uint32_t clientID, const uint32_t req // XXX Validate signature. const std::string dummy_secret = eckit::Resource("$FDB_WIPE_SECRET;fdbWipeSecret", ""); - ASSERT(!dummy_secret.empty()); - - uint64_t expected_hash = inState.hash(dummy_secret); - ASSERT(inState.signature().validSignature(expected_hash)); + if (!acceptUnsigned) { + ASSERT(!dummy_secret.empty()); + uint64_t expected_hash = inState.hash(dummy_secret); + ASSERT(inState.signature().validSignature(expected_hash)); + } // -- From here on, we can trust the state came from the catalogue. -- // The URIs need to be converted to internal URIs for this store. @@ -478,7 +486,9 @@ void StoreHandler::finaliseWipeState(const uint32_t clientID, const uint32_t req // keep state for doWipeURIs if (doit) { - ASSERT(!unsafeAll); // Until Im explicitly told otherwise, we dont support unsafeAll on remote fdb. + if (unsafeAll) { + isEnabled(ControlIdentifier::UnsafeWipeAll); + } wipesInProgress_.emplace(dbkey, WipeInProgress{unsafeAll, std::move(storeState)}); } } diff --git a/src/fdb5/toc/TocCatalogueWriter.cc b/src/fdb5/toc/TocCatalogueWriter.cc index 7a42d488f..61f81e46b 100644 --- a/src/fdb5/toc/TocCatalogueWriter.cc +++ b/src/fdb5/toc/TocCatalogueWriter.cc @@ -52,6 +52,13 @@ TocCatalogueWriter::~TocCatalogueWriter() { // selectIndex is called during schema traversal and in case of out-of-order fieldLocation archival bool TocCatalogueWriter::selectIndex(const Key& idxKey) { + std::lock_guard lock(indexMutex_); + return selectIndexUnsafe(idxKey); +} + +// selectIndexUnsafe is not mutex protected, and should only be called by mutex-protected functions (archive, index, +// selectIndex and reconsolidateIndexesAndTocs) +bool TocCatalogueWriter::selectIndexUnsafe(const Key& idxKey) { currentIndexKey_ = idxKey; @@ -139,14 +146,16 @@ void TocCatalogueWriter::close() { } void TocCatalogueWriter::index(const Key& key, const eckit::URI& uri, eckit::Offset offset, eckit::Length length) { + std::lock_guard lock(indexMutex_); + archivedLocations_++; if (current_.null()) { ASSERT(!currentIndexKey_.empty()); - selectIndex(currentIndexKey_); + selectIndexUnsafe(currentIndexKey_); } - Field field(TocFieldLocation(uri, offset, length, Key()), currentIndex().timestamp()); + Field field(TocFieldLocation(uri, offset, length, Key()), currentIndexUnsafe().timestamp()); current_.put(key, field); @@ -211,6 +220,7 @@ void TocCatalogueWriter::reconsolidateIndexesAndTocs() { close(); // Add masking entries for all the indexes and subtocs visited so far + std::lock_guard lock(indexMutex_); Buffer buf(sizeof(TocRecord) * (subtocs.size() + maskable_indexes)); buf.zero(); @@ -238,10 +248,14 @@ void TocCatalogueWriter::reconsolidateIndexesAndTocs() { } const Index& TocCatalogueWriter::currentIndex() { + std::lock_guard lock(indexMutex_); + return currentIndexUnsafe(); +} +const Index& TocCatalogueWriter::currentIndexUnsafe() { if (current_.null()) { ASSERT(!currentIndexKey_.empty()); - selectIndex(currentIndexKey_); + selectIndexUnsafe(currentIndexKey_); } return current_; @@ -323,12 +337,13 @@ bool TocCatalogueWriter::enabled(const ControlIdentifier& controlIdentifier) con void TocCatalogueWriter::archive(const Key& idxKey, const Key& datumKey, std::shared_ptr fieldLocation) { + std::lock_guard lock(indexMutex_); archivedLocations_++; if (current_.null()) { ASSERT(!currentIndexKey_.empty()); - if (!selectIndex(currentIndexKey_)) { + if (!selectIndexUnsafe(currentIndexKey_)) { createIndex(currentIndexKey_, datumKey.size()); } } @@ -336,13 +351,13 @@ void TocCatalogueWriter::archive(const Key& idxKey, const Key& datumKey, // in case of async archival (out of order store/catalogue archival), currentIndexKey_ can differ from the // indexKey used for store archival. Reset it if (currentIndexKey_ != idxKey) { - if (!selectIndex(idxKey)) { + if (!selectIndexUnsafe(idxKey)) { createIndex(idxKey, datumKey.size()); } } } - Field field(std::move(fieldLocation), currentIndex().timestamp()); + Field field(std::move(fieldLocation), currentIndexUnsafe().timestamp()); current_.put(datumKey, field); diff --git a/src/fdb5/toc/TocCatalogueWriter.h b/src/fdb5/toc/TocCatalogueWriter.h index b1a4e68e7..b8e704903 100644 --- a/src/fdb5/toc/TocCatalogueWriter.h +++ b/src/fdb5/toc/TocCatalogueWriter.h @@ -16,12 +16,13 @@ #ifndef fdb5_TocCatalogueWriter_H #define fdb5_TocCatalogueWriter_H +#include + #include "eckit/os/AutoUmask.h" #include "fdb5/database/Index.h" -#include "fdb5/toc/TocRecord.h" - #include "fdb5/toc/TocCatalogue.h" +#include "fdb5/toc/TocRecord.h" #include "fdb5/toc/TocSerialisationVersion.h" namespace fdb5 { @@ -81,6 +82,9 @@ class TocCatalogueWriter : public TocCatalogue, public CatalogueWriter { private: // methods + bool selectIndexUnsafe(const Key& idxKey); + const Index& currentIndexUnsafe(); + void closeIndexes(); void flushIndexes(); void compactSubTocIndexes(); @@ -110,6 +114,8 @@ class TocCatalogueWriter : public TocCatalogue, public CatalogueWriter { eckit::AutoUmask umask_; size_t archivedLocations_; + + std::mutex indexMutex_; }; //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/fdb5/toc/TocHandler.cc b/src/fdb5/toc/TocHandler.cc index 68f769578..ae15b8aff 100644 --- a/src/fdb5/toc/TocHandler.cc +++ b/src/fdb5/toc/TocHandler.cc @@ -53,6 +53,7 @@ constexpr const char* archive_lock_file = "archive.lock"; constexpr const char* list_lock_file = "list.lock"; constexpr const char* wipe_lock_file = "wipe.lock"; constexpr const char* allow_duplicates_file = "duplicates.allow"; +constexpr const char* unsafe_wipe_lock_file = "unsafe_wipe.lock"; } // namespace const std::map controlfile_lookup{ @@ -60,7 +61,8 @@ const std::map controlfile_lookup{ {ControlIdentifier::Archive, archive_lock_file}, {ControlIdentifier::List, list_lock_file}, {ControlIdentifier::Wipe, wipe_lock_file}, - {ControlIdentifier::UniqueRoot, allow_duplicates_file}}; + {ControlIdentifier::UniqueRoot, allow_duplicates_file}, + {ControlIdentifier::UnsafeWipeAll, unsafe_wipe_lock_file}}; //----------------------------------------------------------------------------------------------------------------------