diff --git a/src/spider/CMakeLists.txt b/src/spider/CMakeLists.txt index c1a4458a1..da1e3f236 100644 --- a/src/spider/CMakeLists.txt +++ b/src/spider/CMakeLists.txt @@ -1,5 +1,8 @@ # set variable as CACHE INTERNAL to access it from other scope set(SPIDER_CORE_SOURCES + core/DataCleaner.cpp + core/DriverCleaner.cpp + core/JobCleaner.cpp core/Task.cpp storage/mysql/MySqlConnection.cpp storage/mysql/MySqlStorageFactory.cpp @@ -13,9 +16,13 @@ set(SPIDER_CORE_SOURCES ) set(SPIDER_CORE_HEADERS + core/Context.hpp core/Error.hpp core/Data.hpp + core/DataCleaner.hpp core/Driver.hpp + core/DriverCleaner.hpp + core/JobCleaner.hpp core/KeyValueData.hpp core/Task.hpp core/TaskGraph.hpp diff --git a/src/spider/client/Data.hpp b/src/spider/client/Data.hpp index f3d7b89e1..b329e439f 100644 --- a/src/spider/client/Data.hpp +++ b/src/spider/client/Data.hpp @@ -1,7 +1,6 @@ #ifndef SPIDER_CLIENT_DATA_HPP #define SPIDER_CLIENT_DATA_HPP -#include #include #include #include @@ -9,9 +8,9 @@ #include #include -#include - #include +#include +#include #include #include // IWYU pragma: keep #include @@ -130,46 +129,37 @@ class Data { conn = std::move(std::get>(conn_result)); } core::StorageErr err; - switch (m_data_source) { - case DataSource::Driver: - err = m_data_store->add_driver_data(*conn, m_source_id, *data); + switch (m_context.get_source()) { + case core::Context::Source::Driver: + err = m_data_store->add_driver_data(*conn, m_context.get_id(), *data); if (!err.success()) { throw ConnectionException(err.description); } break; - case DataSource::TaskContext: - err = m_data_store->add_task_data(*conn, m_source_id, *data); + case core::Context::Source::Task: + err = m_data_store->add_task_data(*conn, m_context.get_id(), *data); if (!err.success()) { throw ConnectionException(err.description); } break; } - return Data{std::move(data), m_data_store, m_storage_factory, m_connection}; + return Data{std::move(data), m_context, m_data_store, m_storage_factory, m_connection}; } private: - enum class DataSource : std::uint8_t { - Driver, - TaskContext - }; - - Builder(std::shared_ptr data_store, - boost::uuids::uuid const source_id, - DataSource const data_source, + Builder(core::Context context, + std::shared_ptr data_store, std::shared_ptr storage_factory) - : m_data_store{std::move(data_store)}, - m_source_id{source_id}, - m_data_source{data_source}, + : m_context{context}, + m_data_store{std::move(data_store)}, m_storage_factory{std::move(storage_factory)} {} - Builder(std::shared_ptr data_store, - boost::uuids::uuid const source_id, - DataSource const data_source, + Builder(core::Context context, + std::shared_ptr data_store, std::shared_ptr storage_factory, std::shared_ptr connection) - : m_data_store{std::move(data_store)}, - m_source_id{source_id}, - m_data_source{data_source}, + : m_context{context}, + m_data_store{std::move(data_store)}, m_storage_factory{std::move(storage_factory)}, m_connection{std::move(connection)} {} @@ -181,8 +171,7 @@ class Data { std::shared_ptr m_storage_factory; std::shared_ptr m_connection = nullptr; - boost::uuids::uuid m_source_id; - DataSource m_data_source; + core::Context m_context; friend class Driver; friend class TaskContext; @@ -192,23 +181,40 @@ class Data { private: Data(std::unique_ptr impl, + core::Context context, std::shared_ptr data_store, std::shared_ptr storage_factory) - : m_impl{std::move(impl)}, + : m_data_cleaner{std::make_unique( + impl->get_id(), + context, + data_store, + storage_factory, + nullptr + )}, + m_impl{std::move(impl)}, m_data_store{std::move(data_store)}, m_storage_factory{std::move(storage_factory)} {} Data(std::unique_ptr impl, + core::Context context, std::shared_ptr data_store, std::shared_ptr storage_factory, std::shared_ptr connection) - : m_impl{std::move(impl)}, + : m_data_cleaner{std::make_unique( + impl->get_id(), + context, + data_store, + storage_factory, + connection + )}, + m_impl{std::move(impl)}, m_data_store{std::move(data_store)}, m_storage_factory{std::move(storage_factory)}, m_connection{std::move(connection)} {} [[nodiscard]] auto get_impl() const -> std::unique_ptr const& { return m_impl; } + std::unique_ptr m_data_cleaner; std::unique_ptr m_impl; std::shared_ptr m_data_store; std::shared_ptr m_storage_factory; diff --git a/src/spider/client/Driver.cpp b/src/spider/client/Driver.cpp index a15f32502..61bfe15f5 100644 --- a/src/spider/client/Driver.cpp +++ b/src/spider/client/Driver.cpp @@ -14,6 +14,7 @@ #include #include +#include #include #include #include // IWYU pragma: keep @@ -44,6 +45,13 @@ Driver::Driver(std::string const& storage_url) throw ConnectionException(err.description); } + m_driver_cleaner = std::make_unique( + m_id, + m_metadata_storage, + m_storage_factory, + m_conn + ); + // Start a thread to send heartbeats // NOLINTNEXTLINE(performance-unnecessary-value-param) m_heartbeat_thread = std::jthread([this](std::stop_token stoken) { @@ -85,6 +93,13 @@ Driver::Driver(std::string const& storage_url, boost::uuids::uuid const id) throw ConnectionException(err.description); } + m_driver_cleaner = std::make_unique( + m_id, + m_metadata_storage, + m_storage_factory, + m_conn + ); + // Start a thread to send heartbeats // NOLINTNEXTLINE(performance-unnecessary-value-param) m_heartbeat_thread = std::jthread([this](std::stop_token stoken) { diff --git a/src/spider/client/Driver.hpp b/src/spider/client/Driver.hpp index e9ba87d6c..324336e19 100644 --- a/src/spider/client/Driver.hpp +++ b/src/spider/client/Driver.hpp @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include @@ -83,9 +83,8 @@ class Driver { auto get_data_builder() -> Data::Builder { using DataBuilder = typename Data::Builder; return DataBuilder{ + core::Context{core::Context::Source::Driver, m_id}, m_data_storage, - m_id, - DataBuilder::DataSource::Driver, m_storage_factory, m_conn }; @@ -228,8 +227,7 @@ class Driver { return Job{ job_id, - Job::JobSource::Driver, - m_id, + core::Context{core::Context::Source::Driver, m_id}, m_metadata_storage, m_data_storage, m_storage_factory, @@ -293,8 +291,7 @@ class Driver { return Job{ job_id, - Job::JobSource::Driver, - m_id, + core::Context{core::Context::Source::Driver, m_id}, m_metadata_storage, m_data_storage, m_storage_factory, @@ -322,6 +319,7 @@ class Driver { private: boost::uuids::uuid m_id; + std::unique_ptr m_driver_cleaner; std::shared_ptr m_metadata_storage; std::shared_ptr m_data_storage; std::shared_ptr m_storage_factory; diff --git a/src/spider/client/Job.hpp b/src/spider/client/Job.hpp index 415dc4111..36df17398 100644 --- a/src/spider/client/Job.hpp +++ b/src/spider/client/Job.hpp @@ -20,8 +20,10 @@ #include #include #include +#include #include #include +#include #include #include // IWYU pragma: keep #include @@ -157,34 +159,37 @@ class Job { } private: - enum class JobSource : uint8_t { - Driver, - Task, - }; - Job(boost::uuids::uuid const id, - JobSource const source, - boost::uuids::uuid const source_id, + core::Context context, std::shared_ptr metadata_storage, std::shared_ptr data_storage, std::shared_ptr storage_factory) : m_id{id}, - m_source{source}, - m_source_id{source_id}, + m_context{context}, + m_job_cleaner{std::make_unique( + id, + metadata_storage, + storage_factory, + nullptr + )}, m_metadata_storage{std::move(metadata_storage)}, m_data_storage{std::move(data_storage)}, m_storage_factory{std::move(storage_factory)} {} Job(boost::uuids::uuid const id, - JobSource const source, - boost::uuids::uuid const source_id, + core::Context context, std::shared_ptr metadata_storage, std::shared_ptr data_storage, std::shared_ptr storage_factory, std::shared_ptr conn) : m_id{id}, - m_source{source}, - m_source_id{source_id}, + m_context{context}, + m_job_cleaner{std::make_unique( + id, + metadata_storage, + storage_factory, + conn + )}, m_metadata_storage{std::move(metadata_storage)}, m_data_storage{std::move(data_storage)}, m_storage_factory{std::move(storage_factory)}, @@ -255,17 +260,17 @@ class Job { if (!optional_data_id.has_value()) { throw ConnectionException{fmt::format("Output data ID is missing")}; } - if (m_source == JobSource::Driver) { + if (m_context.get_source() == core::Context::Source::Driver) { err = m_data_storage->get_driver_data( conn, - m_source_id, + m_context.get_id(), optional_data_id.value(), &data ); } else { err = m_data_storage->get_task_data( conn, - m_source_id, + m_context.get_id(), optional_data_id.value(), &data ); @@ -277,7 +282,9 @@ class Job { } std::get(result) = core::DataImpl::create_data( std::make_unique(std::move(data)), - m_data_storage + m_context, + m_data_storage, + m_storage_factory ); } else { if (output.get_type() != typeid(T).name()) { @@ -329,17 +336,17 @@ class Job { if (!optional_data_id.has_value()) { throw ConnectionException{fmt::format("Output data ID is missing")}; } - if (m_source == JobSource::Driver) { + if (m_context.get_source() == core::Context::Source::Driver) { err = m_data_storage->get_driver_data( conn, - m_source_id, + m_context.get_id(), optional_data_id.value(), &data ); } else { err = m_data_storage->get_task_data( conn, - m_source_id, + m_context.get_id(), optional_data_id.value(), &data ); @@ -351,6 +358,7 @@ class Job { } return core::DataImpl::create_data( std::make_unique(std::move(data)), + m_context, m_data_storage, m_storage_factory ); @@ -378,8 +386,8 @@ class Job { // NOLINTEND(readability-function-cognitive-complexity) boost::uuids::uuid m_id; - JobSource m_source; - boost::uuids::uuid m_source_id; + core::Context m_context; + std::unique_ptr m_job_cleaner; std::shared_ptr m_metadata_storage; std::shared_ptr m_data_storage; std::shared_ptr m_storage_factory; diff --git a/src/spider/client/TaskContext.hpp b/src/spider/client/TaskContext.hpp index 0a8696287..3fe15fb49 100644 --- a/src/spider/client/TaskContext.hpp +++ b/src/spider/client/TaskContext.hpp @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include @@ -61,9 +61,8 @@ class TaskContext { auto get_data_builder() -> Data::Builder { using DataBuilder = typename Data::Builder; return DataBuilder{ + core::Context{core::Context::Source::Task, m_task_id}, m_data_store, - m_task_id, - DataBuilder::DataSource::TaskContext, m_storage_factory }; } @@ -174,8 +173,7 @@ class TaskContext { return Job{ job_id, - Job::JobSource::Task, - m_task_id, + core::Context{core::Context::Source::Task, m_task_id}, m_metadata_store, m_data_store, m_storage_factory @@ -233,8 +231,7 @@ class TaskContext { return Job{ job_id, - Job::JobSource::Task, - m_task_id, + core::Context{core::Context::Source::Task, m_task_id}, m_metadata_store, m_data_store, m_storage_factory diff --git a/src/spider/core/Context.hpp b/src/spider/core/Context.hpp new file mode 100644 index 000000000..af1b1da00 --- /dev/null +++ b/src/spider/core/Context.hpp @@ -0,0 +1,34 @@ +#ifndef SPIDER_CORE_CONTEXT_HPP +#define SPIDER_CORE_CONTEXT_HPP + +#include + +#include + +namespace spider::core { +/** + * Represents the execution context in which operations are performed. + * + * The Context class encapsulates information about whether the current + * execution is occurring in a client (e.g. driver) or within a task. + */ +class Context { +public: + enum class Source : uint8_t { + Driver, + Task, + }; + + Context(Source const source, boost::uuids::uuid const id) : m_source{source}, m_id{id} {} + + [[nodiscard]] auto get_source() const -> Source { return m_source; } + + [[nodiscard]] auto get_id() const -> boost::uuids::uuid { return m_id; } + +private: + Source m_source; + boost::uuids::uuid m_id; +}; +} // namespace spider::core + +#endif diff --git a/src/spider/core/DataCleaner.cpp b/src/spider/core/DataCleaner.cpp new file mode 100644 index 000000000..61a2ad0b9 --- /dev/null +++ b/src/spider/core/DataCleaner.cpp @@ -0,0 +1,53 @@ +#include "DataCleaner.hpp" + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include +#include + +namespace spider::core { +DataCleaner::DataCleaner( + boost::uuids::uuid data_id, + Context const& context, + std::shared_ptr data_storage, + std::shared_ptr storage_factory, + std::shared_ptr storage_connection +) + : m_data_id{data_id}, + m_context{context}, + m_data_store{std::move(data_storage)}, + m_storage_factory{std::move(storage_factory)}, + m_connection{std::move(storage_connection)} {} + +DataCleaner::~DataCleaner() noexcept { + int const num_exceptions = std::uncaught_exceptions(); + // If destructor is called during stack unwinding, do not remove data reference. + if (num_exceptions > m_num_exceptions) { + return; + } + std::shared_ptr conn = m_connection; + if (nullptr == conn) { + std::variant, StorageErr> conn_result + = m_storage_factory->provide_storage_connection(); + // If we cannot get the connection, that means we are in a worker. + // Just let the reference stays until the job is removed. + if (std::holds_alternative(conn_result)) { + return; + } + conn = std::move(std::get>(conn_result)); + } + if (m_context.get_source() == Context::Source::Driver) { + m_data_store->remove_driver_reference(*conn, m_data_id, m_context.get_id()); + } else { + m_data_store->remove_task_reference(*conn, m_data_id, m_context.get_id()); + } +} +} // namespace spider::core diff --git a/src/spider/core/DataCleaner.hpp b/src/spider/core/DataCleaner.hpp new file mode 100644 index 000000000..682dc4113 --- /dev/null +++ b/src/spider/core/DataCleaner.hpp @@ -0,0 +1,56 @@ +#ifndef SPIDER_CORE_DATACLEANER_HPP +#define SPIDER_CORE_DATACLEANER_HPP + +#include +#include + +#include + +#include +#include +#include +#include + +namespace spider::core { +/* + * Tracks the number of exceptions and ensures proper cleanup when the `Data` + * object—holding a `unique_ptr` to this class—is destroyed. + * + * We avoid placing a custom destructor directly inside `Data` because, when a + * `Data` object is moved, its destructor is still invoked. This can result in + * double deletion. By using a `unique_ptr`, we ensure that destruction only + * occurs for the final owner, preventing such issues. + * + * We use std::uncaught_exceptions() to track the number of exceptions so that + * we don't clean up if the destructor is called during exception handling. + */ +class DataCleaner { +public: + DataCleaner( + boost::uuids::uuid data_id, + Context const& context, + std::shared_ptr data_storage, + std::shared_ptr storage_factory, + std::shared_ptr storage_connection + ); + ~DataCleaner() noexcept; + + // Delete copy constructor and assignment operator + DataCleaner(DataCleaner const&) = delete; + auto operator=(DataCleaner const&) -> DataCleaner& = delete; + // Default move constructor and assignment operator + DataCleaner(DataCleaner&&) = default; + auto operator=(DataCleaner&&) -> DataCleaner& = default; + +private: + int m_num_exceptions = std::uncaught_exceptions(); + + boost::uuids::uuid m_data_id; + Context m_context; + std::shared_ptr m_data_store; + std::shared_ptr m_storage_factory; + std::shared_ptr m_connection = nullptr; +}; +} // namespace spider::core + +#endif diff --git a/src/spider/core/DataImpl.hpp b/src/spider/core/DataImpl.hpp index bd7001399..be4090c2c 100644 --- a/src/spider/core/DataImpl.hpp +++ b/src/spider/core/DataImpl.hpp @@ -5,6 +5,7 @@ #include #include +#include #include #include @@ -14,10 +15,11 @@ class DataImpl { template static auto create_data( std::unique_ptr data, + Context const& context, std::shared_ptr data_store, std::shared_ptr storage_factory ) -> spider::Data { - return spider::Data{std::move(data), data_store, storage_factory}; + return spider::Data{std::move(data), context, data_store, storage_factory}; } template diff --git a/src/spider/core/DriverCleaner.cpp b/src/spider/core/DriverCleaner.cpp new file mode 100644 index 000000000..1d1b1d77c --- /dev/null +++ b/src/spider/core/DriverCleaner.cpp @@ -0,0 +1,46 @@ +#include "DriverCleaner.hpp" + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +namespace spider::core { +DriverCleaner::DriverCleaner( + boost::uuids::uuid driver_id, + std::shared_ptr metadata_store, + std::shared_ptr storage_factory, + std::shared_ptr connection +) + : m_driver_id{driver_id}, + m_metadata_store{std::move(metadata_store)}, + m_storage_factory{std::move(storage_factory)}, + m_connection{std::move(connection)} {} + +DriverCleaner::~DriverCleaner() noexcept { + int const num_exceptions = std::uncaught_exceptions(); + // If destructor is called during stack unwinding, do not remove data reference. + if (num_exceptions > m_num_exceptions) { + return; + } + std::shared_ptr conn = m_connection; + if (nullptr == conn) { + std::variant, StorageErr> conn_result + = m_storage_factory->provide_storage_connection(); + // If we cannot get the connection, just wait for the heartbeat to fail + // and remove the driver. + if (std::holds_alternative(conn_result)) { + return; + } + conn = std::move(std::get>(conn_result)); + } + m_metadata_store->remove_driver(*conn, m_driver_id); +} +} // namespace spider::core diff --git a/src/spider/core/DriverCleaner.hpp b/src/spider/core/DriverCleaner.hpp new file mode 100644 index 000000000..32efb828c --- /dev/null +++ b/src/spider/core/DriverCleaner.hpp @@ -0,0 +1,54 @@ +#ifndef SPIDER_CORE_DRIVERCLEANER_HPP +#define SPIDER_CORE_DRIVERCLEANER_HPP + +#include +#include + +#include + +#include +#include +#include + +namespace spider::core { +/* + * Tracks the number of exceptions and ensures proper cleanup when the `Driver` + * object—holding a `unique_ptr` to this class—is destroyed. + * + * We avoid placing a custom destructor directly inside `Driver` because, when a + * `Driver` object is moved, its destructor is still invoked. This can result in + * double deletion. By using a `unique_ptr`, we ensure that destruction only + * occurs for the final owner, preventing such issues. + * + * We use std::uncaught_exceptions() to track the number of exceptions so that + * we don't clean up if the destructor is called during exception handling. + */ +class DriverCleaner { +public: + DriverCleaner( + boost::uuids::uuid driver_id, + std::shared_ptr metadata_store, + std::shared_ptr storage_factory, + std::shared_ptr connection + ); + + ~DriverCleaner() noexcept; + + // Delete copy constructor and assignment operator + DriverCleaner(DriverCleaner const&) = delete; + auto operator=(DriverCleaner const&) -> DriverCleaner& = delete; + // Default move constructor and assignment operator + DriverCleaner(DriverCleaner&&) = default; + auto operator=(DriverCleaner&&) -> DriverCleaner& = default; + +private: + int m_num_exceptions = std::uncaught_exceptions(); + + boost::uuids::uuid m_driver_id; + std::shared_ptr m_metadata_store; + std::shared_ptr m_storage_factory; + std::shared_ptr m_connection = nullptr; +}; +} // namespace spider::core + +#endif diff --git a/src/spider/core/JobCleaner.cpp b/src/spider/core/JobCleaner.cpp new file mode 100644 index 000000000..71996f4b0 --- /dev/null +++ b/src/spider/core/JobCleaner.cpp @@ -0,0 +1,46 @@ +#include "JobCleaner.hpp" + +#include +#include +#include +#include + +#include + +#include +#include +#include +#include + +namespace spider::core { +JobCleaner::JobCleaner( + boost::uuids::uuid job_id, + std::shared_ptr metadata_store, + std::shared_ptr storage_factory, + std::shared_ptr connection +) + : m_job_id{job_id}, + m_metadata_store{std::move(metadata_store)}, + m_storage_factory{std::move(storage_factory)}, + m_connection{std::move(connection)} {} + +JobCleaner::~JobCleaner() noexcept { + int const num_exceptions = std::uncaught_exceptions(); + // If destructor is called during stack unwinding, do not remove data reference. + if (num_exceptions > m_num_exceptions) { + return; + } + std::shared_ptr conn = m_connection; + if (nullptr == conn) { + std::variant, StorageErr> conn_result + = m_storage_factory->provide_storage_connection(); + // If we cannot get the connection, we cannot remove the job. + if (std::holds_alternative(conn_result)) { + return; + } + conn = std::move(std::get>(conn_result)); + } + m_metadata_store->remove_job(*conn, m_job_id); +} +} // namespace spider::core + diff --git a/src/spider/core/JobCleaner.hpp b/src/spider/core/JobCleaner.hpp new file mode 100644 index 000000000..320755da5 --- /dev/null +++ b/src/spider/core/JobCleaner.hpp @@ -0,0 +1,54 @@ +#ifndef SPIDER_CORE_JOB_CLEANER_HPP +#define SPIDER_CORE_JOB_CLEANER_HPP + +#include +#include + +#include + +#include +#include +#include + +namespace spider::core { +/* + * Tracks the number of exceptions and ensures proper cleanup when the `Job` + * object—holding a `unique_ptr` to this class—is destroyed. + * + * We avoid placing a custom destructor directly inside `Job` because, when a + * `Job` object is moved, its destructor is still invoked. This can result in + * double deletion. By using a `unique_ptr`, we ensure that destruction only + * occurs for the final owner, preventing such issues. + * + * We use std::uncaught_exceptions() to track the number of exceptions so that + * we don't clean up if the destructor is called during exception handling. + */ +class JobCleaner { +public: + JobCleaner( + boost::uuids::uuid job_id, + std::shared_ptr metadata_store, + std::shared_ptr storage_factory, + std::shared_ptr connection + ); + + ~JobCleaner() noexcept; + + // Delete copy constructor and assignment operator + JobCleaner(JobCleaner const&) = delete; + auto operator=(JobCleaner const&) -> JobCleaner& = delete; + // Default move constructor and assignment operator + JobCleaner(JobCleaner&&) = default; + auto operator=(JobCleaner&&) -> JobCleaner& = default; + +private: + int m_num_exceptions = std::uncaught_exceptions(); + + boost::uuids::uuid m_job_id; + std::shared_ptr m_metadata_store; + std::shared_ptr m_storage_factory; + std::shared_ptr m_connection = nullptr; +}; +} // namespace spider::core + +#endif diff --git a/src/spider/scheduler/scheduler.cpp b/src/spider/scheduler/scheduler.cpp index 8679eab4c..8629f3b2b 100644 --- a/src/spider/scheduler/scheduler.cpp +++ b/src/spider/scheduler/scheduler.cpp @@ -273,5 +273,7 @@ auto main(int argc, char** argv) -> int { return cSignalExitBase + SIGTERM; } + metadata_store->remove_driver(*conn, scheduler_id); + return 0; } diff --git a/src/spider/storage/DataStorage.hpp b/src/spider/storage/DataStorage.hpp index c9c88f56c..e2be19848 100644 --- a/src/spider/storage/DataStorage.hpp +++ b/src/spider/storage/DataStorage.hpp @@ -74,7 +74,7 @@ class DataStorage { StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid task_id - ) -> StorageErr + ) noexcept -> StorageErr = 0; virtual auto add_driver_reference( StorageConnection& conn, @@ -86,7 +86,7 @@ class DataStorage { StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid driver_id - ) -> StorageErr + ) noexcept -> StorageErr = 0; virtual auto remove_dangling_data(StorageConnection& conn) -> StorageErr = 0; diff --git a/src/spider/storage/MetadataStorage.hpp b/src/spider/storage/MetadataStorage.hpp index dca6b4466..39a3e4375 100644 --- a/src/spider/storage/MetadataStorage.hpp +++ b/src/spider/storage/MetadataStorage.hpp @@ -29,6 +29,9 @@ class MetadataStorage { virtual auto add_driver(StorageConnection& conn, Driver const& driver) -> StorageErr = 0; virtual auto add_scheduler(StorageConnection& conn, Scheduler const& scheduler) -> StorageErr = 0; + virtual auto remove_driver(StorageConnection& conn, boost::uuids::uuid id) noexcept + -> StorageErr + = 0; virtual auto get_active_scheduler(StorageConnection& conn, std::vector* schedulers) -> StorageErr = 0; @@ -72,7 +75,8 @@ class MetadataStorage { std::vector* job_ids ) -> StorageErr = 0; - virtual auto remove_job(StorageConnection& conn, boost::uuids::uuid id) -> StorageErr = 0; + virtual auto remove_job(StorageConnection& conn, boost::uuids::uuid id) noexcept -> StorageErr + = 0; virtual auto reset_job(StorageConnection& conn, boost::uuids::uuid id) -> StorageErr = 0; virtual auto add_child(StorageConnection& conn, boost::uuids::uuid parent_id, Task const& child) -> StorageErr diff --git a/src/spider/storage/mysql/MySqlStorage.cpp b/src/spider/storage/mysql/MySqlStorage.cpp index bb23bf7b9..a7c321d9e 100644 --- a/src/spider/storage/mysql/MySqlStorage.cpp +++ b/src/spider/storage/mysql/MySqlStorage.cpp @@ -218,6 +218,29 @@ auto MySqlMetadataStorage::get_active_scheduler( return StorageErr{}; } +auto +MySqlMetadataStorage::remove_driver(StorageConnection& conn, boost::uuids::uuid const id) noexcept + -> StorageErr { + try { + std::unique_ptr statement( + static_cast(conn)->prepareStatement( + "DELETE FROM `drivers` WHERE `id` = ?" + ) + ); + sql::bytes id_bytes = uuid_get_bytes(id); + statement->setBytes(1, &id_bytes); + statement->executeUpdate(); + } catch (sql::SQLException& e) { + static_cast(conn)->rollback(); + if (e.getErrorCode() == ErKeyNotFound) { + return StorageErr{StorageErrType::KeyNotFoundErr, e.what()}; + } + return StorageErr{StorageErrType::OtherErr, e.what()}; + } + static_cast(conn)->commit(); + return StorageErr{}; +} + void MySqlMetadataStorage::add_task( MySqlConnection& conn, sql::bytes job_id, @@ -1053,7 +1076,7 @@ auto MySqlMetadataStorage::get_jobs_by_client_id( return StorageErr{}; } -auto MySqlMetadataStorage::remove_job(StorageConnection& conn, boost::uuids::uuid id) +auto MySqlMetadataStorage::remove_job(StorageConnection& conn, boost::uuids::uuid id) noexcept -> StorageErr { try { std::unique_ptr statement( @@ -2275,7 +2298,7 @@ auto MySqlDataStorage::remove_task_reference( StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid task_id -) -> StorageErr { +) noexcept -> StorageErr { try { std::unique_ptr statement( static_cast(conn)->prepareStatement( @@ -2326,7 +2349,7 @@ auto MySqlDataStorage::remove_driver_reference( StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid driver_id -) -> StorageErr { +) noexcept -> StorageErr { try { std::unique_ptr statement( static_cast(conn)->prepareStatement( diff --git a/src/spider/storage/mysql/MySqlStorage.hpp b/src/spider/storage/mysql/MySqlStorage.hpp index ecf96ee5b..90808dac4 100644 --- a/src/spider/storage/mysql/MySqlStorage.hpp +++ b/src/spider/storage/mysql/MySqlStorage.hpp @@ -38,6 +38,8 @@ class MySqlMetadataStorage : public MetadataStorage { auto initialize(StorageConnection& conn) -> StorageErr override; auto add_driver(StorageConnection& conn, Driver const& driver) -> StorageErr override; auto add_scheduler(StorageConnection& conn, Scheduler const& scheduler) -> StorageErr override; + auto remove_driver(StorageConnection& conn, boost::uuids::uuid id) noexcept + -> StorageErr override; auto get_active_scheduler(StorageConnection& conn, std::vector* schedulers) -> StorageErr override; auto add_job( @@ -71,7 +73,7 @@ class MySqlMetadataStorage : public MetadataStorage { boost::uuids::uuid client_id, std::vector* job_ids ) -> StorageErr override; - auto remove_job(StorageConnection& conn, boost::uuids::uuid id) -> StorageErr override; + auto remove_job(StorageConnection& conn, boost::uuids::uuid id) noexcept -> StorageErr override; auto reset_job(StorageConnection& conn, boost::uuids::uuid id) -> StorageErr override; auto add_child(StorageConnection& conn, boost::uuids::uuid parent_id, Task const& child) -> StorageErr override; @@ -169,7 +171,7 @@ class MySqlDataStorage : public DataStorage { StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid task_id - ) -> StorageErr override; + ) noexcept -> StorageErr override; auto add_driver_reference( StorageConnection& conn, boost::uuids::uuid id, @@ -179,7 +181,7 @@ class MySqlDataStorage : public DataStorage { StorageConnection& conn, boost::uuids::uuid id, boost::uuids::uuid driver_id - ) -> StorageErr override; + ) noexcept -> StorageErr override; auto remove_dangling_data(StorageConnection& conn) -> StorageErr override; auto add_client_kv_data(StorageConnection& conn, KeyValueData const& data) diff --git a/src/spider/worker/FunctionManager.hpp b/src/spider/worker/FunctionManager.hpp index 7a5982b60..3ba6369d6 100644 --- a/src/spider/worker/FunctionManager.hpp +++ b/src/spider/worker/FunctionManager.hpp @@ -314,6 +314,7 @@ class FunctionInvoker { std::get(args_tuple) = DataImpl::create_data>( std::move(data), + Context{Context::Source::Task, task_id}, data_store, TaskContextImpl::get_storage_factory(context) ); diff --git a/tests/scheduler/test-SchedulerPolicy.cpp b/tests/scheduler/test-SchedulerPolicy.cpp index 167bc78ea..c246a32f0 100644 --- a/tests/scheduler/test-SchedulerPolicy.cpp +++ b/tests/scheduler/test-SchedulerPolicy.cpp @@ -93,6 +93,9 @@ TEMPLATE_LIST_TEST_CASE( // Schedule when no task available optional_task_id = policy.schedule_next(gen(), ""); REQUIRE(!optional_task_id.has_value()); + + // Clean up + REQUIRE(metadata_store->remove_driver(*conn, scheduler_id).success()); } TEMPLATE_LIST_TEST_CASE( @@ -150,6 +153,9 @@ TEMPLATE_LIST_TEST_CASE( } REQUIRE(metadata_store->remove_job(*conn, job_id).success()); + REQUIRE(data_store->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_store->remove_driver(*conn, scheduler_id).success()); + REQUIRE(metadata_store->remove_driver(*conn, client_id).success()); } TEMPLATE_LIST_TEST_CASE( @@ -205,6 +211,9 @@ TEMPLATE_LIST_TEST_CASE( } REQUIRE(metadata_store->remove_job(*conn, job_id).success()); + REQUIRE(data_store->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_store->remove_driver(*conn, scheduler_id).success()); + REQUIRE(metadata_store->remove_driver(*conn, client_id).success()); } } // namespace diff --git a/tests/scheduler/test-SchedulerServer.cpp b/tests/scheduler/test-SchedulerServer.cpp index c51070bb3..30931f5d1 100644 --- a/tests/scheduler/test-SchedulerServer.cpp +++ b/tests/scheduler/test-SchedulerServer.cpp @@ -117,6 +117,9 @@ TEMPLATE_LIST_TEST_CASE( } socket.close(); server.stop(); + + // Clean up + REQUIRE(metadata_store->remove_driver(*conn, scheduler_id).success()); } } // namespace diff --git a/tests/storage/test-DataStorage.cpp b/tests/storage/test-DataStorage.cpp index 71fb009cd..e415ed68f 100644 --- a/tests/storage/test-DataStorage.cpp +++ b/tests/storage/test-DataStorage.cpp @@ -62,6 +62,9 @@ TEMPLATE_LIST_TEST_CASE( // Get data should fail REQUIRE(spider::core::StorageErrType::KeyNotFoundErr == data_storage->get_data(*conn, data.get_id(), &result).type); + + // Clean up + REQUIRE(metadata_storage->remove_driver(*conn, driver_id).success()); } TEMPLATE_LIST_TEST_CASE( @@ -100,6 +103,10 @@ TEMPLATE_LIST_TEST_CASE( auto err = data_storage->get_client_kv_data(*conn, driver_id, "key", &value); REQUIRE(data_storage->get_client_kv_data(*conn, driver_id, "key", &value).success()); REQUIRE(data.get_value() == value); + + // Clean up + REQUIRE(data_storage->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_storage->remove_driver(*conn, driver_id).success()); } TEMPLATE_LIST_TEST_CASE( @@ -245,6 +252,11 @@ TEMPLATE_LIST_TEST_CASE( // Remove driver reference REQUIRE(data_storage->remove_driver_reference(*conn, data.get_id(), driver_id_2).success()); + + // Clean up + REQUIRE(data_storage->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_storage->remove_driver(*conn, driver_id).success()); + REQUIRE(metadata_storage->remove_driver(*conn, driver_id_2).success()); } } // namespace diff --git a/tests/storage/test-MetadataStorage.cpp b/tests/storage/test-MetadataStorage.cpp index 254fcc11e..40ce74723 100644 --- a/tests/storage/test-MetadataStorage.cpp +++ b/tests/storage/test-MetadataStorage.cpp @@ -69,6 +69,9 @@ TEMPLATE_LIST_TEST_CASE("Driver heartbeat", "[storage]", spider::test::StorageFa REQUIRE(std::ranges::none_of(ids, [&driver_id](boost::uuids::uuid id) { return id == driver_id; })); + + // Clean up + REQUIRE(storage->remove_driver(*conn, driver_id).success()); } TEMPLATE_LIST_TEST_CASE("Scheduler addr", "[storage]", spider::test::StorageFactoryTypeList) { @@ -100,6 +103,9 @@ TEMPLATE_LIST_TEST_CASE("Scheduler addr", "[storage]", spider::test::StorageFact // Get non-exist scheduler should fail REQUIRE(spider::core::StorageErrType::KeyNotFoundErr == storage->get_scheduler_addr(*conn, gen(), &addr_res, &port_res).type); + + // Clean up + REQUIRE(storage->remove_driver(*conn, scheduler_id).success()); } TEMPLATE_LIST_TEST_CASE( @@ -593,6 +599,7 @@ TEMPLATE_LIST_TEST_CASE( // Clean up REQUIRE(storage->remove_job(*conn, job_id).success()); + REQUIRE(storage->remove_driver(*conn, scheduler_id).success()); } } // namespace diff --git a/tests/worker/test-FunctionManager.cpp b/tests/worker/test-FunctionManager.cpp index d89ea7a95..2749b9271 100644 --- a/tests/worker/test-FunctionManager.cpp +++ b/tests/worker/test-FunctionManager.cpp @@ -219,6 +219,7 @@ TEMPLATE_LIST_TEST_CASE( REQUIRE(metadata_storage->remove_job(*conn, job_id).success()); REQUIRE(data_storage->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_storage->remove_driver(*conn, driver_id).success()); } } // namespace diff --git a/tests/worker/test-TaskExecutor.cpp b/tests/worker/test-TaskExecutor.cpp index 1cca3cadf..d7d279399 100644 --- a/tests/worker/test-TaskExecutor.cpp +++ b/tests/worker/test-TaskExecutor.cpp @@ -221,6 +221,7 @@ TEMPLATE_LIST_TEST_CASE( // Clean up REQUIRE(metadata_storage->remove_job(*conn, job_id).success()); REQUIRE(data_storage->remove_data(*conn, data.get_id()).success()); + REQUIRE(metadata_storage->remove_driver(*conn, driver_id).success()); } constexpr int cLargeInputSize = 300;