diff --git a/test/src/unit-Reader.cc b/test/src/unit-Reader.cc index 8b5242ca9bb..c3e0750390c 100644 --- a/test/src/unit-Reader.cc +++ b/test/src/unit-Reader.cc @@ -175,7 +175,7 @@ TEST_CASE_METHOD( array.memory_tracker(), tracker_, lq_state_machine, - CancellationSource(context.storage_manager()), + context.make_cancellation_source(), array.opened_array(), config, nullopt, diff --git a/test/src/unit-enumerations.cc b/test/src/unit-enumerations.cc index 482496bd76a..5c7d24d3973 100644 --- a/test/src/unit-enumerations.cc +++ b/test/src/unit-enumerations.cc @@ -2462,18 +2462,10 @@ TEST_CASE_METHOD( auto qc1 = create_qc("attr1", (int)2, QueryConditionOp::EQ); qc1.set_use_enumeration(false); - Query q1( - ctx_.resources(), - ctx_.cancellation_source(), - ctx_.storage_manager(), - array); + Query q1(ctx_, array); throw_if_not_ok(q1.set_condition(qc1)); - Query q2( - ctx_.resources(), - ctx_.cancellation_source(), - ctx_.storage_manager(), - array); + Query q2(ctx_, array); ser_des_query(&q1, &q2, client_side, ser_type); auto qc2 = q2.condition(); @@ -2506,18 +2498,10 @@ TEST_CASE_METHOD( throw_if_not_ok(qc1.combine(qc2, QueryConditionCombinationOp::OR, &qc3)); - Query q1( - ctx_.resources(), - ctx_.cancellation_source(), - ctx_.storage_manager(), - array); + Query q1(ctx_, array); throw_if_not_ok(q1.set_condition(qc3)); - Query q2( - ctx_.resources(), - ctx_.cancellation_source(), - ctx_.storage_manager(), - array); + Query q2(ctx_, array); ser_des_query(&q1, &q2, client_side, ser_type); auto qc4 = q2.condition(); diff --git a/tiledb/CMakeLists.txt b/tiledb/CMakeLists.txt index 49d4e203cf2..16eb528c9be 100644 --- a/tiledb/CMakeLists.txt +++ b/tiledb/CMakeLists.txt @@ -304,6 +304,7 @@ set(TILEDB_CORE_SOURCES ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/storage_manager/cancellation_source.cc ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/storage_manager/context.cc ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/storage_manager/context_resources.cc + ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/storage_manager/job.cc ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/storage_manager/storage_manager.cc ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/subarray/range_subset.cc ${TILEDB_CORE_INCLUDE_DIR}/tiledb/sm/subarray/relevant_fragment_generator.cc diff --git a/tiledb/api/c_api/context/context_api.cc b/tiledb/api/c_api/context/context_api.cc index 64ef47825b2..9c1d2ca5e4e 100644 --- a/tiledb/api/c_api/context/context_api.cc +++ b/tiledb/api/c_api/context/context_api.cc @@ -115,7 +115,7 @@ capi_return_t tiledb_ctx_is_supported_fs( } capi_return_t tiledb_ctx_cancel_tasks(tiledb_ctx_t* ctx) { - throw_if_not_ok(ctx->storage_manager()->cancel_all_tasks()); + ctx->cancel_all_tasks(); return TILEDB_OK; } diff --git a/tiledb/api/c_api/context/context_api_internal.h b/tiledb/api/c_api/context/context_api_internal.h index 4a7866123fc..0f23ede6b06 100644 --- a/tiledb/api/c_api/context/context_api_internal.h +++ b/tiledb/api/c_api/context/context_api_internal.h @@ -68,12 +68,8 @@ struct tiledb_ctx_handle_t return ctx_.resources().config(); } - inline tiledb::sm::StorageManager* storage_manager() { - return ctx_.storage_manager(); - } - - inline tiledb::sm::CancellationSource cancellation_source() { - return ctx_.cancellation_source(); + inline void cancel_all_tasks() { + return ctx_.cancel_all_tasks(); } inline tiledb::sm::RestClient& rest_client() { diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index 03e56d14389..5b92abff29e 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -496,7 +496,7 @@ capi_return_t tiledb_group_consolidate_metadata( ensure_group_uri_argument_is_valid(group_uri); auto cfg = (config == nullptr) ? ctx->config() : config->config(); - tiledb::sm::Group::consolidate_metadata(ctx->resources(), group_uri, cfg); + tiledb::sm::Group::consolidate_metadata(ctx->context(), group_uri, cfg); return TILEDB_OK; } @@ -506,7 +506,7 @@ capi_return_t tiledb_group_vacuum_metadata( ensure_group_uri_argument_is_valid(group_uri); auto cfg = (config == nullptr) ? ctx->config() : config->config(); - sm::Group::vacuum_metadata(ctx->resources(), group_uri, cfg); + sm::Group::vacuum_metadata(ctx->context(), group_uri, cfg); return TILEDB_OK; } diff --git a/tiledb/api/c_api_test_support/storage_manager_stub/storage_manager_override.h b/tiledb/api/c_api_test_support/storage_manager_stub/storage_manager_override.h index 7bf7fa8a59e..bde6f5282c6 100644 --- a/tiledb/api/c_api_test_support/storage_manager_stub/storage_manager_override.h +++ b/tiledb/api/c_api_test_support/storage_manager_stub/storage_manager_override.h @@ -48,15 +48,15 @@ class StorageManagerStub { public: static constexpr bool is_overriding_class = true; - StorageManagerStub( - ContextResources&, - const std::shared_ptr&, - const Config& config) + StorageManagerStub(ContextResources&, const Config& config) : config_(config) { } - inline Status cancel_all_tasks() { - return Status{}; + void cancel_all_tasks() { + } + + bool cancellation_in_progress() const { + return false; } }; diff --git a/tiledb/common/CMakeLists.txt b/tiledb/common/CMakeLists.txt index bcfe1f394da..a19a7e3dd61 100644 --- a/tiledb/common/CMakeLists.txt +++ b/tiledb/common/CMakeLists.txt @@ -44,6 +44,7 @@ add_subdirectory(exception) add_subdirectory(governor) add_subdirectory(interval) add_subdirectory(random) +add_subdirectory(registry) add_subdirectory(thread_pool) add_subdirectory(types) add_subdirectory(util) diff --git a/tiledb/common/registry/CMakeLists.txt b/tiledb/common/registry/CMakeLists.txt new file mode 100644 index 00000000000..956a85570b5 --- /dev/null +++ b/tiledb/common/registry/CMakeLists.txt @@ -0,0 +1,26 @@ +# +# tiledb/common/registry/CMakeLists.txt +# +# The MIT License +# +# Copyright (c) 2023-2024 TileDB, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +add_test_subdirectory() diff --git a/tiledb/common/registry/registry.h b/tiledb/common/registry/registry.h new file mode 100644 index 00000000000..0028ce0ef1f --- /dev/null +++ b/tiledb/common/registry/registry.h @@ -0,0 +1,480 @@ +/** + * @file tiledb/common/registry/registry.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2017-2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file defines the registry classes. As they are all template classes, the + * full definitions are herein. + */ + +#ifndef TILEDB_REGISTRY_H +#define TILEDB_REGISTRY_H + +#include +#include +#include +#include +#include +#include + +namespace tiledb::common { + +// Forward declaration for RegistryEntry +template +class Registry; + +/** + * An entry contained with an instance of `class Registry`. + * + * A registry entry allows an item to refer to itself directly without + * requiring a container look-up. + * + * This class is declared as `struct` because it's an internal type only + * visible to `class Registry` and `class RegistryHandle`. Since those two + * classes are friends, there's no particular benefit to private members here. + */ +template +struct RegistryEntry { + using registry_type = Registry; + + /** + * The item that this entry refers to + * + * Note that this reference alone does not guarantee validity of the referent + * object, which might not have returned from its constructor yet. + */ + T& item_; + + /** + * A non-owning pointer to the item + * + * This member variable is default-initialized to owning nothing. It's + * assigned to directly in `RegistryHandle::register_shared_ptr`. + */ + std::weak_ptr item_ptr_{}; + + /** + * The registry that this entry occurs within + * + * @section Design + * + * It might seem wasteful to keep a reference to a registry as a member + * variable here, since instances of this type are held only in a container of + * that same registry. If it weren't present here, it would have to be present + * somehow within items of `class T`, either explicitly as a member variable + * or implicitly as a member of a handle member variable (of a different type + * than used here). + * + * This decision puts all the complexity of implementation on one side of the + * interface. Registered item instances only need to do two things: + * - Define a handle member variable + * - Initialize that handle in the constructor by registering itself + * Removal of an item from the registry will then happen automatically during + * the destruction of its corresponding handle. + */ + registry_type& registry_; + + /** + * The type to which this handle refers + */ + using value_type = T; + + /** + * There's no item handle without an item. + */ + RegistryEntry() = delete; + + /** + * Ordinary constructor + * + * @param i The item to register + * @param r The registry that will contain the item + */ + RegistryEntry(T& i, registry_type& r) + : item_(i) + , registry_(r) { + } + + /** + * The destructor is default + */ + ~RegistryEntry() = default; +}; + +// Forward for `class RegistryHandle` +template +class RegistryHandle; + +/** + * A synchronized registry + * + * The registry holds references to its registered items, not the items + * themselves. + */ +template +class Registry { + /** + * `class RegistryHandle` is a friend because it needs to know the iterator + * type of the container used to implement the registry + */ + friend class RegistryHandle; + + /** + * Mutex that synchronizes `registry_list_` + */ + mutable std::mutex m_; + + /** + * Condition variable used to implement alarms on registry size. We notify + * every time we change the size of the registry. + */ + mutable std::condition_variable cv_; + + /** + * The type of the scope-based lock used throughout the implementation. This + * class only rarely does anything complicated enough to need something more. + */ + using lock_guard_type = std::lock_guard; + + /** + * The type of the container used to hold registry entries. + */ + using registry_list_type = std::list>; + + /** + * The iterator type is only visible to friends. + */ + using registry_iterator_type = typename registry_list_type::iterator; + + /** + * List of entries with this registry + * + * Note that presence of an entry within this list does _not_ guarantee + * validity of the item within the entry. The reason for this is that the + * handle, as a member variable of the item, is initialized within its + * constructor. Since creation of a handle requires adding an entry to this + * container, entries appear before their constructors return. + * + * This property is necessarily true of any registration system that operates + * at construction time. While it's an inherent limitation, it's better than + * performing all registration outside the class and relying on user code to + * be fully reliable. All that's necessary to complete registration is a + * single call on the handle after the constructor returns. In the present + * case that call is `RegistryHandle::register_shared_ptr`. + * + * It's also worth noting that entries within this container never refer to + * stale objects, ones whose destructors have already run. That's because the + * handle, as a member variable, is destroyed before the destructor returns. + */ + registry_list_type registry_list_; + + public: + /** + * The type of the item within a registry entry. + */ + using value_type = T; + + /** + * The return type of `size()` is forwarded from its underlying container. + */ + using size_type = typename registry_list_type::size_type; + + /** + * The handle type. A registered class should have a member variable of this + * type for auto-deregistration at destruction. + */ + using registry_handle_type = RegistryHandle; + + /** + * Create an entry within this registry + * + * @param item A reference to the item to be entered + * @return A handle to the new entry within this registry + * @postcondition The item within the return value refers to the argument + */ + const registry_handle_type register_item(value_type& item) { + lock_guard_type lg(m_); + registry_list_.emplace_back(item, *this); + cv_.notify_all(); // `emplace_back` adds one to the registry size + /* + * `emplace_back` returns a reference to the list member, not its iterator. + * We have to construct an iterator instead. + */ + return registry_handle_type(--registry_list_.end()); // mandatory RVO + } + + /** + * The current number of entries within the registry + * + * @section Caution + * + * This isn't a high-performance function, because it acquires a lock for + * every call. It's not advised to poll this function. + */ + size_type size() const noexcept { + /* + * We're locking because the standard library does not offer a guarantee + * that `size()` is atomic + */ + lock_guard_type lg(m_); + return registry_list_.size(); + } + + /** + * Block until the registry is empty. + */ + void wait_for_empty() const { + std::unique_lock lck(m_); + cv_.wait(lck, [this]() { return registry_list_.size() == 0; }); + } + + /** + * Iterate over each element and apply a function to each. + * + * Caution: This function holds a lock during the operation. For long-running + * operations, use this function only to dispatch to other threads. + * + * @tparam F A unary function `void f(value_type &)` + * @param f A callable object of type F + */ + template + void for_each(F f) const { + lock_guard_type lg(m_); + /* + * We iterate over entries, but apply the function to valid items. Thus we + * need to provide `for_each` with an adapter. + */ + auto g{[&f](const RegistryEntry& entry) -> void { + /* + * We can only iterate over items that have registered `shared_ptr`. + * Otherwise we might iterate over a dangling reference. + */ + auto item_ptr{entry.item_ptr_.lock()}; + if (item_ptr) { + f(*item_ptr); + } + }}; + std::for_each(registry_list_.begin(), registry_list_.end(), g); + } + + private: + /** + * Remove an entry from this registry. + * + * @section Design + * + * This function is private so that it's called only by `RegistryHandle`, + * which does so in its destructor. Calling this function in any other + * circumstance would invalidate the iterator held inside of the handle. This + * would cause a violation of the class invariant of `RegistryHandle` that + * the handle is always valid. + * + * @param iter Iterator to the entry to remove from this registry. + */ + void deregister(registry_iterator_type& iter) { + lock_guard_type lg(m_); + registry_list_.erase(iter); + cv_.notify_all(); // `erase` subtracts one from the registry size + } +}; + +/** + * Handle for an entry within a registry + * + * Handles are values and should be passed by value, not by reference and not + * through a pointer. The handle encapsulates a referential object whose details + * are opaque outside the class. + * + * @section Design + * + * The class invariant that the handle always refers to something might look + * bland, but it's crucial. The invariant says that we may have neither empty + * handles (referring to no object) nor dangling handles (referring to a + * destroyed object). + * + * @invariant The handle refers to an item inside the registry. + * @tparam T class whose instances have entries within a registry + */ +template +class RegistryHandle { + /* + * `class Registry` is a friend so that `class T` sees only the public + * interface of this handle class, and in particular not its constructor. + * The registration function of the registry acts as a factory for this class + * and is the only way to construct objects of this type. + */ + friend class Registry; + + /** + * This class is a wrapper around an instance of this referential type. + * + * The type is an iterator into the internal container of the registry. At the + * end of the duration where an item is registered, an iterator allows + * immediate erasure of the entry; there's no separate index or other + * secondary structure required. + */ + using referential_type = typename Registry::registry_iterator_type; + + /** + * The iterator that comprises the underlying data of this handle. + */ + referential_type iter_; + + public: + /** + * Default constructor is deleted. + * + * There's no such this as a registry handle without a corresponding entry in + * the registry. + */ + RegistryHandle() = delete; + + private: + /** + * Ordinary constructor is ordinary. + * + * @param iter An iterator to the internal container of a registry + */ + explicit RegistryHandle(const referential_type& iter) + : iter_(iter) { + } + + public: + /** + * Destroying a handle removes its corresponding entry in its origin registry. + */ + ~RegistryHandle() { + iter_->registry_.deregister(iter_); + } + + /** + * Register a shared_ptr to an item already in the registry. + * + * Registration is optional for construction of a handle, but required to + * access the original item. This is a consequence of the life cycle + * assumptions, since the handle can exist before the construction of its item + * has completed. + * + * Note that we can't write a postcondition that the item has a shared_ptr + * already registered to it. That's to account for the possible, albeit very + * unlikely, situation that (1) the destructor for the shared_ptr that the one + * that the argument was copied from runs, and (2) that it was the last + * shared_ptr to its referent, and thus (3) that the use count on the control + * block goes to zero before the call returns. + * + * @pre This function has not been called before on this handle + * @pre `ptr` is not null + * @pre The `ptr` argument must point to the same object as the item that this + * handle refers to + * @param ptr A shared_ptr to (putatively) the same item that's in the handle + * @throws std::logic_error if there's already a shared_ptr registered + * @throws std::invalid_argument if the precondition is not met + */ + void register_shared_ptr(std::shared_ptr ptr) { + /* + * Note that this code is implemented here, in the handle class, rather than + * in the entry class itself, where it would be more natural. This is an + * optimization to avoid a possible copy of the `shared_ptr` argument. + */ + if (iter_->item_ptr_.use_count() != 0) { + /* + * This checks for a precondition failure, but it's not a full validation + * of the precondition that the function has not been called. That's + * because this handle does not have a mutex; if the function were to be + * called in two different threads this function can fail. + * + * The implementation decision here is that it's not worth the expense of + * a mutex in order to guard against an active attempt to violate the + * precondition. + */ + throw std::logic_error( + "May not register a shared_ptr twice on the same handle"); + } + if (!ptr) { + throw std::invalid_argument("May not register a null shared_ptr"); + } + if (std::addressof(*ptr) != std::addressof(iter_->item_)) { + throw std::invalid_argument( + "Argument does not refer to the same object that this handle does"); + } + iter_->item_ptr_ = ptr; + } + + /** + * Release any reference held within the registry. + * + * This function is an inverse to `register_shared_ptr`; it takes the entry + * back to its default state of not referring to any `shared_ptr`. + * + * @section Caution + * + * This function is not needed in the typical use case of the registry where + * objects are registered immediately after construction and automatically + * removed from the registry when destroyed. In this case existence within the + * registry and accessibility to the object are basically, though with + * caveats, coterminous. This function would be required, for example, if the + * same object is going to have a `shared_ptr` registered, reset, and then + * registered again. + */ + void reset() noexcept { + iter_->iter_ptr_.reset(); + } + + /** + * Access the underlying item, if any. + * + * This function always returns a `shared_ptr`, but it does not always return + * a non-empty pointer. If the returned pointer is empty, it's ambiguous + * between two possibilities: + * - Beginning of life span. The item never had a `shared_ptr` registered in + * the first place. + * - End of life span. The original `shared_ptr` and any copies have all + * been destroyed. Note that the destructor for the underlying object + * might not have completed yet; if not, it will be pending in some other + * thread. + * + * If the returned `shared_ptr` is non-empty, then the underlying object is + * guaranteed to exist. That doesn't always mean, however, that there's any + * other `shared_ptr` keeping the referent alive. The return value cam be the + * last `shared_ptr` to the referent. + * + * @section Design + * + * This function returns a `shared_ptr` rather than `T&` so that it can + * never return a dangling reference. The existence of a handle does not + * guarantee existence of any underlying object. + * + * @return A shared_ptr to the underlying item of the handle. + */ + std::shared_ptr get() noexcept { + return iter_->item_ptr_.lock(); + } +}; + +} // namespace tiledb::common + +#endif // TILEDB_REGISTRY_H diff --git a/tiledb/common/registry/test/CMakeLists.txt b/tiledb/common/registry/test/CMakeLists.txt new file mode 100644 index 00000000000..f9728753af6 --- /dev/null +++ b/tiledb/common/registry/test/CMakeLists.txt @@ -0,0 +1,34 @@ +# +# tiledb/common/registry/test/CMakeLists.txt +# +# The MIT License +# +# Copyright (c) 2024 TileDB, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +include(unit_test) + +# +# Test runner `unit_registry` +# +commence(unit_test registry) + this_target_sources(unit_registry.cc) + # no object library; registry is header-only +conclude(unit_test) diff --git a/tiledb/common/registry/test/unit_registry.cc b/tiledb/common/registry/test/unit_registry.cc new file mode 100644 index 00000000000..83f785313f7 --- /dev/null +++ b/tiledb/common/registry/test/unit_registry.cc @@ -0,0 +1,114 @@ +/* + * @file tiledb/common/registry/test/unit_registry.cpp + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file provides unit tests for `template class Registry`. + */ + +#include + +#include "../registry.h" + +struct Item; +using item_registry_type = tiledb::common::Registry; +using item_handle_type = item_registry_type::registry_handle_type; + +struct Item { + item_handle_type handle_; + + explicit Item(item_registry_type& registry) + : handle_(registry.register_item(*this)) { + } + + void register_shared_ptr(std::shared_ptr p) { + handle_.register_shared_ptr(p); + } +}; + +/** + * Check that (1) the size of the registry argument matches the size argument + * and (2) that the size found through iteration also matches. + */ +void check_size(const item_registry_type& r, item_registry_type::size_type n) { + CHECK(r.size() == n); + item_registry_type::size_type m{0}; + auto f{[&m](const item_registry_type::value_type&) -> void { ++m; }}; + r.for_each(f); + CHECK(r.size() == m); +} + +TEST_CASE("Registry - construct") { + item_registry_type r; +} + +TEST_CASE("Registry - construct and add, single") { + item_registry_type r; + CHECK(r.size() == 0); + { + Item i{r}; + CHECK(r.size() == 1); + } + CHECK(r.size() == 0); +} + +TEST_CASE("Registry - construct and add, two nested") { + item_registry_type r; + CHECK(r.size() == 0); + { + Item i{r}; + CHECK(r.size() == 1); + { + Item i2{r}; + CHECK(r.size() == 2); + } + CHECK(r.size() == 1); + } + CHECK(r.size() == 0); +} + +/* + * This test also exercises `register_shared_ptr` and `for_each`. + */ +TEST_CASE("Registry - construct and add, two interleaved") { + item_registry_type r; + check_size(r, 0); + { + auto i1{std::make_shared(r)}; + i1->register_shared_ptr(i1); + check_size(r, 1); + { + auto i2{std::make_shared(r)}; + i2->register_shared_ptr(i2); + check_size(r, 2); + i1.reset(); + check_size(r, 1); + } + check_size(r, 0); + } + check_size(r, 0); +} diff --git a/tiledb/sm/c_api/tiledb.cc b/tiledb/sm/c_api/tiledb.cc index 71ac3033e7d..90c13573462 100644 --- a/tiledb/sm/c_api/tiledb.cc +++ b/tiledb/sm/c_api/tiledb.cc @@ -418,11 +418,8 @@ int32_t tiledb_query_alloc( } // Create query - (*query)->query_ = new (std::nothrow) tiledb::sm::Query( - ctx->resources(), - ctx->cancellation_source(), - ctx->storage_manager(), - array->array_); + (*query)->query_ = + new (std::nothrow) tiledb::sm::Query(ctx->context(), array->array_); if ((*query)->query_ == nullptr) { auto st = Status_Error( "Failed to allocate TileDB query object; Memory allocation failed"); @@ -1711,13 +1708,12 @@ int32_t tiledb_array_consolidate( tiledb_ctx_t* ctx, const char* array_uri, tiledb_config_t* config) { api::ensure_config_is_valid_if_present(config); tiledb::sm::Consolidator::array_consolidate( - ctx->resources(), + ctx->context(), array_uri, tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0, - (config == nullptr) ? ctx->config() : config->config(), - ctx->storage_manager()); + (config == nullptr) ? ctx->config() : config->config()); return TILEDB_OK; } @@ -1737,14 +1733,13 @@ int32_t tiledb_array_consolidate_fragments( } tiledb::sm::Consolidator::fragments_consolidate( - ctx->resources(), + ctx->context(), array_uri, tiledb::sm::EncryptionType::NO_ENCRYPTION, nullptr, 0, uris, - (config == nullptr) ? ctx->config() : config->config(), - ctx->storage_manager()); + (config == nullptr) ? ctx->config() : config->config()); return TILEDB_OK; } @@ -1752,10 +1747,9 @@ int32_t tiledb_array_consolidate_fragments( int32_t tiledb_array_vacuum( tiledb_ctx_t* ctx, const char* array_uri, tiledb_config_t* config) { tiledb::sm::Consolidator::array_vacuum( - ctx->resources(), + ctx->context(), array_uri, - (config == nullptr) ? ctx->config() : config->config(), - ctx->storage_manager()); + (config == nullptr) ? ctx->config() : config->config()); return TILEDB_OK; } @@ -2751,11 +2745,8 @@ int32_t tiledb_deserialize_query_and_array( } // Create query - (*query)->query_ = new (std::nothrow) tiledb::sm::Query( - ctx->resources(), - ctx->cancellation_source(), - ctx->storage_manager(), - (*array)->array_); + (*query)->query_ = + new (std::nothrow) tiledb::sm::Query(ctx->context(), (*array)->array_); if ((*query)->query_ == nullptr) { auto st = Status_Error( "Failed to allocate TileDB query object; Memory allocation failed"); @@ -3336,11 +3327,7 @@ capi_return_t tiledb_handle_query_plan_request( api::ensure_buffer_is_valid(request); api::ensure_buffer_is_valid(response); - tiledb::sm::Query query( - ctx->resources(), - ctx->cancellation_source(), - ctx->storage_manager(), - array->array_); + tiledb::sm::Query query(ctx->context(), array->array_); tiledb::sm::serialization::deserialize_query_plan_request( static_cast(serialization_type), request->buffer(), diff --git a/tiledb/sm/c_api/tiledb_filestore.cc b/tiledb/sm/c_api/tiledb_filestore.cc index c6be1d3b0e4..6ab8babb866 100644 --- a/tiledb/sm/c_api/tiledb_filestore.cc +++ b/tiledb/sm/c_api/tiledb_filestore.cc @@ -266,11 +266,7 @@ int32_t tiledb_filestore_uri_import( auto buffer_size = get_buffer_size_from_config(context.resources().config(), tile_extent); - tiledb::sm::Query query( - context.resources(), - context.cancellation_source(), - context.storage_manager(), - array); + tiledb::sm::Query query(context, array); throw_if_not_ok(query.set_layout(tiledb::sm::Layout::GLOBAL_ORDER)); std::vector buffer(buffer_size); @@ -292,11 +288,7 @@ int32_t tiledb_filestore_uri_import( query.set_subarray(subarray); auto tiledb_cloud_fix = [&](uint64_t start, uint64_t end) { - tiledb::sm::Query query( - context.resources(), - context.cancellation_source(), - context.storage_manager(), - array); + tiledb::sm::Query query(context, array); throw_if_not_ok(query.set_layout(tiledb::sm::Layout::ROW_MAJOR)); tiledb::sm::Subarray subarray_cloud_fix( array.get(), nullptr, context.resources().logger(), true); @@ -425,11 +417,7 @@ int32_t tiledb_filestore_uri_export( static_cast(subarray_range_arr), sizeof(uint64_t) * 2); subarray.add_range(0, std::move(subarray_range)); - tiledb::sm::Query query( - context.resources(), - context.cancellation_source(), - context.storage_manager(), - array); + tiledb::sm::Query query(context, array); throw_if_not_ok(query.set_layout(tiledb::sm::Layout::ROW_MAJOR)); query.set_subarray(subarray); @@ -532,11 +520,7 @@ int32_t tiledb_filestore_buffer_import( static_cast(0), ""); - tiledb::sm::Query query( - context.resources(), - context.cancellation_source(), - context.storage_manager(), - array); + tiledb::sm::Query query(context, array); throw_if_not_ok(query.set_layout(tiledb::sm::Layout::ROW_MAJOR)); tiledb::sm::Subarray subarray( @@ -605,11 +589,7 @@ int32_t tiledb_filestore_buffer_export( static_cast(subarray_range_arr), sizeof(uint64_t) * 2); subarray.add_range(0, std::move(subarray_range)); - tiledb::sm::Query query( - context.resources(), - context.cancellation_source(), - context.storage_manager(), - array); + tiledb::sm::Query query(context, array); throw_if_not_ok(query.set_layout(tiledb::sm::Layout::ROW_MAJOR)); query.set_subarray(subarray); uint64_t size_tmp = size; diff --git a/tiledb/sm/consolidator/array_meta_consolidator.cc b/tiledb/sm/consolidator/array_meta_consolidator.cc index 0608227adcc..7541adf8d36 100644 --- a/tiledb/sm/consolidator/array_meta_consolidator.cc +++ b/tiledb/sm/consolidator/array_meta_consolidator.cc @@ -36,7 +36,7 @@ #include "tiledb/sm/enums/query_type.h" #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" using namespace tiledb::common; @@ -47,10 +47,8 @@ namespace tiledb::sm { /* ****************************** */ ArrayMetaConsolidator::ArrayMetaConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager) - : Consolidator(resources, storage_manager) { + JobParent& parent, const Config& config) + : Consolidator(parent) { auto st = set_config(config); if (!st.ok()) { throw std::logic_error(st.message()); diff --git a/tiledb/sm/consolidator/array_meta_consolidator.h b/tiledb/sm/consolidator/array_meta_consolidator.h index a8ad9d1a731..f47794c7022 100644 --- a/tiledb/sm/consolidator/array_meta_consolidator.h +++ b/tiledb/sm/consolidator/array_meta_consolidator.h @@ -55,21 +55,10 @@ class ArrayMetaConsolidator : public Consolidator { /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of all Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * ArrayMetaConsolidator(ContextResources&, const Config&). - * - * @param resources The context resources. + * @param parent The parent of this consolidation job * @param config Config. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ - explicit ArrayMetaConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager); + explicit ArrayMetaConsolidator(JobParent& parent, const Config& config); /** Destructor. */ ~ArrayMetaConsolidator() = default; diff --git a/tiledb/sm/consolidator/commits_consolidator.cc b/tiledb/sm/consolidator/commits_consolidator.cc index 35e7cffa5f1..16ba1e32711 100644 --- a/tiledb/sm/consolidator/commits_consolidator.cc +++ b/tiledb/sm/consolidator/commits_consolidator.cc @@ -32,13 +32,9 @@ #include "tiledb/sm/consolidator/commits_consolidator.h" #include "tiledb/common/logger.h" -#include "tiledb/common/stdx_string.h" -#include "tiledb/sm/enums/datatype.h" #include "tiledb/sm/enums/query_type.h" -#include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/misc/tdb_time.h" -#include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" using namespace tiledb::common; @@ -48,9 +44,8 @@ namespace tiledb::sm { /* CONSTRUCTOR */ /* ****************************** */ -CommitsConsolidator::CommitsConsolidator( - ContextResources& resources, StorageManager* storage_manager) - : Consolidator(resources, storage_manager) { +CommitsConsolidator::CommitsConsolidator(JobParent& parent) + : Consolidator(parent) { } /* ****************************** */ diff --git a/tiledb/sm/consolidator/commits_consolidator.h b/tiledb/sm/consolidator/commits_consolidator.h index ba1bf226864..4a3a27ba27b 100644 --- a/tiledb/sm/consolidator/commits_consolidator.h +++ b/tiledb/sm/consolidator/commits_consolidator.h @@ -55,18 +55,9 @@ class CommitsConsolidator : public Consolidator { /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of all Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * CommitsConsolidator(ContextResources&). - * - * @param resources The context resources. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) + * @param parent The parent of this consolidation job */ - explicit CommitsConsolidator( - ContextResources& resources, StorageManager* storage_manager); + explicit CommitsConsolidator(JobParent& parent); /** Destructor. */ ~CommitsConsolidator() = default; diff --git a/tiledb/sm/consolidator/consolidator.cc b/tiledb/sm/consolidator/consolidator.cc index a003a20fc5b..428abcc2cd4 100644 --- a/tiledb/sm/consolidator/consolidator.cc +++ b/tiledb/sm/consolidator/consolidator.cc @@ -42,7 +42,7 @@ #include "tiledb/sm/enums/encryption_type.h" #include "tiledb/sm/object/object.h" #include "tiledb/sm/rest/rest_client.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" #include "tiledb/storage_format/uri/generate_uri.h" using namespace tiledb::common; @@ -53,28 +53,37 @@ namespace tiledb::sm { /* FACTORY METHODS */ /* ********************************* */ -/** Factory function to create the consolidator depending on mode. */ +/** + * Factory function to create the consolidator depending on mode. + */ shared_ptr Consolidator::create( - ContextResources& resources, - const ConsolidationMode mode, - const Config& config, - StorageManager* storage_manager) { + JobParent& parent, const ConsolidationMode mode, const Config& config) { switch (mode) { - case ConsolidationMode::FRAGMENT_META: - return make_shared( - HERE(), resources, storage_manager); - case ConsolidationMode::FRAGMENT: - return make_shared( - HERE(), resources, config, storage_manager); - case ConsolidationMode::ARRAY_META: - return make_shared( - HERE(), resources, config, storage_manager); - case ConsolidationMode::COMMITS: - return make_shared( - HERE(), resources, storage_manager); - case ConsolidationMode::GROUP_META: - return make_shared( - HERE(), resources, config, storage_manager); + case ConsolidationMode::FRAGMENT_META: { + auto job{make_shared(HERE(), parent)}; + job->register_shared_ptr(job); + return job; + } + case ConsolidationMode::FRAGMENT: { + auto job{make_shared(HERE(), parent, config)}; + job->register_shared_ptr(job); + return job; + } + case ConsolidationMode::ARRAY_META: { + auto job{make_shared(HERE(), parent, config)}; + job->register_shared_ptr(job); + return job; + } + case ConsolidationMode::COMMITS: { + auto job{make_shared(HERE(), parent)}; + job->register_shared_ptr(job); + return job; + } + case ConsolidationMode::GROUP_META: { + auto job{make_shared(HERE(), parent, config)}; + job->register_shared_ptr(job); + return job; + } default: return nullptr; } @@ -109,18 +118,15 @@ ConsolidationMode Consolidator::mode_from_config( /* CONSTRUCTORS & DESTRUCTORS */ /* ****************************** */ -Consolidator::Consolidator( - ContextResources& resources, StorageManager* storage_manager) - : resources_(resources) - , storage_manager_(storage_manager) +Consolidator::Consolidator(JobParent& parent) + : JobBranch(parent) + , resources_(parent.resources()) , consolidator_memory_tracker_(resources_.create_memory_tracker()) , stats_(resources_.stats().create_child("Consolidator")) , logger_(resources_.logger()->clone("Consolidator", ++logger_id_)) { consolidator_memory_tracker_->set_type(MemoryTrackerType::CONSOLIDATOR); } -Consolidator::~Consolidator() = default; - /* ****************************** */ /* API */ /* ****************************** */ @@ -138,13 +144,13 @@ void Consolidator::vacuum([[maybe_unused]] const char* array_name) { } void Consolidator::array_consolidate( - ContextResources& resources, + JobParent& parent, const char* array_name, EncryptionType encryption_type, const void* encryption_key, uint32_t key_length, - const Config& config, - StorageManager* storage_manager) { + const Config& config) { + auto& resources{parent.resources()}; // Check array URI URI array_uri(array_name); if (array_uri.is_invalid()) { @@ -190,22 +196,21 @@ void Consolidator::array_consolidate( // Consolidate auto mode = Consolidator::mode_from_config(config); - auto consolidator = - Consolidator::create(resources, mode, config, storage_manager); + auto consolidator = Consolidator::create(parent, mode, config); throw_if_not_ok(consolidator->consolidate( array_name, encryption_type, encryption_key, key_length)); } } void Consolidator::fragments_consolidate( - ContextResources& resources, + JobParent& parent, const char* array_name, EncryptionType encryption_type, const void* encryption_key, uint32_t key_length, const std::vector fragment_uris, - const Config& config, - StorageManager* storage_manager) { + const Config& config) { + auto& resources{parent.resources()}; // Check array URI URI array_uri(array_name); if (array_uri.is_invalid()) { @@ -246,8 +251,8 @@ void Consolidator::fragments_consolidate( } // Consolidate - auto consolidator = Consolidator::create( - resources, ConsolidationMode::FRAGMENT, config, storage_manager); + auto consolidator = + Consolidator::create(parent, ConsolidationMode::FRAGMENT, config); auto fragment_consolidator = dynamic_cast(consolidator.get()); throw_if_not_ok(fragment_consolidator->consolidate_fragments( @@ -313,10 +318,8 @@ void Consolidator::write_consolidated_commits_file( } void Consolidator::array_vacuum( - ContextResources& resources, - const char* array_name, - const Config& config, - StorageManager* storage_manager) { + JobParent& parent, const char* array_name, const Config& config) { + auto& resources{parent.resources()}; URI array_uri(array_name); if (array_uri.is_tiledb()) { throw_if_not_ok( @@ -325,8 +328,7 @@ void Consolidator::array_vacuum( } auto mode = Consolidator::mode_from_config(config, true); - auto consolidator = - Consolidator::create(resources, mode, config, storage_manager); + auto consolidator = Consolidator::create(parent, mode, config); consolidator->vacuum(array_name); } diff --git a/tiledb/sm/consolidator/consolidator.h b/tiledb/sm/consolidator/consolidator.h index dcbdc37ad2a..b63e873380e 100644 --- a/tiledb/sm/consolidator/consolidator.h +++ b/tiledb/sm/consolidator/consolidator.h @@ -38,6 +38,7 @@ #include "tiledb/common/status.h" #include "tiledb/sm/array/array.h" #include "tiledb/sm/storage_manager/context_resources.h" +#include "tiledb/sm/storage_manager/job.h" #include "tiledb/sm/storage_manager/storage_manager_declaration.h" #include @@ -68,8 +69,27 @@ enum class ConsolidationMode { }; /** Handles array consolidation. */ -class Consolidator { +class Consolidator : public JobBranch { public: + /** + * Default constructor is deleted + */ + Consolidator() = delete; + + protected: + /** + * Constructor is protected. + * + * @param parent The parent of this consolidation job + */ + explicit Consolidator(JobParent& parent); + + public: + /** + * Virtual destructor is override from `JobBranch`. + */ + ~Consolidator() override = default; + /* ********************************* */ /* FACTORY METHODS */ /* ********************************* */ @@ -77,25 +97,14 @@ class Consolidator { /** * Factory method to make a new Consolidator instance given the config mode. * - * @section Maturity Notes - * This is a transitional method in the sense that we are working - * on removing the dependency of the Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be create(ContextResources&, ...). - * - * @param resources The context resources. + * @param parent The parent of consolidator to be created * @param mode Consolidation mode. * @param config Configuration parameters for the consolidation * (`nullptr` means default). - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) * @return New Consolidator instance or nullptr on error. */ static shared_ptr create( - ContextResources& resources, - const ConsolidationMode mode, - const Config& config, - StorageManager* storage_manager); + JobParent& parent, const ConsolidationMode mode, const Config& config); /** * Returns the ConsolidationMode from the config. @@ -107,13 +116,6 @@ class Consolidator { static ConsolidationMode mode_from_config( const Config& config, const bool vacuum_mode = false); - /* ********************************* */ - /* DESTRUCTORS */ - /* ********************************* */ - - /** Destructor. */ - virtual ~Consolidator(); - /* ********************************* */ /* API */ /* ********************************* */ @@ -144,15 +146,8 @@ class Consolidator { /** * Consolidates the fragments of an array into a single one. * - * @section Maturity Notes - * This is a transitional method in the sense that we are working - * on removing the dependency of the Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * array_consolidate(ContextResources&, ...). - * - * @param resources The context resources. - * @param array_name The name of the array to be consolidated. + * @param parent The context under which this consolidation operates + * @param array_name The name of the array to be consolidated * @param encryption_type The encryption type of the array * @param encryption_key If the array is encrypted, the private encryption * key. For unencrypted arrays, pass `nullptr`. @@ -160,30 +155,20 @@ class Consolidator { * @param config Configuration parameters for the consolidation * (`nullptr` means default, which will use the config associated with * this instance). - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ static void array_consolidate( - ContextResources& resources, + JobParent& parent, const char* array_name, EncryptionType encryption_type, const void* encryption_key, uint32_t key_length, - const Config& config, - StorageManager* storage_manager); + const Config& config); /** * Consolidates the fragments of an array into a single one. * - * @section Maturity Notes - * This is a transitional method in the sense that we are working - * on removing the dependency of the Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * fragments_consolidate(ContextResources&, ...). - * - * @param resources The context resources. - * @param array_name The name of the array to be consolidated. + * @param parent The context under which this consolidation operates + * @param array_name The name of the array to be consolidated * @param encryption_type The encryption type of the array * @param encryption_key If the array is encrypted, the private encryption * key. For unencrypted arrays, pass `nullptr`. @@ -192,18 +177,15 @@ class Consolidator { * @param config Configuration parameters for the consolidation * (`nullptr` means default, which will use the config associated with * this instance). - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ static void fragments_consolidate( - ContextResources& resources, + JobParent& parent, const char* array_name, EncryptionType encryption_type, const void* encryption_key, uint32_t key_length, const std::vector fragment_uris, - const Config& config, - StorageManager* storage_manager); + const Config& config); /** * Writes a consolidated commits file. @@ -224,24 +206,12 @@ class Consolidator { * metadata. Note that this will coarsen the granularity of time traveling * (see docs for more information). * - * @section Maturity Notes - * This is a transitional method in the sense that we are working - * on removing the dependency of the Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * array_vacuum(ContextResources&, ...). - * - * @param resources The context resources. + * @param parent The context under which this vacuum operates * @param array_name The name of the array to be vacuumed. * @param config Configuration parameters for vacuuming. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ static void array_vacuum( - ContextResources& resources, - const char* array_name, - const Config& config, - StorageManager* storage_manager); + JobParent& parent, const char* array_name, const Config& config); /* ********************************* */ /* TYPE DEFINITIONS */ @@ -261,20 +231,11 @@ class Consolidator { /* ********************************* */ /** - * Constructor. - * - * Constructs a Consolidator object given a ContextResources reference. - * This is a transitional constructor in the sense that we are working - * on removing the dependency of the Consolidator class on StorageManager. For - * now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be Consolidator(ContextResources&). - * - * @param resources The context resources. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) + * Derived from `JobBranch` */ - explicit Consolidator( - ContextResources& resources, StorageManager* storage_manager); + ContextResources& resources() const override { + return resources_; + } /** * Checks if the array is remote. @@ -284,15 +245,12 @@ class Consolidator { void check_array_uri(const char* array_name); /* ********************************* */ - /* PROTECTED ATTRIBUTES */ + /* Member Variables */ /* ********************************* */ /** Resources used to perform the operation. */ ContextResources& resources_; - /** The storage manager. */ - StorageManager* storage_manager_; - /** The consolidator memory tracker. */ shared_ptr consolidator_memory_tracker_; diff --git a/tiledb/sm/consolidator/fragment_consolidator.cc b/tiledb/sm/consolidator/fragment_consolidator.cc index 4c07c709c32..4e8943f6769 100644 --- a/tiledb/sm/consolidator/fragment_consolidator.cc +++ b/tiledb/sm/consolidator/fragment_consolidator.cc @@ -40,7 +40,7 @@ #include "tiledb/sm/misc/tdb_time.h" #include "tiledb/sm/query/query.h" #include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" #include "tiledb/storage_format/uri/generate_uri.h" #include @@ -187,10 +187,8 @@ void FragmentConsolidationWorkspace::resize_buffers( /* ****************************** */ FragmentConsolidator::FragmentConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager) - : Consolidator(resources, storage_manager) { + JobParent& parent, const Config& config) + : Consolidator(parent) { auto st = set_config(config); if (!st.ok()) { throw FragmentConsolidatorException(st.message()); @@ -687,14 +685,8 @@ Status FragmentConsolidator::create_queries( // is not a user input prone to errors). // Create read query - query_r = tdb_unique_ptr(tdb_new( - Query, - resources_, - CancellationSource(storage_manager_), - storage_manager_, - array_for_reads, - nullopt, - read_memory_budget)); + query_r = tdb_unique_ptr( + tdb_new(Query, *this, array_for_reads, nullopt, read_memory_budget)); throw_if_not_ok(query_r->set_layout(Layout::GLOBAL_ORDER)); // Dense consolidation will do a tile aligned read. @@ -720,13 +712,7 @@ Status FragmentConsolidator::create_queries( // Create write query query_w = tdb_unique_ptr(tdb_new( - Query, - resources_, - CancellationSource(storage_manager_), - storage_manager_, - array_for_writes, - fragment_name, - write_memory_budget)); + Query, *this, array_for_writes, fragment_name, write_memory_budget)); throw_if_not_ok(query_w->set_layout(Layout::GLOBAL_ORDER)); throw_if_not_ok(query_w->disable_checks_consolidation()); query_w->set_fragment_size(config_.max_fragment_size_); diff --git a/tiledb/sm/consolidator/fragment_consolidator.h b/tiledb/sm/consolidator/fragment_consolidator.h index 3fcae353d7e..e1945fea5d5 100644 --- a/tiledb/sm/consolidator/fragment_consolidator.h +++ b/tiledb/sm/consolidator/fragment_consolidator.h @@ -174,21 +174,10 @@ class FragmentConsolidator : public Consolidator { /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of all Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * FragmentConsolidator(ContextResources&, const Config&). - * - * @param resources The context resources. + * @param parent The parent of this consolidation job * @param config Config. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ - explicit FragmentConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager); + explicit FragmentConsolidator(JobParent& parent, const Config& config); /** Destructor. */ ~FragmentConsolidator() = default; diff --git a/tiledb/sm/consolidator/fragment_meta_consolidator.cc b/tiledb/sm/consolidator/fragment_meta_consolidator.cc index 3ab957f9a35..a41e3e47047 100644 --- a/tiledb/sm/consolidator/fragment_meta_consolidator.cc +++ b/tiledb/sm/consolidator/fragment_meta_consolidator.cc @@ -37,7 +37,7 @@ #include "tiledb/sm/fragment/fragment_identifier.h" #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" #include "tiledb/sm/tile/generic_tile_io.h" #include "tiledb/sm/tile/tile.h" #include "tiledb/storage_format/uri/generate_uri.h" @@ -50,9 +50,8 @@ namespace tiledb::sm { /* CONSTRUCTOR */ /* ****************************** */ -FragmentMetaConsolidator::FragmentMetaConsolidator( - ContextResources& resources, StorageManager* storage_manager) - : Consolidator(resources, storage_manager) { +FragmentMetaConsolidator::FragmentMetaConsolidator(JobParent& parent) + : Consolidator(parent) { } /* ****************************** */ diff --git a/tiledb/sm/consolidator/fragment_meta_consolidator.h b/tiledb/sm/consolidator/fragment_meta_consolidator.h index 01c1ef574d9..3fe676c5f92 100644 --- a/tiledb/sm/consolidator/fragment_meta_consolidator.h +++ b/tiledb/sm/consolidator/fragment_meta_consolidator.h @@ -55,18 +55,9 @@ class FragmentMetaConsolidator : public Consolidator { /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of all Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * FragmentMetaConsolidator(ContextResources&). - * - * @param resources The context resources. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) + * @param parent The parent of this consolidation job. */ - explicit FragmentMetaConsolidator( - ContextResources& resources, StorageManager* storage_manager); + explicit FragmentMetaConsolidator(JobParent& parent); /** Destructor. */ ~FragmentMetaConsolidator() = default; diff --git a/tiledb/sm/consolidator/group_meta_consolidator.cc b/tiledb/sm/consolidator/group_meta_consolidator.cc index d534e1e89fe..f487129a30c 100644 --- a/tiledb/sm/consolidator/group_meta_consolidator.cc +++ b/tiledb/sm/consolidator/group_meta_consolidator.cc @@ -38,7 +38,7 @@ #include "tiledb/sm/group/group_details_v1.h" #include "tiledb/sm/misc/parallel_functions.h" #include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/job.h" using namespace tiledb::common; @@ -49,10 +49,8 @@ namespace tiledb::sm { /* ****************************** */ GroupMetaConsolidator::GroupMetaConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager) - : Consolidator(resources, storage_manager) { + JobParent& parent, const Config& config) + : Consolidator(parent) { auto st = set_config(config); if (!st.ok()) { throw std::logic_error(st.message()); diff --git a/tiledb/sm/consolidator/group_meta_consolidator.h b/tiledb/sm/consolidator/group_meta_consolidator.h index 5137584ba9f..fcbd7641e00 100644 --- a/tiledb/sm/consolidator/group_meta_consolidator.h +++ b/tiledb/sm/consolidator/group_meta_consolidator.h @@ -54,21 +54,10 @@ class GroupMetaConsolidator : public Consolidator { /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of all Consolidator classes on StorageManager. - * For now we still need to keep the storage_manager argument, but once the - * dependency is gone the signature will be - * GroupMetaConsolidator(ContextResources&, const Config&). - * - * @param resources The context resources. + * @param parent The parent of this consolidation job * @param config Config. - * @param storage_manager A StorageManager pointer. - * (this will go away in the near future) */ - explicit GroupMetaConsolidator( - ContextResources& resources, - const Config& config, - StorageManager* storage_manager); + explicit GroupMetaConsolidator(JobParent& parent, const Config& config); /** Destructor. */ ~GroupMetaConsolidator() = default; diff --git a/tiledb/sm/global_state/watchdog.cc b/tiledb/sm/global_state/watchdog.cc index a474346797a..21a5ed1e84f 100644 --- a/tiledb/sm/global_state/watchdog.cc +++ b/tiledb/sm/global_state/watchdog.cc @@ -80,7 +80,7 @@ void Watchdog::watchdog_thread(Watchdog* watchdog) { if (SignalHandlers::signal_received()) { for (auto* sm : GlobalState::GetGlobalState().storage_managers()) { - throw_if_not_ok(sm->cancel_all_tasks()); + sm->cancel_all_tasks(); } } diff --git a/tiledb/sm/group/group.cc b/tiledb/sm/group/group.cc index aa8eed1fbf4..14eb1ee1251 100644 --- a/tiledb/sm/group/group.cc +++ b/tiledb/sm/group/group.cc @@ -567,29 +567,28 @@ void Group::set_metadata_loaded(const bool metadata_loaded) { } void Group::consolidate_metadata( - ContextResources& resources, const char* group_name, const Config& config) { + JobParent& parent, const char* group_name, const Config& config) { // Check group URI URI group_uri(group_name); if (group_uri.is_invalid()) { throw GroupException("Cannot consolidate group metadata; Invalid URI"); } // Check if group exists - if (object_type(resources, group_uri) != ObjectType::GROUP) { + if (object_type(parent.resources(), group_uri) != ObjectType::GROUP) { throw GroupException( "Cannot consolidate group metadata; Group does not exist"); } // Consolidate // Encryption credentials are loaded by Group from config - StorageManager sm(resources, resources.logger(), config); - auto consolidator = Consolidator::create( - resources, ConsolidationMode::GROUP_META, config, &sm); + auto consolidator = + Consolidator::create(parent, ConsolidationMode::GROUP_META, config); throw_if_not_ok(consolidator->consolidate( group_name, EncryptionType::NO_ENCRYPTION, nullptr, 0)); } void Group::vacuum_metadata( - ContextResources& resources, const char* group_name, const Config& config) { + JobParent& parent, const char* group_name, const Config& config) { // Check group URI URI group_uri(group_name); if (group_uri.is_invalid()) { @@ -597,13 +596,12 @@ void Group::vacuum_metadata( } // Check if group exists - if (object_type(resources, group_uri) != ObjectType::GROUP) { + if (object_type(parent.resources(), group_uri) != ObjectType::GROUP) { throw GroupException("Cannot vacuum group metadata; Group does not exist"); } - StorageManager sm(resources, resources.logger(), config); - auto consolidator = Consolidator::create( - resources, ConsolidationMode::GROUP_META, config, &sm); + auto consolidator = + Consolidator::create(parent, ConsolidationMode::GROUP_META, config); consolidator->vacuum(group_name); } diff --git a/tiledb/sm/group/group.h b/tiledb/sm/group/group.h index b2ebb2f022b..55e0fa66a65 100644 --- a/tiledb/sm/group/group.h +++ b/tiledb/sm/group/group.h @@ -42,6 +42,7 @@ #include "tiledb/sm/group/group_directory.h" #include "tiledb/sm/group/group_member.h" #include "tiledb/sm/metadata/metadata.h" +#include "tiledb/sm/storage_manager/job.h" using namespace tiledb::common; @@ -229,7 +230,7 @@ class Group { /** * Consolidates the metadata of a group into a single file. * - * @param resources The context resources. + * @param parent The parent of this consolidation job * @param group_name The name of the group whose metadata will be * consolidated. * @param config Configuration parameters for the consolidation @@ -237,14 +238,12 @@ class Group { * this instance). */ static void consolidate_metadata( - ContextResources& resources, - const char* group_name, - const Config& config); + JobParent& parent, const char* group_name, const Config& config); /** * Vacuums the consolidated metadata files of a group. * - * @param resources The context resources. + * @param parent The parent of this consolidation job * @param group_name The name of the group whose metadata will be * vacuumed. * @param config Configuration parameters for vacuuming @@ -252,9 +251,7 @@ class Group { * this instance). */ static void vacuum_metadata( - ContextResources& resources, - const char* group_name, - const Config& config); + JobParent& parent, const char* group_name, const Config& config); /** Returns a constant pointer to the encryption key. */ const EncryptionKey* encryption_key() const; diff --git a/tiledb/sm/group/group_details_v2.cc b/tiledb/sm/group/group_details_v2.cc index b77d8a1cb9a..97d07a532d0 100644 --- a/tiledb/sm/group/group_details_v2.cc +++ b/tiledb/sm/group/group_details_v2.cc @@ -31,6 +31,7 @@ */ #include "tiledb/sm/group/group_details_v2.h" +#include "group.h" #include "tiledb/common/common.h" #include "tiledb/common/logger.h" diff --git a/tiledb/sm/group/group_details_v2.h b/tiledb/sm/group/group_details_v2.h index 36ddfd04e6c..1159a467049 100644 --- a/tiledb/sm/group/group_details_v2.h +++ b/tiledb/sm/group/group_details_v2.h @@ -42,7 +42,6 @@ #include "tiledb/sm/group/group_details_v1.h" #include "tiledb/sm/group/group_member.h" #include "tiledb/sm/metadata/metadata.h" -#include "tiledb/sm/storage_manager/storage_manager.h" #include "tiledb/storage_format/serialization/serializers.h" using namespace tiledb::common; diff --git a/tiledb/sm/query/dimension_label/array_dimension_label_queries.cc b/tiledb/sm/query/dimension_label/array_dimension_label_queries.cc index 4f961970d5a..990dc0d2509 100644 --- a/tiledb/sm/query/dimension_label/array_dimension_label_queries.cc +++ b/tiledb/sm/query/dimension_label/array_dimension_label_queries.cc @@ -50,15 +50,14 @@ using namespace tiledb::common; namespace tiledb::sm { ArrayDimensionLabelQueries::ArrayDimensionLabelQueries( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, Array* array, const Subarray& subarray, const std::unordered_map& label_buffers, const std::unordered_map& array_buffers, const optional& fragment_name) - : resources_(resources) - , storage_manager_(storage_manager) + : JobBranch(parent) + , resources_(parent.resources()) , label_range_queries_by_dim_idx_(subarray.dim_num(), nullptr) , label_data_queries_by_dim_idx_(subarray.dim_num()) , range_query_status_{QueryStatus::UNINITIALIZED} @@ -249,12 +248,7 @@ void ArrayDimensionLabelQueries::add_read_queries( // Create the range query. range_queries_.emplace_back(tdb_new( - DimensionLabelQuery, - resources_, - storage_manager_, - dim_label, - dim_label_ref, - label_ranges)); + DimensionLabelQuery, *this, dim_label, dim_label_ref, label_ranges)); label_range_queries_by_dim_idx_[dim_idx] = range_queries_.back().get(); } catch (const StatusException& err) { throw DimensionLabelQueryException( @@ -283,8 +277,7 @@ void ArrayDimensionLabelQueries::add_read_queries( // Create the data query. data_queries_.emplace_back(tdb_new( DimensionLabelQuery, - resources_, - storage_manager_, + *this, dim_label, dim_label_ref, subarray, @@ -335,8 +328,7 @@ void ArrayDimensionLabelQueries::add_write_queries( // Create the dimension label query. data_queries_.emplace_back(tdb_new( DimensionLabelQuery, - resources_, - storage_manager_, + *this, dim_label, dim_label_ref, subarray, diff --git a/tiledb/sm/query/dimension_label/array_dimension_label_queries.h b/tiledb/sm/query/dimension_label/array_dimension_label_queries.h index 0d73b42eed1..d4eacdd1e47 100644 --- a/tiledb/sm/query/dimension_label/array_dimension_label_queries.h +++ b/tiledb/sm/query/dimension_label/array_dimension_label_queries.h @@ -52,7 +52,7 @@ class Subarray; enum class QueryType : uint8_t; -class ArrayDimensionLabelQueries { +class ArrayDimensionLabelQueries : public JobBranch { public: /** * Size type for the number of dimensions of an array and for dimension @@ -66,17 +66,15 @@ class ArrayDimensionLabelQueries { /** Default constructor is not C.41 compliant. */ ArrayDimensionLabelQueries() = delete; + /** + * Virtual destructor is override from `JobBranch`. + */ + ~ArrayDimensionLabelQueries() override = default; + /** * Constructor. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of the Query class on StorageManager. - * For now, we still need to keep the storage_manager argument, but once the - * dependency is gone, the signature will be - * ArrayDimensionLabelQueries(ContextResources&, Array*, ...). - * - * @param resources The context resources. - * @param storage_manager Storage manager object. + * @param parent The parent of this query as a job * @param array Parent array the dimension labels are defined on. * @param subarray Subarray for the query on the parent array. * @param label_buffers A map of query buffers containing label data. @@ -85,8 +83,7 @@ class ArrayDimensionLabelQueries { * @param fragment_name Optional fragment name for writing fragments. */ ArrayDimensionLabelQueries( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, Array* array, const Subarray& subarray, const std::unordered_map& label_buffers, @@ -170,9 +167,6 @@ class ArrayDimensionLabelQueries { /** The context resources. */ ContextResources& resources_; - /** The storage manager. */ - StorageManager* storage_manager_; - /** Map from label name to dimension label opened by this query. */ std::unordered_map> dimension_labels_; @@ -257,6 +251,13 @@ class ArrayDimensionLabelQueries { const URI& dim_label_uri, const std::string& dim_label_name, const QueryType& query_type); + + /** + * Derived from `JobBranch` + */ + ContextResources& resources() const override { + return resources_; + } }; } // namespace tiledb::sm diff --git a/tiledb/sm/query/dimension_label/dimension_label_query.cc b/tiledb/sm/query/dimension_label/dimension_label_query.cc index 6ce26c3cafd..60f12fa3a49 100644 --- a/tiledb/sm/query/dimension_label/dimension_label_query.cc +++ b/tiledb/sm/query/dimension_label/dimension_label_query.cc @@ -50,20 +50,14 @@ using namespace tiledb::common; namespace tiledb::sm { DimensionLabelQuery::DimensionLabelQuery( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, shared_ptr dim_label, const DimensionLabel& dim_label_ref, const Subarray& parent_subarray, const QueryBuffer& label_buffer, const QueryBuffer& index_buffer, optional fragment_name) - : Query( - resources, - CancellationSource(storage_manager), - storage_manager, - dim_label, - fragment_name) + : Query(parent, dim_label, fragment_name) , dim_label_name_{dim_label_ref.name()} { switch (dim_label->get_query_type()) { case (QueryType::READ): @@ -113,17 +107,11 @@ DimensionLabelQuery::DimensionLabelQuery( } DimensionLabelQuery::DimensionLabelQuery( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, shared_ptr dim_label, const DimensionLabel& dim_label_ref, const std::vector& label_ranges) - : Query( - resources, - CancellationSource(storage_manager), - storage_manager, - dim_label, - nullopt) + : Query(parent, dim_label, nullopt) , dim_label_name_{dim_label_ref.name()} , index_data_{IndexDataCreate::make_index_data( array_schema().dimension_ptr(0)->type(), diff --git a/tiledb/sm/query/dimension_label/dimension_label_query.h b/tiledb/sm/query/dimension_label/dimension_label_query.h index 5f8ae54b997..ff163216d69 100644 --- a/tiledb/sm/query/dimension_label/dimension_label_query.h +++ b/tiledb/sm/query/dimension_label/dimension_label_query.h @@ -66,14 +66,7 @@ class DimensionLabelQuery : public Query { /** * Constructor for queries to read or write label data. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of the Query class on StorageManager. - * For now, we still need to keep the storage_manager argument, but once the - * dependency is gone, the signature will be - * DimensionLabelQuery(ContextResources&, shared_ptr, ...). - * - * @param resources The context resources. - * @param storage_manager Storage manager object. + * @param parent The parent of this query as a job * @param dim_label Opened dimension label for the query. * @param dim_label_ref Description of the dimension label. * @param parent_subarray Subarray of the parent array. @@ -83,8 +76,7 @@ class DimensionLabelQuery : public Query { * @param fragment_name Name to use when writing the fragment. */ DimensionLabelQuery( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, shared_ptr dim_label, const DimensionLabel& dim_label_ref, const Subarray& parent_subarray, @@ -95,21 +87,13 @@ class DimensionLabelQuery : public Query { /** * Constructor for range queries. * - * This is a transitional constructor in the sense that we are working - * on removing the dependency of the Query class on StorageManager. - * For now, we still need to keep the storage_manager argument, but once the - * dependency is gone, the signature will be - * DimensionLabelQuery(ContextResources&, shared_ptr, ...). - * - * @param resources The context resources. - * @param storage_manager Storage manager object. + * @param parent The parent of this query as a job * @param dim_label Opened dimension label for the query. * @param dim_label_ref Description of the dimension label. * @param label_ranges Label ranges to read index ranges from. */ DimensionLabelQuery( - ContextResources& resources, - StorageManager* storage_manager, + JobParent& parent, shared_ptr dim_label, const DimensionLabel& dim_label_ref, const std::vector& label_ranges); diff --git a/tiledb/sm/query/query.cc b/tiledb/sm/query/query.cc index 8fd1189db3d..8168f23d5c2 100644 --- a/tiledb/sm/query/query.cc +++ b/tiledb/sm/query/query.cc @@ -83,13 +83,12 @@ static uint64_t get_effective_memory_budget( /* ****************************** */ Query::Query( - ContextResources& resources, - CancellationSource cancellation_source, - StorageManager* storage_manager, + JobParent& parent, shared_ptr array, optional fragment_name, optional memory_budget) - : resources_(resources) + : JobBranch(parent) + , resources_(parent.resources()) , stats_(resources_.stats().create_child("Query")) , logger_(resources_.logger()->clone("Query", ++logger_id_)) , query_memory_tracker_(resources_.memory_tracker_manager().create_tracker( @@ -107,8 +106,7 @@ Query::Query( (type_ == QueryType::READ || array_schema_->dense()) ? Layout::ROW_MAJOR : Layout::UNORDERED) - , cancellation_source_(cancellation_source) - , storage_manager_(storage_manager) + , cancellation_source_(parent.make_cancellation_source()) , dim_label_queries_(nullptr) , has_coords_buffer_(false) , has_zipped_coords_buffer_(false) @@ -497,7 +495,7 @@ Status Query::submit_and_finalize() { } init(); - throw_if_not_ok(storage_manager_->query_submit(this)); + throw_if_not_ok(storage_manager()->query_submit(this)); throw_if_not_ok(strategy_->finalize()); status_ = QueryStatus::COMPLETED; @@ -714,8 +712,7 @@ void Query::init() { // Initialize the dimension label queries. dim_label_queries_ = tdb_unique_ptr(tdb_new( ArrayDimensionLabelQueries, - resources_, - storage_manager_, + *this, array_, subarray_, label_buffers_, @@ -1638,7 +1635,7 @@ Status Query::submit() { return Status::Ok(); } init(); - throw_if_not_ok(storage_manager_->query_submit(this)); + throw_if_not_ok(storage_manager()->query_submit(this)); reset_coords_markers(); return Status::Ok(); diff --git a/tiledb/sm/query/query.h b/tiledb/sm/query/query.h index 5d39ed133cf..a44ecc1e758 100644 --- a/tiledb/sm/query/query.h +++ b/tiledb/sm/query/query.h @@ -56,6 +56,7 @@ #include "tiledb/sm/query/validity_vector.h" #include "tiledb/sm/rest/rest_client.h" #include "tiledb/sm/storage_manager/cancellation_source.h" +#include "tiledb/sm/storage_manager/job.h" #include "tiledb/sm/subarray/subarray.h" using namespace tiledb::common; @@ -68,8 +69,14 @@ class ArrayDimensionLabelQueries; enum class QueryStatus : uint8_t; enum class QueryType : uint8_t; -/** Processes a (read/write) query. */ -class Query { +/** + * Processes any query. + * + * `class Query` is a `JobBranch`. It's a supervised activity in an obvious way, + * typically running directly under a context. It's also a job parent because + * dimension label queries have ordinary queries inside them. + */ +class Query : public JobBranch { public: /* ********************************* */ /* PUBLIC DATATYPES */ @@ -139,31 +146,17 @@ class Query { * case the query will be used as writes and the given URI should be used * for the name of the new fragment to be created. * - * @section Maturity - * - * This is a transitional constructor. There is still a `StorageManager` - * argument, and there is also a vestige of it with the `CancellationSource` - * argument. These argument now only pertain to job control of query with - * respect to its context. Once this facility is rewritten, these constructor - * argument may be dropped. - * * @pre Array must be a properly opened array. * - * @param resources The context resources. - * @param cancellation_source A source of external cancellation events - * @param storage_manager Storage manager object. + * @param parent The parent of this query as a job * @param array The array that is being queried. - * @param fragment_uri The full URI for the new fragment. Only used for + * @param fragment_name The full URI for the new fragment. Only used for * writes. - * @param fragment_base_uri Optional base name for new fragment. Only used for - * writes and only if fragment_uri is empty. * @param memory_budget Total memory budget. If set to nullopt, the value * will be obtained from the sm.mem.total_budget config option. */ Query( - ContextResources& resources, - CancellationSource cancellation_source, - StorageManager* storage_manager, + JobParent& parent, shared_ptr array, optional fragment_name = nullopt, optional memory_budget = nullopt); @@ -986,9 +979,6 @@ class Query { */ CancellationSource cancellation_source_; - /** The storage manager. */ - StorageManager* storage_manager_; - /** The query strategy. */ tdb_unique_ptr strategy_; @@ -1203,6 +1193,13 @@ class Query { /** Copies the data from the aggregates to the user buffers. */ void copy_aggregates_data_to_user_buffer(); + + /** + * Derived from `JobBranch` + */ + ContextResources& resources() const override { + return resources_; + } }; } // namespace tiledb::sm diff --git a/tiledb/sm/query_plan/test/unit_query_plan.cc b/tiledb/sm/query_plan/test/unit_query_plan.cc index c7974c6555f..3ea2d07de3b 100644 --- a/tiledb/sm/query_plan/test/unit_query_plan.cc +++ b/tiledb/sm/query_plan/test/unit_query_plan.cc @@ -44,7 +44,7 @@ #include "tiledb/sm/enums/layout.h" #include "tiledb/sm/query/query.h" #include "tiledb/sm/stats/stats.h" -#include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/storage_manager/context.h" #include "tiledb/storage_format/uri/parse_uri.h" using namespace tiledb; @@ -65,9 +65,9 @@ struct QueryPlanFx { TemporaryLocalDirectory temp_dir_; Config cfg_; + tiledb::sm::Context ctx_; shared_ptr logger_; - ContextResources resources_; - shared_ptr sm_; + ContextResources& resources_; }; tdb_unique_ptr QueryPlanFx::create_array(const URI uri) { @@ -110,10 +110,11 @@ URI QueryPlanFx::array_uri(const std::string& array_name) { } QueryPlanFx::QueryPlanFx() - : memory_tracker_(tiledb::test::create_test_memory_tracker()) + : cfg_() + , ctx_(cfg_) + , memory_tracker_(tiledb::test::create_test_memory_tracker()) , logger_(make_shared(HERE(), "foo")) - , resources_(cfg_, logger_, 1, 1, "") - , sm_(make_shared(resources_, logger_, cfg_)) { + , resources_(ctx_.resources()) { } TEST_CASE_METHOD(QueryPlanFx, "Query plan dump_json", "[query_plan][dump]") { @@ -126,8 +127,7 @@ TEST_CASE_METHOD(QueryPlanFx, "Query plan dump_json", "[query_plan][dump]") { REQUIRE(st.ok()); shared_ptr array_shared = std::move(array); - Query query( - resources_, CancellationSource(sm_.get()), sm_.get(), array_shared); + Query query(ctx_, array_shared); REQUIRE(query.set_layout(Layout::ROW_MAJOR).ok()); stats::Stats stats("foo"); diff --git a/tiledb/sm/serialization/query.cc b/tiledb/sm/serialization/query.cc index ed4f50c932e..efb52d4d01e 100644 --- a/tiledb/sm/serialization/query.cc +++ b/tiledb/sm/serialization/query.cc @@ -74,7 +74,6 @@ #include "tiledb/sm/serialization/config.h" #include "tiledb/sm/serialization/fragment_metadata.h" #include "tiledb/sm/serialization/query.h" -#include "tiledb/sm/storage_manager/storage_manager.h" #include "tiledb/sm/subarray/subarray_partitioner.h" using namespace tiledb::common; diff --git a/tiledb/sm/storage_manager/CMakeLists.txt b/tiledb/sm/storage_manager/CMakeLists.txt index cbdb8ac0d9d..9cf92540a1e 100644 --- a/tiledb/sm/storage_manager/CMakeLists.txt +++ b/tiledb/sm/storage_manager/CMakeLists.txt @@ -3,7 +3,7 @@ # # The MIT License # -# Copyright (c) 2023 TileDB, Inc. +# Copyright (c) 2023-2024 TileDB, Inc. # # Permission is hereby granted, free of charge, to any person obtaining a copy # of this software and associated documentation files (the "Software"), to deal @@ -33,3 +33,5 @@ commence(object_library context_resources) this_target_sources(context_resources.cc) this_target_object_libraries(baseline config rest_client stats thread_pool vfs) conclude(object_library) + +add_test_subdirectory() diff --git a/tiledb/sm/storage_manager/cancellation_source.cc b/tiledb/sm/storage_manager/cancellation_source.cc index 2d8dbdd34cc..5e8d4af2b49 100644 --- a/tiledb/sm/storage_manager/cancellation_source.cc +++ b/tiledb/sm/storage_manager/cancellation_source.cc @@ -1,5 +1,5 @@ /** - * @file cancellation_source.h + * @file cancellation_source.cc * * @section LICENSE * @@ -35,16 +35,12 @@ namespace tiledb::sm { -CancellationSource::CancellationSource(const StorageManager* sm) +LegacyCancellationSource::LegacyCancellationSource(StorageManager& sm) : sm_(sm) { - if (sm_ == nullptr) { - throw std::invalid_argument( - "[CancellationSource] StorageManager argument may not be null"); - } } -bool CancellationSource::cancellation_in_progress() const { - return sm_->cancellation_in_progress(); +bool LegacyCancellationSource::cancellation_in_progress() const { + return sm_.cancellation_in_progress(); } } // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/cancellation_source.h b/tiledb/sm/storage_manager/cancellation_source.h index 5a429a870e4..bd567d64003 100644 --- a/tiledb/sm/storage_manager/cancellation_source.h +++ b/tiledb/sm/storage_manager/cancellation_source.h @@ -1,5 +1,5 @@ /** - * @file cancellation_source + * @file cancellation_source.h * * @section LICENSE * @@ -33,22 +33,98 @@ #ifndef TILEDB_CANCELLATION_SOURCE_H #define TILEDB_CANCELLATION_SOURCE_H +#include "tiledb/stdx/stop_token" // substitutes for +//-------- clang-format separator -------- #include "storage_manager_declaration.h" namespace tiledb::sm { /** - * The cancellation source is, at present, a wrapper around `StorageManager` - * with a very restricted interface. + * The legacy cancellation source is a wrapper around `StorageManager` but with + * a restricted interface. */ -class CancellationSource { - const StorageManager* sm_; +class LegacyCancellationSource { + StorageManager& sm_; public: - CancellationSource(const StorageManager* sm); + explicit LegacyCancellationSource(StorageManager& sm); - bool cancellation_in_progress() const; + [[nodiscard]] bool cancellation_in_progress() const; }; +using CancellationSource = LegacyCancellationSource; + } // namespace tiledb::sm +namespace tiledb::common { + +/** + * Marker class for cancellation source constructor + */ +struct CancellationOriginT { + CancellationOriginT() = default; +}; +/** + * Marker element for cancellation source constructor + */ +constexpr static CancellationOriginT cancellation_origin{}; + +/** + * This is the new cancellation source. It has not yet replaced the legacy one, + * but it's here in stub form so that the constructors of the job system may be + * defined as they will be later. + * + * @section Usage + * + * Each job is a cancellation origin. Use the origin-marked constructor to + * create the cancellation source member variable of a job. + * + * Activities within a job are subordinate cancellation sources. Create such + * objects with the copy constructor. They should be passed by value, not by + * reference. + + * @section Design + * + * Cancellation is propagated explicitly downward away from the root, not + * implicitly by copying the cancellation source. This choice is required so + * that individual branches of a job tree may be cancelled separately, that is, + * to cancel a branch only without cancelling the whole tree. + * + * Using a simple copy for subordinate object is possible because + * `std::stop_source` contains all the referential apparatus to ensure that + * copies of a stop source refer to the same `std::stop_state`. + */ +class NewCancellationSource { + /** + * Cancellation state + */ + std::stop_token stop_token_{}; + + public: + /** + * Default constructor is deleted. + * + * `std::stop_token` does have a default constructor, creating an object with + * no associated `stop_state`. In the present case it would be inimical to the + * goals of reliable cancellation to admit the possibility that a cancellation + * source couldn't cancel anything. + */ + NewCancellationSource() = delete; + + /** + * Constructor for an origin cancellation source + * + * This constructor uses a marker class, instead of being the default + * constructor, in order to clearly indicate that it's an origin. + */ + explicit NewCancellationSource(CancellationOriginT){}; + + /** + * Copy constructor + */ + NewCancellationSource(const NewCancellationSource&) = default; + + [[nodiscard]] bool cancellation_requested() const; +}; + +} // namespace tiledb::common #endif // TILEDB_CANCELLATION_SOURCE_H diff --git a/tiledb/sm/storage_manager/context.cc b/tiledb/sm/storage_manager/context.cc index 8e6a38a7811..11624595da7 100644 --- a/tiledb/sm/storage_manager/context.cc +++ b/tiledb/sm/storage_manager/context.cc @@ -46,62 +46,40 @@ class ContextException : public StatusException { } }; -static common::Logger::Level get_log_level(const Config& config); - -/* ****************************** */ -/* CONSTRUCTORS & DESTRUCTORS */ -/* ****************************** */ - -// Constructor. Note order of construction: storage_manager depends on the -// preceding members to be initialized for its initialization. -Context::Context(const Config& config) - : last_error_(nullopt) - , logger_(make_shared( - HERE(), - logger_prefix_ + std::to_string(++logger_id_), - get_log_level(config))) - , resources_( - config, - logger_, - get_compute_thread_count(config), - get_io_thread_count(config), - // TODO: Remove `.StorageManager` from statistic names - // We're sticking with `Context.StorageManager` here because - // it is part of the public facing API. - "Context.StorageManager") - , storage_manager_{resources_, logger_, config} { - /* - * Logger class is not yet C.41-compliant - */ - init_loggers(config); -} - -/* ****************************** */ -/* API */ -/* ****************************** */ - -optional Context::last_error() { - std::lock_guard lock(mtx_); - return last_error_; -} - -void Context::save_error(const Status& st) { - std::lock_guard lock(mtx_); - last_error_ = st.to_string(); -} - -void Context::save_error(const std::string& msg) { - std::lock_guard lock(mtx_); - last_error_ = msg; -} - -void Context::save_error(const StatusException& st) { - std::lock_guard lock(mtx_); - last_error_ = st.what(); +/** + * Obtain the log level from a configuration. + * + * Supports the initialization of `class Context` + */ +common::Logger::Level get_log_level(const Config& config) { + auto cfg_level = config.get("config.logging_level"); + if (cfg_level == "0") { + return Logger::Level::FATAL; + } else if (cfg_level == "1") { + return Logger::Level::ERR; + } else if (cfg_level == "2") { + return Logger::Level::WARN; + } else if (cfg_level == "3") { + return Logger::Level::INFO; + } else if (cfg_level == "4") { + return Logger::Level::DBG; + } else if (cfg_level == "5") { + return Logger::Level::TRACE; + } else { + return Logger::Level::ERR; + } } -Status Context::get_config_thread_count( - const Config& config, uint64_t& config_thread_count_ret) { +/** + * Obtain the thread count to use for initializing `class Context`, taking into + * account a number of legacy configuration variables. + * + * @param logger Logger to use to record misconfigurations + * @param config The configuration to examine + * @param max_thread_count The return value + */ +Status get_config_thread_count( + Logger& logger, const Config& config, uint64_t& max_thread_count) { // The "sm.num_async_threads", "sm.num_reader_threads", // "sm.num_tbb_threads", "sm.num_writer_threads", and "sm.num_vfs_threads" // have been removed. If they are set, we will log an error message. @@ -117,7 +95,7 @@ Status Context::get_config_thread_count( config.get("sm.num_async_threads", &num_async_threads, &found)); if (found) { config_thread_count = std::max(config_thread_count, num_async_threads); - logger_->error( + logger.error( "[Context::get_config_thread_count] " "Config parameter \"sm.num_async_threads\" has been removed; use " "config parameter \"sm.compute_concurrency_level\"."); @@ -128,7 +106,7 @@ Status Context::get_config_thread_count( "sm.num_reader_threads", &num_reader_threads, &found)); if (found) { config_thread_count = std::max(config_thread_count, num_reader_threads); - logger_->error( + logger.error( "[Context::get_config_thread_count] " "Config parameter \"sm.num_reader_threads\" has been removed; use " "config parameter \"sm.compute_concurrency_level\"."); @@ -139,7 +117,7 @@ Status Context::get_config_thread_count( "sm.num_writer_threads", &num_writer_threads, &found)); if (found) { config_thread_count = std::max(config_thread_count, num_writer_threads); - logger_->error( + logger.error( "[Context::get_config_thread_count] " "Config parameter \"sm.num_writer_threads\" has been removed; use " "config parameter \"sm.compute_concurrency_level\"."); @@ -150,7 +128,7 @@ Status Context::get_config_thread_count( config.get("sm.num_vfs_threads", &num_vfs_threads, &found)); if (found) { config_thread_count = std::max(config_thread_count, num_vfs_threads); - logger_->error( + logger.error( "[Context::get_config_thread_count] " "Config parameter \"sm.num_vfs_threads\" has been removed; use " "config parameter \"sm.io_concurrency_level\"."); @@ -166,20 +144,29 @@ Status Context::get_config_thread_count( if (found) { config_thread_count = std::max(config_thread_count, static_cast(num_tbb_threads)); - logger_->error( + logger.error( "[Context::get_config_thread_count] " "Config parameter \"sm.num_tbb_threads\" has been removed; use " "config parameter \"sm.io_concurrency_level\"."); } - config_thread_count_ret = static_cast(config_thread_count); + max_thread_count = static_cast(config_thread_count); return Status::Ok(); } -size_t Context::get_compute_thread_count(const Config& config) { +/** + * Obtain the number of threads in a compute thread pool from a configuration + * + * Returns the maximum of the configured value and the thread count returned + * from get_config_thread_count(). + * + * @param config A configuration that specifies the compute thread + * @return Compute thread count + */ +size_t get_compute_thread_count(Logger& logger, const Config& config) { uint64_t config_thread_count{0}; - if (!get_config_thread_count(config, config_thread_count).ok()) { + if (!get_config_thread_count(logger, config, config_thread_count).ok()) { throw std::logic_error("Cannot get compute thread count"); } @@ -199,9 +186,9 @@ size_t Context::get_compute_thread_count(const Config& config) { std::max(config_thread_count, compute_concurrency_level)); } -size_t Context::get_io_thread_count(const Config& config) { +size_t get_io_thread_count(Logger& logger, const Config& config) { uint64_t config_thread_count{0}; - if (!get_config_thread_count(config, config_thread_count).ok()) { + if (!get_config_thread_count(logger, config, config_thread_count).ok()) { throw std::logic_error("Cannot get config thread count"); } @@ -221,6 +208,67 @@ size_t Context::get_io_thread_count(const Config& config) { std::max(config_thread_count, io_concurrency_level)); } +/** + * Note order of construction: `storage_manager_` depends on the + * preceding members to be initialized for its initialization. + */ +ContextBase::ContextBase(const Config& config) + : logger_(make_shared( + HERE(), + logger_prefix_ + std::to_string(++logger_id_), + get_log_level(config))) + , resources_( + config, + logger_, + get_compute_thread_count(*logger_, config), + get_io_thread_count(*logger_, config), + // TODO: Remove `.StorageManager` from statistic names + // We're sticking with `Context.StorageManager` here because + // it is part of the public facing API. + "Context.StorageManager") + , storage_manager_{resources_, config} { +} + +Context::Context(const Config& config) + : ContextBase(config) + , JobRoot{storage_manager_} + , context_handle_(ContextRegistry::get().register_context(*this)) + , last_error_(nullopt) { + /* + * Logger class is not yet C.41-compliant + */ + init_loggers(config); +} + +/** + * @section Implementation + * + * At present cancellation is still in `StorageManager`. + */ +void Context::cancel_all_tasks() { + storage_manager_.cancel_all_tasks(); +} + +optional Context::last_error() { + std::lock_guard lock(mtx_); + return last_error_; +} + +void Context::save_error(const Status& st) { + std::lock_guard lock(mtx_); + last_error_ = st.to_string(); +} + +void Context::save_error(const std::string& msg) { + std::lock_guard lock(mtx_); + last_error_ = msg; +} + +void Context::save_error(const StatusException& st) { + std::lock_guard lock(mtx_); + last_error_ = st.what(); +} + void Context::init_loggers(const Config& config) { // temporarily set level to error so that possible errors reading // configuration are visible to the user @@ -250,23 +298,4 @@ void Context::init_loggers(const Config& config) { logger_->set_level(static_cast(level)); } -common::Logger::Level get_log_level(const Config& config) { - auto cfg_level = config.get("config.logging_level"); - if (cfg_level == "0") { - return Logger::Level::FATAL; - } else if (cfg_level == "1") { - return Logger::Level::ERR; - } else if (cfg_level == "2") { - return Logger::Level::WARN; - } else if (cfg_level == "3") { - return Logger::Level::INFO; - } else if (cfg_level == "4") { - return Logger::Level::DBG; - } else if (cfg_level == "5") { - return Logger::Level::TRACE; - } else { - return Logger::Level::ERR; - } -} - } // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/context.h b/tiledb/sm/storage_manager/context.h index 77370f0d3f1..7a521fa7eb3 100644 --- a/tiledb/sm/storage_manager/context.h +++ b/tiledb/sm/storage_manager/context.h @@ -33,11 +33,11 @@ #ifndef TILEDB_CONTEXT_H #define TILEDB_CONTEXT_H +#include "context_registry.h" +#include "job.h" #include "tiledb/common/exception/exception.h" #include "tiledb/common/thread_pool/thread_pool.h" #include "tiledb/sm/config/config.h" -#include "tiledb/sm/stats/global_stats.h" -#include "tiledb/sm/storage_manager/cancellation_source.h" #include "tiledb/sm/storage_manager/context_resources.h" #include "tiledb/sm/storage_manager/storage_manager.h" @@ -51,11 +51,65 @@ using namespace tiledb::common; namespace tiledb::sm { +/** + * Base class for `Context` + * + * This class exists to deal with order-of-initialization between + * `StorageManager` and `CancellationSource`. A cancellation source is required + * for the base class `JobRoot`, but currently the constructor for + * `CancellationSource` requires a pointer to a storage manager. This class + * breaks what would otherwise be a cycle by moving the `StorageManager` member + * variable into a base class. + * + * This class won't need to exist once `StorageManager` is gone. + */ +struct ContextBase { + /** The class logger. */ + shared_ptr logger_; + + /** + * Initializer for `logger_prefix_` cannot throw during static initialization. + */ + static std::string build_logger_prefix() noexcept { + try { + return std::to_string( + std::chrono::duration_cast( + std::chrono::system_clock::now().time_since_epoch()) + .count()) + + "-Context: "; + } catch (...) { + try { + return {"TimeError-Context:"}; + } catch (...) { + return {}; + } + } + } + + /** The class unique logger prefix */ + inline static std::string logger_prefix_{build_logger_prefix()}; + + /** + * Counter for generating unique identifiers for `Logger` objects. + */ + inline static std::atomic logger_id_ = 0; + + /** The class resources. */ + mutable ContextResources resources_; + + /** + * The storage manager. + */ + StorageManager storage_manager_; + + explicit ContextBase(const Config& config); +}; + /** * This class manages the context for the C API, wrapping a * storage manager object. */ -class Context { +class Context : public ContextBase, public JobRoot { public: /* ********************************* */ /* CONSTRUCTORS & DESTRUCTORS */ @@ -70,7 +124,7 @@ class Context { explicit Context(const Config&); /** Destructor. */ - ~Context() = default; + ~Context() override = default; /* ********************************* */ /* API */ @@ -94,21 +148,24 @@ class Context { */ void save_error(const StatusException& st); - /** Pointer to the underlying storage manager. */ - inline StorageManager* storage_manager() { - return &storage_manager_; - } - - /** Pointer to the underlying storage manager. */ - inline const StorageManager* storage_manager() const { - return &storage_manager_; - } - - inline CancellationSource cancellation_source() const { - return CancellationSource(storage_manager()); - } + /** + * Cancel all free-running activity under this context. + * + * This function is synchronous. It does not return until all activity under + * the context has ended. + * + * @section Maturity + * + * At the present time, not all activities that can operate under a context + * are interruptible by the context. They'll all eventually end, but it may + * not be promptly. + */ + void cancel_all_tasks(); - [[nodiscard]] inline ContextResources& resources() const { + /** + * Derived from `JobRoot` + */ + [[nodiscard]] ContextResources& resources() const override { return resources_; } @@ -142,6 +199,11 @@ class Context { /* PRIVATE ATTRIBUTES */ /* ********************************* */ + /** + * The handle of this context within the context registry. + */ + ContextRegistry::handle_type context_handle_; + /** The last error occurred. */ optional last_error_{nullopt}; @@ -150,64 +212,10 @@ class Context { */ std::mutex mtx_; - /** The class logger. */ - shared_ptr logger_; - - /** The class unique logger prefix */ - inline static std::string logger_prefix_ = - std::to_string(std::chrono::duration_cast( - std::chrono::system_clock::now().time_since_epoch()) - .count()) + - "-Context: "; - - /** - * Counter for generating unique identifiers for `Logger` objects. - */ - inline static std::atomic logger_id_ = 0; - - /** The class resources. */ - mutable ContextResources resources_; - - /** - * The storage manager. - */ - StorageManager storage_manager_; - /* ********************************* */ /* PRIVATE METHODS */ /* ********************************* */ - /** - * Get maximum number of threads to use in thread pools, based on config - * parameters. - * - * @param config The Config to look up max thread information from. - * @param max_thread_count (out) Variable to store max thread count. - * @return Status of request. - */ - Status get_config_thread_count( - const Config& config, uint64_t& max_thread_count); - - /** - * Get number of threads to use in compute thread pool, based on config - * parameters. Will return the max of the configured value and the max thread - * count returned by get_max_thread_count() - * - * @param config The Config to look up the compute thread information from. - * @return Compute thread count. - */ - size_t get_compute_thread_count(const Config& config); - - /** - * Get number of threads to use in io thread pool, based on config - * parameters. Will return the max of the configured value and the max thread - * count returned by get_max_thread_count() - * - * @param config The Config to look up the io thread information from. - * @return IO thread count. - */ - size_t get_io_thread_count(const Config& config); - /** * Initializes global and local logger. * diff --git a/tiledb/sm/storage_manager/context_registry.cc b/tiledb/sm/storage_manager/context_registry.cc new file mode 100644 index 00000000000..139c0a6c500 --- /dev/null +++ b/tiledb/sm/storage_manager/context_registry.cc @@ -0,0 +1,36 @@ +/** + * @file tiledb/sm/storage_manager/context_registry.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file defines the remainder `class ContextRegistry`. + * + */ + +#include "context_registry.h" + +namespace tiledb::sm {} // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/context_registry.h b/tiledb/sm/storage_manager/context_registry.h new file mode 100644 index 00000000000..a70434fadc6 --- /dev/null +++ b/tiledb/sm/storage_manager/context_registry.h @@ -0,0 +1,109 @@ +/** + * @file tiledb/sm/storage_manager/context_registry.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file declares `class ContextRegistry`. + * + * @section Life Cycle + * + * `class ContextRegistry` is used as a singleton. It's defined within the + * accessor function for the singleton, and is thus dynamically initialized. + */ + +#ifndef TILEDB_CONTEXT_REGISTRY_H +#define TILEDB_CONTEXT_REGISTRY_H + +#include "tiledb/common/registry/registry.h" + +namespace tiledb::sm { + +// Forward +class Context; + +class ContextRegistry { + friend class Context; + using registry_type = common::Registry; + + registry_type registry_; + + using handle_type = registry_type::registry_handle_type; + + /** + * Register a context in this registry. + * + * This function is private so that it's accessible only to `class Context`. + * + * @param context + * @return + */ + handle_type register_context(Context& context) { + return registry_.register_item(context); + } + + public: + /** + * Ordinary constructor is the default one. + * + * @section Design + * + * Having a default constructor is an intentional design choice. Because this + * class is used only as a singleton, it's desirable that it be nonparametric, + * exactly so that it can be initialized without interacting with the life + * cycle of the library itself, including such issues as dynamic loading. + * + * This decision is a consequence of the fact that the library itself does not + * require a designated point of initialization. There's no class that + * represents the library, for example, nor is there a C API call to + * initialize the library. As such, we have no good place to designate + * parameters, and thus this class does not use any. + */ + ContextRegistry() = default; + + /** + * Destructor is the default one. + */ + ~ContextRegistry() = default; + + /** + * Accessor function to the singleton. + * + * The registry is not parametric. It uses no configuration variables nor + * arguments. As a result, the singleton instance can be default-constructed + * in the body of this function. + * + * @return a reference to the singleton instance of `ContextRegistry` + */ + static ContextRegistry& get() { + static ContextRegistry context_registry{}; + return context_registry; + } +}; + +} // namespace tiledb::sm + +#endif // TILEDB_CONTEXT_REGISTRY_H diff --git a/tiledb/sm/storage_manager/doc/job_class_relationship.plantuml b/tiledb/sm/storage_manager/doc/job_class_relationship.plantuml new file mode 100644 index 00000000000..1c7f8b7237d --- /dev/null +++ b/tiledb/sm/storage_manager/doc/job_class_relationship.plantuml @@ -0,0 +1,45 @@ +@startuml +title Job Class Relationships + +package Root { + package "Upper Half" { + class "Nonchild" as Nonchild_Root + } + package "Lower Half" { + class "Parent" as Parent_Root + } + Nonchild_Root <-d- Parent_Root +} + +package Branch { + package "Upper Half" { + class "Child" as Child_Branch + } + package "Lower Half" { + class "Parent" as Parent_Branch + } + Child_Branch <-d- Parent_Branch +} +Parent_Root <--> Child_Branch + +package Leaf { + package "Upper Half" { + class "Child" as Child_Leaf + } + package "Lower Half" { + class "Nonparent" as Parent_Leaf + } + Child_Leaf <-d- Parent_Leaf +} +Parent_Branch <--> Child_Leaf + +package Isolate { + package "Upper Half" { + class "Nonchild" as Child_Isolate + } + package "Lower Half" { + class "Nonparent" as Parent_Isolate + } + Child_Isolate <-d- Parent_Isolate +} +@enduml \ No newline at end of file diff --git a/tiledb/sm/storage_manager/doc/job_class_with_mixin.plantuml b/tiledb/sm/storage_manager/doc/job_class_with_mixin.plantuml new file mode 100644 index 00000000000..b4c49421e17 --- /dev/null +++ b/tiledb/sm/storage_manager/doc/job_class_with_mixin.plantuml @@ -0,0 +1,33 @@ +@startuml + +title Class Structure with Mixin + +package "Upper Half" { + ActivityBase <|-d- ActivityMixin + ActivityMixin <|-d- ChildBase + ActivityMixin <|-d- NonchildBase + ChildBase <|-d- ChildMixin + NonchildBase <|-d- NonchildMixin +} + +package "Lower Half" { + SupervisionBase <|-d- SupervisionMixin + SupervisionMixin <|-d- ParentBase + SupervisionMixin <|-d- NonparentBase + ParentBase <|-d- ParentMixin + NonparentBase <|-d- NonparentMixin +} + +package "JobSystem" { + JobRoot --u-* NonchildMixin + JobRoot --u-* ParentMixin + JobBranch --u-* ChildMixin + JobBranch --u-* ParentMixin + JobLeaf --u--* ChildMixin + JobLeaf --u--* NonparentMixin + JobIsolate --u-* NonchildMixin + JobIsolate --u-* NonparentMixin + JobRoot -d[hidden]- JobLeaf +} + +@enduml \ No newline at end of file diff --git a/tiledb/sm/storage_manager/doc/job_class_without_mixin.plantuml b/tiledb/sm/storage_manager/doc/job_class_without_mixin.plantuml new file mode 100644 index 00000000000..695e94aeb12 --- /dev/null +++ b/tiledb/sm/storage_manager/doc/job_class_without_mixin.plantuml @@ -0,0 +1,17 @@ +@startuml + +title Class Structure without Mixin + +package "Upper Half" { + Activity <|-d- Child + Activity <|-d- Nonchild +} + +package "Lower Half" { + Supervision <|-d- Parent + Supervision <|-d- Nonparent +} + +"Upper Half" <-d- "Lower Half" + +@enduml \ No newline at end of file diff --git a/tiledb/sm/storage_manager/doc/job_inheritance_mixin.plantuml b/tiledb/sm/storage_manager/doc/job_inheritance_mixin.plantuml new file mode 100644 index 00000000000..4dee5f81bd1 --- /dev/null +++ b/tiledb/sm/storage_manager/doc/job_inheritance_mixin.plantuml @@ -0,0 +1,50 @@ +@startuml + +title Inheritance before and after mixin + +skinparam packageStyle rectangle + + +package Original { + class "" as B + note left : The original base class + class "" as D + note left : The original derived class + B <|-d- D +} + +' Hidden note tweaks layout +note "N1" as N1 +Original .d. N1 +N1 .l. "Derived-Mix Group" +hide N1 + +package "Base-Mix Group" { + class "Base" as BaseBase + class "Mixin" as BaseMixin { + +Mixin(Base&) + } + note right of BaseBase + The original base class + becomes the base of the + mixed version of that class. + end note + BaseBase <|-- BaseMixin +} + +package "Derived-Mix Group" { + class "Base" as DerivedBase { + } + class "Mixin" as DerivedMixin { + +Mixin(Base&) + } + BaseMixin <|-d- DerivedBase + note right of DerivedBase + The original derived class + now derives from the mixed + version of the base class. + end note + DerivedBase <|-d- DerivedMixin +} + +@enduml \ No newline at end of file diff --git a/tiledb/sm/storage_manager/job.cc b/tiledb/sm/storage_manager/job.cc new file mode 100644 index 00000000000..f0c7bab8e89 --- /dev/null +++ b/tiledb/sm/storage_manager/job.cc @@ -0,0 +1,37 @@ +/** + * @file tiledb/sm/storage_manager/job.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file defines class Job. + */ +#include "job.h" + +namespace tiledb::sm { +class Context; + +} // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/job.h b/tiledb/sm/storage_manager/job.h new file mode 100644 index 00000000000..cf17448886d --- /dev/null +++ b/tiledb/sm/storage_manager/job.h @@ -0,0 +1,228 @@ +/** + * @file tiledb/sm/storage_manager/job.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file declares the mixin for using the job system in the TileDB library. + */ + +#ifndef TILEDB_CONTEXT_JOB_H +#define TILEDB_CONTEXT_JOB_H + +#include "tiledb/sm/storage_manager/job_system.h" + +#include "cancellation_source.h" +#include "storage_manager_declaration.h" + +//------------------------------------------------------- +// Library-specific configuration +//------------------------------------------------------- +namespace tiledb::sm { +// Forward declaration +class ContextResources; + +namespace job { + +namespace tcj = tiledb::common::job; + +/** + * The library mixin for the job system focuses on resources. + * + * This mixin focuses on two entities: + * - The legacy storage manager. `StorageManager` is only used to handle + * query cancellation; this will be replaced with a cancellation system + * intrinsic to the job system. + * - `ContextResources`. Resources are now consistently accessed through a job + * parent, rather than going back to `class Context` or being passed as a + * function argument. + * + * Future uses of this mixin might include the following. All of them share in + * common that they benefit from a hierarchy. + * - Performance measurement. This is currently in `class Stats`. Gathering + * performance metrics through the job system will allow hierarchical + * reporting. + * - Resource budgets, including memory. Subdivision of budget is only + * possible when the division is explicitly modeled. + */ +struct JobResourceMixin { + using Self = JobResourceMixin; + struct ParentMixin; + + /** + * Each activity holds a reference to a storage manager, which is currently + * the means used to implement cancellation. + */ + class ActivityMixin : public tcj::ActivityBase { + StorageManager& sm_; + + public: + ActivityMixin() = delete; + explicit ActivityMixin(StorageManager& sm) + : ActivityBase() + , sm_(sm){}; + + /** + * Accessor for the storage manager associated with this activity. + */ + [[nodiscard]] StorageManager* storage_manager() { + return &sm_; + } + }; + + /** + * The child activity uses the storage manager of its parent. + */ + class ChildMixin : public tcj::ChildBase { + public: + ChildMixin() = delete; + explicit ChildMixin(ParentMixin& parent) + : ChildBase{parent, parent.parent_storage_manager()} { + } + }; + + /** + * The nonchild activity uses the storage manager from its constructor + * argument. + */ + struct NonchildMixin : public tcj::NonchildBase { + NonchildMixin() = delete; + explicit NonchildMixin(StorageManager& sm) + : NonchildBase(sm) { + } + }; + + /** + * The supervision mixin adds nothing over the default. + */ + struct SupervisionMixin : public tcj::SupervisionBase { + SupervisionMixin() = delete; + explicit SupervisionMixin(ActivityMixin& activity) + : SupervisionBase(activity){}; + }; + + /** + * The parent (lower) propagates the storage manager of its activity (upper). + */ + struct ParentMixin : public tcj::ParentBase { + friend class ChildMixin; + activity_type& activity_; + + StorageManager& parent_storage_manager() const { + return *activity_.storage_manager(); + } + + public: + ParentMixin() = delete; + + explicit ParentMixin(activity_type& activity) + : tcj::ParentBase(activity) + , activity_(activity) { + } + + virtual ~ParentMixin() = default; + + /** + * Accessor for the resources of this Parent + * + * @section Design + * + * This function is virtual to anticipate a future subdivision of resources. + * At present `class ContextResources` is only an accessor to resource + * objects; it does not carry any limitations or budget with it. In such + * course as operations are more tightly budgeted, the resources they have + * may be accordingly tracked and possibly limited. + * + * Each class derived from `Parent` implements its own resources accessor. + * Typically this accessor will simply return its own member variable. + */ + [[nodiscard]] virtual ContextResources& resources() const = 0; + + /** + * Factory for cancellation source objects that are tied to the cancellation + * state of this parent. + */ + sm::LegacyCancellationSource make_cancellation_source() { + return sm::LegacyCancellationSource(parent_storage_manager()); + } + }; + + /** + * The nonparent mixin adds nothing over the default. + */ + struct NonparentMixin : public tcj::NonparentBase { + NonparentMixin() = delete; + explicit NonparentMixin(ActivityMixin& activity) + : tcj::NonparentBase(activity) { + } + }; +}; + +using system_type = tcj::JobSystem; +} // namespace job + +//------------------------------------------------------- +// Exports +//------------------------------------------------------- +/* + * The namespace `tiledb::sm::job` is an internal namespace, not to be used + * externally. Below are the type declarations exported from this header. + */ +/** + * `JobParent` should be used only as an interface, not as a base class. + */ +using JobParent = job::system_type::JobParent; + +/** + * The root class of a job tree. + * + * The only class derived from this is `class Context`. + */ +using JobRoot = job::system_type::JobRoot; + +/** + * The branch class of a job tree. + * + * Branches are both parent and child. Any composite activity is a branch. + */ +using JobBranch = job::system_type::JobBranch; + +/** + * The leaf class of a job tree. + * + * Leaves are non-composite activities. For example, a single long-lived I/O + * operation could be a leaf. + */ +using JobLeaf = job::system_type::JobLeaf; + +/** + * A degenerate tree, with exactly one element. + */ +using JobIsolate = job::system_type::JobIsolate; + +} // namespace tiledb::sm + +#endif // TILEDB_CONTEXT_JOB_H diff --git a/tiledb/sm/storage_manager/job_owner.h b/tiledb/sm/storage_manager/job_owner.h new file mode 100644 index 00000000000..4824b13fbe2 --- /dev/null +++ b/tiledb/sm/storage_manager/job_owner.h @@ -0,0 +1,38 @@ +/** + * @file tiledb/sm/storage_manager/job_registry.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file defines class JobRegistry, a type-specific version of Registry. + */ + +#ifndef TILEDB_JOB_OWNER_H +#define TILEDB_JOB_OWNER_H + +} // namespace tiledb::sm + +#endif // TILEDB_CONTEXT_H diff --git a/tiledb/sm/storage_manager/job_system.h b/tiledb/sm/storage_manager/job_system.h new file mode 100644 index 00000000000..dcc3dc115a9 --- /dev/null +++ b/tiledb/sm/storage_manager/job_system.h @@ -0,0 +1,777 @@ +/** + * @file tiledb/sm/storage_manager/job_system.h + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file declares class Job. + * + * @section Overview + * + * A job is a supervised activity. Jobs may supervise other jobs, forming a + * tree. At the TileDB library uses this job system, the root of a job tree is a + * `Context`. Major user-visible operations, such as query and consolidation, + * are branches in the tree. + * + * @section Design + * + * Each job object is the composition of an activity object and a supervision + * object. At the leaf of the job tree, the supervision object is trivial, since + * a leaf does not supervise anything else. At the root of the job tree, the + * activity object is special, since it's a unique activity that must support + * the need to act as the root of supervision. + * + * Each job object is a composition between two halves, and each half comes in + * two variants depending on its position in the tree. The halves are denoted as + * "upper" and "lower", with the convention that the root of the tree is at the + * top and the tree grows downward. The upper half is an `Activity`; its two + * variants are `Child` and `Nonchild`. The lower half is a `Supervision`; its + * two variants are `Parent` and `Nonparent`. To construct a `Child`, a + * reference to a `Parent` in some other object is needed. To construct a + * `Parent`, on the other hand, a reference to the `Child` in the same object is + * needed. The `Parent` reference in the `Child` comes from outside the system; + * it's the responsibility of the user. Conversely, the `Child` reference in the + * `Parent` is handled entirely with the jobs system; it's not visible from the + * outside. + * + * `class Activity` is a simple base class for classes that do something. That + * "something" is specified with a mixin class. Each of the six structural + * classes have their own mixin in order to allow appropriate application + * behavior coherent with the job tree. The job system itself, however, has no + * specific knowledge of what that something is. + * + * For example, as the job system is used within this library, `class Context` + * is a job root but does not directly see queries. Instead, it sees them only + * indirectly as job branches. That's because queries are not the only kind of + * branch. This principle is vital to keeping cyclic dependencies out of the + * library. If it were otherwise, say, by `class Context` being able to list + * queries and their `QueryStatus` reporting values, it would mean that `class + * Context` and everything that references it would need to link most all of the + * library. This would be fatal to the policy that code be testable with only + * minimal dependencies. + * + * @section Life Cycle + * + * Supervision in job tree does not extend to creating the job objects; the job + * system does not supply factories for making specific jobs. We thus use the + * word "supervision" instead of "control". Supervision watches what's happening + * and is able to perform certain operations on generic jobs, but doesn't + * control everthing that's part of the tree. + * + * The lifespan of a job parent must be strictly larger than that of a job + * child. This is because the child holds both a direct reference to its parent + * as well as a handle to a registry entry within the parent. (The handle + * assumes existence of the registry to work correctly.) It would be technically + * possible to weaken this requirement at the cost of greater internal + * complexity; the design decision here was that this lifespan requirement was + * naturally satisfied in the typical case where an operation has suboperations + * that must finish before the operation itself does. + * + * @section Job States + * + * The activity of a job is one with some definite life span that can end. + * Because of the life cycle requirement that the life span of a parent must + * encompass that of a child, long-lived jobs are only appropriate near the top + * of the tree. + * + * Jobs exist in one of three states: quiescent, active, halted. The initial + * state is quiescent. The final state is halted. A job cannot be halted while + * it still has active operations. When a job is ordered to halt, the transition + * to the halted state does not happen immediately but only after all its active + * operations have stopped. + * + * @section Maturity + * + * The mature part of the library are the structural elements around + * construction of a job tree. This is a necessary first step that allows the + * job system to be integrated with the rest of the library. + * + * State management is a stub at present. There are some functions defined but + * not used. There's a tree-lock system that needs to be fully implemented, as + * well as a tree-walk function the requires locking the tree. + * + * The use of job control system of this library is nascent at present. The only + * activity that's currently tracked all centers around queries. As yet there's + * plenty of activity that's not tracked as a `class Activity` through some job, + * in addition to supervisors of such activity. Operations throughout the code + * will have to be packaged as job classes for them to fall under the + * supervision of a parent, and the parents will have to become job branches. + * The work remaining to finish building out the jobs system can be estimated by + * the amount of activity that's not implemented within some operation class. + */ + +#ifndef TILEDB_COMMON_JOB_SYSTEM_H +#define TILEDB_COMMON_JOB_SYSTEM_H + +#include +#include "tiledb/stdx/stop_token" // substitutes for + +#include "cancellation_source.h" +#include "tiledb/common/registry/registry.h" + +namespace tiledb::common::job { + +//---------------------------------- +// Supervision: Parent and Nonparent +//---------------------------------- +namespace test { +template +class WhiteboxSupervisionBase; +template +class WhiteboxParentBase; +template +class WhiteboxNonparentBase; +} // namespace test + +/** + * Base class for the supervision (lower) half of a job. + * + * This class is the ultimate base of both the parent class and the not-parent + * class. Parent objects supervise other jobs (either a branch or leaf). + * Nonparent objects do not supervise anything. + */ +template +class SupervisionBase { + template + friend class WhiteboxSupervisionBase; + + public: + using activity_type = typename Mixin::ActivityMixin; + + private: + /** + * Reference to the upper half of a job object. + */ + activity_type& activity_; + + public: + SupervisionBase() = delete; + + explicit SupervisionBase(activity_type& activity) + : activity_(activity) { + } + + /** + * Accessor to the upper half of a job object. + * + * Supervision objects are the lower half, constructed second. + */ + activity_type activity() { + return activity_; + } +}; + +/* + * Forward declaration + */ +template +class ChildBase; + +/** + * Base for classes with subordinate jobs: `JobRoot` and `JobBranch`. Aliased as + * `JobParent` + * + * The overall responsibility of a job parent is to subdivide resources. In + * order to fulfill this responsibility, this base class does two things: + * - Has a registry of all subordinate jobs + * - Provides resources to its subordinate jobs. + */ +template +class ParentBase : public Mixin::SupervisionMixin, + private Registry> { + template + friend class WhiteboxParentBase; + + using Base = Registry>; + + friend class ChildBase; + + public: + /** + * The type of the job handle to be held by an instance of `class Job` + */ + using job_handle_type = typename Base::registry_handle_type; + + private: + /** + * Register a job as one to be governed by this context. + * + * @return A handle that refers to its argument + */ + job_handle_type register_job(ChildBase& job) { + return Base::register_item(job); + } + + public: + ParentBase() = delete; + + template + explicit ParentBase(typename Mixin::ActivityMixin& activity, Args&&... args) + : Mixin::SupervisionMixin(activity, std::forward(args)...) { + } + + static constexpr bool is_parent = true; + + using job_size_type = typename Base::size_type; + + /** + * The current number of jobs in this registry. + */ + [[nodiscard]] job_size_type number_of_jobs() const { + return Base::size(); + } + + /* + * Stub state predicates + * + * We need a for_each to distinguish between active and halted. + */ + bool is_active() { + return Base::size() > 0; + } + bool is_quiescent() { + return Base::size() == 0; + } + bool is_halted() { + if (is_active()) { + return true; + } + return false; + } +}; + +/** + * Base class for `Nonparent` + */ +template +class NonparentBase : public Mixin::SupervisionMixin { + template + friend class WhiteboxNonparentBase; + + public: + explicit NonparentBase(typename Mixin::ActivityMixin& activity) + : Mixin::SupervisionMixin(activity) { + } + + static constexpr bool is_parent = false; + + constexpr bool is_active() { + return false; + } + constexpr bool is_quiescent() { + return true; + } + constexpr bool is_halted() { + return false; + } +}; + +//------------------------------------------------------- +// Activity: Child and Nonchild +//------------------------------------------------------- +/* + * The core activity of a job is the upper half of the composite. If a job + * is supervised by a parent, the activity is the target of supervision. + */ + +//---------------------------------- +// Activity +//---------------------------------- +namespace test { +template +class WhiteboxActivityBase; +template +class WhiteboxChildBase; +template +class WhiteboxNonchildBase; +} // namespace test + +/** + * Base class for job activities + * + * An activity is the part of a job that's worth being supervised and monitored. + * + * Supervised: + * - Has a `cancel()` method + * - (maybe later) `start()` method. Currently it's start at construction. + * - (maybe later) `suspend()`, `resume()` methods. Would need to based on + * `std::jthread` to be meaningful. + * Monitored: + * - A node in the job tree, visible during tree traversal + * - (later) A nexus for performance measurement, a.k.a. "stats" + * + * `class Activity` is the base class for both `class Child` and `class + * Nonchild`, depending on whether the instance is registered with a parent. + * + */ +template +class ActivityBase { + template + friend class test::WhiteboxActivityBase; + + NewCancellationSource new_cancellation_source_; + + protected: + /** + * Constructor is protected, because this is always a base class. + */ + ActivityBase() + : new_cancellation_source_(cancellation_origin) { + } + + public: + /** + * Predicate for the "quiescent" state + */ + bool is_quiescent() { + if (is_active()) { + return true; + } + return !new_cancellation_source_.cancellation_requested(); + } + + /** + * Predicate for the "halted" state + */ + bool is_halted() { + if (is_active()) { + return true; + } + return new_cancellation_source_.cancellation_requested(); + }; + + /** + * Predicate for the "active" state + * + * This function is virtual to allow subclasses to define their own sense + * of `active`. + * + * @section Design + * + * This method is a concession to the maturity of the code base as a whole. + * Ideally each activity class knows when it's active and when it's not, but + * at present that information is not always explicit. + */ + virtual bool is_active() { + return true; + } + + /** + * Lock an activity against state change + * + * The base class does not itself own a mutex, so the function here is + * trivial. It's the responsibility of each activity class to implement + * locking in coordination with its own state changes. + */ + virtual void lock() { + } + + /** + * Release the lock obtained by `lock()` + * + * The base class does not itself own a mutex, so the function here is + * trivial. It's the responsibility of each activity class to implement + * locking in coordination with its own state changes. + */ + virtual void unlock() { + } + + /** + * Returns whether the lock is integrated into state change of the activity + * class. + * + * The default is `true`, since this base class does no locking of its own. + * Activity classes that do not have explicit state change governed by a mutex + * will also have nothing to lock, and thus don't need to override this class. + * + * @section Design + * + * This method is a concession to the maturity of the code base as a whole. + * Ideally every activity class has non-trivial locking, meaning that an + * activity is expected to be cancellable, even for long-running I/O. Once + * that's accomplished, `lock()` and `unlock()` can be made pure-virtual and + * this method removed. + */ + [[nodiscard]] virtual bool has_trivial_locking() const { + return true; + } +}; + +//---------------------------------- +// Child +//---------------------------------- +/** + * Base for classes with a supervisor: `JobBranch` and `JobLeaf` + */ +template +class ChildBase : public Mixin::ActivityMixin { + template + friend class test::WhiteboxChildBase; + + public: + using parent_type = typename Mixin::ParentMixin; + + private: + /** + * The parent supervises this activity. + */ + parent_type& parent_; + + /* + * This alias definition repeats the one in ParentBase. Were it not to be + * repeated, + */ + using job_handle_type = + typename Registry>::registry_handle_type; + + /** + * The job handle for this job, as provided by its parent + */ + job_handle_type job_handle_; + + public: + /** + * Default constructor is deleted. There no such thing as an child job that + * does not have a parent. + */ + ChildBase() = delete; + + /** + * Ordinary constructor + * + * The constructor registers the existence of this activity with its parent. + * The caller of this constructor is responsible for doing the second phase of + * registration. + * + * @param parent The parent of this job + */ + template + explicit ChildBase(parent_type& parent, Args&&... args) + : Mixin::ActivityMixin(std::forward(args)...) + , parent_(parent) + , job_handle_(parent.register_job(*this)) { + } + + /** + * Destructor + */ + ~ChildBase() = default; + + /** + * Accessor to parent. + */ + [[nodiscard]] parent_type& parent() const { + return parent_; + } + + /** + * Property self-declaration. For testing. + */ + static constexpr bool is_child{true}; + + /** + * Register this object in the parent registry with the `shared_ptr` which + * holds this object. This function should be called immediately after the + * object is constructed. + * + * @param ptr `shared_ptr` that points to this object + */ + void register_shared_ptr(std::shared_ptr ptr) { + job_handle_.register_shared_ptr(ptr); + } +}; + +//---------------------------------- +// Nonchild +//---------------------------------- +/** + * Base for classes without a supervisor: `JobRoot` and `JobIsolate` + */ +template +class NonchildBase : public Mixin::ActivityMixin { + template + friend class test::WhiteboxNonchildBase; + + public: + template + explicit NonchildBase(Args&&... args) + : Mixin::ActivityMixin(std::forward(args)...) { + } + + static constexpr bool is_child{false}; +}; + +//------------------------------------------------------- +// Mixin +//------------------------------------------------------- +/* + * The job system uses a mix-in class to separate two concerns: + * - What supervision and activity do generically + * - Specific requirements needed for how the job system is used + * + * All specific functionality is present in the mix-in class. The overall + * structure looks this example inheritance hierarchy: + * - `template class ${part}Base` + * - `Mixin::${part}Mixin: public ${part}Base` + * - `template class ${subpart}Base : public ${part}Mixin` + * - `Mixin::{subpart}Mixin: public ${subpart}Base` + */ + +/** + * Null mixin class for the job system. + * + * This is a do-nothing mixin class used to define the job system. It's not + * suitable for a complete job system, because it doesn't hook into any + * particular application mechanisms. Instead, this class illustrates how to + * define a mixin class for actual use. The class structure and inheritance + * illustrates how a production mixin system should look. + * + * Note that extraneous default constructors are deleted. This is so that any + * defects in the inheritance hierarchy are exposed promptly. + */ +class NullMixin { + /** + * The `Self` alias is used for clarity when the mixin classes refer to the + * mixed versions of the job classes. The system structure is more important + * than the specific name of this class. + */ + using Self = NullMixin; + + public: + struct ParentMixin; + + /** + * Mixin class for Activity + */ + struct ActivityMixin : public ActivityBase { + explicit ActivityMixin() + : ActivityBase(){}; + }; + + /** + * Mixin class for Child + */ + struct ChildMixin : public ChildBase { + ChildMixin() = delete; + explicit ChildMixin(ParentMixin& parent) + : ChildBase(parent){}; + }; + + /** + * Mixin class for Nonchild + */ + struct NonchildMixin : public NonchildBase { + explicit NonchildMixin() + : NonchildBase() { + } + }; + + /** + * Mixin class for Supervision + */ + struct SupervisionMixin : public SupervisionBase { + SupervisionMixin() = delete; + explicit SupervisionMixin(ActivityMixin& activity) + : SupervisionBase(activity){}; + }; + + /** + * Mixin class for Parent + */ + struct ParentMixin : public ParentBase { + ParentMixin() = delete; + explicit ParentMixin(ActivityMixin& activity) + : ParentBase(activity) { + } + }; + + /** + * Mixin class for Nonparent + */ + struct NonparentMixin : public NonparentBase { + NonparentMixin() = delete; + explicit NonparentMixin(ActivityMixin& activity) + : NonparentBase(activity) { + } + }; +}; + +//------------------------------------------------------- +// Job and JobSystem +//------------------------------------------------------- +/** + * Integration template unifying all of the root, branch, and leaf aspects of + * the job system + * + * Note that we use public inheritance so that, for example, we can declare + * `JobParent` arguments in constructors. + * + * @section States + * + * There are three states a job can be in: + * - Quiescent. Nothing is active and new activity is possible + * - Active. Something is active, either itself or some descendant. + * - Halted. Nothing is active and new activity will not occur. + * + * There is no associated state machine that represents the state of a job. + * Instead of an explicit state variable there are instead three predicate + * functions, one for each possible state. + * + * There's a pseudostate "halting" state that could be made detectable, but it's + * not implemented as a predicate function. "Halting" means it's been ordered to + * halt but it's operations have not concluded yet. As such "halting" is a + * sub-state of "active". It's not exposed because there's nothing further to do + * once the job has been told to stop. + * + * @tparam SupervisionT Base class for parent/nonparent behavior + * @tparam ActivityT Base class for child/nonchild behavior + */ +template +class Job : public ActivityT, public SupervisionT { + using upper_type = ActivityT; + using lower_type = SupervisionT; + + private: + /** + * A scope-based lock guard that manages locks on a subtree. + * + * We lock the child first and unlock it last. This ensures that the activity + * in a node is locked before the activities of any children are. + */ + class SubtreeLockGuard { + SubtreeLockGuard() { + upper_type::lock(); + lower_type::lock(); + } + ~SubtreeLockGuard() { + lower_type::unlock(); + upper_type::unlock(); + } + }; + + public: + /** + * Constructor. + * + * We forward any arguments to the upper half (the activity). Supervision + * classes have fixed constructors. + */ + template + explicit Job(Args&&... args) + : upper_type(std::forward(args)...) + , lower_type(*static_cast(this)) { + } + + bool quiescent() { + SubtreeLockGuard lg; + if (lower_type::is_active() || upper_type::is_active()) { + return false; + } + return lower_type::is_quiescent() || upper_type::is_quiescent(); + } + bool active() { + return lower_type::is_active() || upper_type::is_active(); + } + bool halted() { + if (lower_type::is_active() || upper_type::is_active()) { + return false; + } + return lower_type::is_halted() && upper_type::is_halted(); + }; +}; + +/** + * The whole job system, consistently instantiated with the same mixin template + * + * Note that this is the only place that `NullMixin` is used as the default + * for a template argument. This singular occurrence ensures that each of the + * job class is instantiated consistently with the others. + */ +template +struct JobSystem { + /** + * `JobParent` should be used only as an interface, not as a base class. + */ + using JobParent = typename Mixin::ParentMixin; + + using parent_type = typename Mixin::ParentMixin; + using child_type = typename Mixin::ChildMixin; + using nonparent_type = typename Mixin::NonparentMixin; + using nonchild_type = typename Mixin::NonchildMixin; + /** + * The root class of a job tree. + */ + class JobRoot : public Job { + public: + template + explicit JobRoot(Args&&... args) + : Job(std::forward(args)...) { + } + }; + + /** + * The branch class of a job tree. + */ + class JobBranch : public Job { + public: + template + explicit JobBranch(JobParent& parent, Args&&... args) + : Job( + parent, std::forward(args)...) { + } + }; + + /** + * The leaf class of a job tree. + */ + class JobLeaf : public Job { + public: + template + explicit JobLeaf(JobParent& parent, Args&&... args) + : Job( + parent, std::forward(args)...) { + } + }; + + /** + * A degenerate tree, with exactly one element. + * + * This class is essentially a `job::Activity` but with the same interface as + * the other job classes. + */ + class JobIsolate : public Job { + public: + template + explicit JobIsolate(Args&&... args) + : Job( + std::forward(args)...) { + } + }; +}; + +} // namespace tiledb::common::job + +#endif // TILEDB_CONTEXT_JOB_H diff --git a/tiledb/sm/storage_manager/storage_manager.cc b/tiledb/sm/storage_manager/storage_manager.cc index dad55991dc5..e3ff2481d7a 100644 --- a/tiledb/sm/storage_manager/storage_manager.cc +++ b/tiledb/sm/storage_manager/storage_manager.cc @@ -57,9 +57,7 @@ class StorageManagerException : public StatusException { * the constructor signature as yet. */ StorageManagerCanonical::StorageManagerCanonical( - ContextResources& resources, - const shared_ptr&, // unused - const Config& config) + ContextResources& resources, const Config& config) : vfs_(resources.vfs()) , cancellation_in_progress_(false) , config_(config) @@ -73,7 +71,7 @@ StorageManagerCanonical::StorageManagerCanonical( StorageManagerCanonical::~StorageManagerCanonical() { global_state::GlobalState::GetGlobalState().unregister_storage_manager(this); - throw_if_not_ok(cancel_all_tasks()); + cancel_all_tasks(); bool found{false}; bool use_malloc_trim{false}; @@ -89,7 +87,7 @@ StorageManagerCanonical::~StorageManagerCanonical() { /* API */ /* ****************************** */ -Status StorageManagerCanonical::cancel_all_tasks() { +void StorageManagerCanonical::cancel_all_tasks() { // Check if there is already a "cancellation" in progress. bool handle_cancel = false; { @@ -112,8 +110,6 @@ Status StorageManagerCanonical::cancel_all_tasks() { std::unique_lock lck(cancellation_in_progress_mtx_); cancellation_in_progress_ = false; } - - return Status::Ok(); } bool StorageManagerCanonical::cancellation_in_progress() const { diff --git a/tiledb/sm/storage_manager/storage_manager_canonical.h b/tiledb/sm/storage_manager/storage_manager_canonical.h index 11e80b06246..fb4c9d03d52 100644 --- a/tiledb/sm/storage_manager/storage_manager_canonical.h +++ b/tiledb/sm/storage_manager/storage_manager_canonical.h @@ -50,30 +50,39 @@ #include "tiledb/common/status.h" #include "tiledb/common/thread_pool/thread_pool.h" #include "tiledb/sm/array/array_directory.h" +#include "tiledb/sm/config/config.h" #include "tiledb/sm/enums/walk_order.h" #include "tiledb/sm/filesystem/uri.h" -#include "tiledb/sm/group/group.h" #include "tiledb/sm/misc/cancelable_tasks.h" #include "tiledb/sm/misc/types.h" #include "tiledb/sm/storage_manager/context_resources.h" -using namespace tiledb::common; - namespace tiledb::sm { +class Context; class Query; enum class EncryptionType : uint8_t; /** The storage manager that manages pretty much nothing in TileDB. */ class StorageManagerCanonical { - public: + /* + * Friend declaration allows `class Context` to construct `StorageManager` + * objects. + */ + friend class Context; + friend struct ContextBase; // temporary class + /* ********************************* */ /* CONSTRUCTORS & DESTRUCTORS */ /* ********************************* */ /** - * Complete, C.41-compliant constructor + * Constructor is private, available only to its friend `class Context`. + * + * As preparations to `StorageManager` proceed, making the constructor + * `private` is a natural step. Exactly one class still has a `StorageManager` + * member variable, and that's `class Context`. * * The `resources` argument is only used for its `vfs()` member function. This * is the VFS instance that's waited on in `cancel_all_tasks`. @@ -82,10 +91,7 @@ class StorageManagerCanonical { * @param resources Resource object from associated context * @param config The configuration parameters. */ - StorageManagerCanonical( - ContextResources& resources, - const shared_ptr& logger, - const Config& config); + StorageManagerCanonical(ContextResources& resources, const Config& config); public: /** Destructor. */ @@ -99,7 +105,7 @@ class StorageManagerCanonical { /* ********************************* */ /** Cancels all background tasks. */ - Status cancel_all_tasks(); + void cancel_all_tasks(); /** Returns true while all tasks are being cancelled. */ bool cancellation_in_progress() const; diff --git a/tiledb/sm/storage_manager/test/CMakeLists.txt b/tiledb/sm/storage_manager/test/CMakeLists.txt new file mode 100644 index 00000000000..7b6b98e70f6 --- /dev/null +++ b/tiledb/sm/storage_manager/test/CMakeLists.txt @@ -0,0 +1,45 @@ +# +# tiledb/sm/storage_manager/test/CMakeLists.txt +# +# The MIT License +# +# Copyright (c) 2023-2024 TileDB, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +include(unit_test) + +# +# Test runner `unit_context` +# +commence(unit_test context) + this_target_sources(unit_job.cc) + # + # Temporary substitute for a `job` object library + # + this_target_sources( + ../cancellation_source.cc + ../context.cc + ../job.cc + ) + this_target_object_libraries(context_resources) + # This is defined in c_api_test_support, but we're using it temporarily here + # in order to test the code that will allow the stub to go away + this_target_object_libraries(storage_manager_stub) +conclude(unit_test) diff --git a/tiledb/sm/storage_manager/test/compile_job_stub_main.cc b/tiledb/sm/storage_manager/test/compile_job_stub_main.cc new file mode 100644 index 00000000000..11f75757545 --- /dev/null +++ b/tiledb/sm/storage_manager/test/compile_job_stub_main.cc @@ -0,0 +1,36 @@ +/** + * @file compile_job_stub_main.cc + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include "../job.h" + +int main() { + tiledb::sm::Config config; + tiledb::sm::Context context(config); + tiledb::sm::Job job(context); + return 0; +} diff --git a/tiledb/sm/storage_manager/test/unit_job.cc b/tiledb/sm/storage_manager/test/unit_job.cc new file mode 100644 index 00000000000..968f5c63de1 --- /dev/null +++ b/tiledb/sm/storage_manager/test/unit_job.cc @@ -0,0 +1,436 @@ +/* + * @file tiledb/sm/storage_manager/test/unit_job.cpp + * + * @section LICENSE + * + * The MIT License + * + * @copyright Copyright (c) 2024 TileDB, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + * @section DESCRIPTION + * + * This file provides unit tests for `template class Registry`. + * + * @section Maturity + * + * This test file combines tests for both the job system as well as the + * production mixin for that job system. The job system and the mixin are in + * separate files and in different namespaces. This structure anticipates moving + * the generic job system out of this directory, but these tests have not yet + * been separated. + */ + +#include + +#include "../context.h" +#include "../job.h" +#include "tiledb/common/logger.h" + +/* + * This `using` declarations pulls in production version of JobParent, JobRoot, + * and all the others. + */ +using namespace tiledb::sm; +namespace tcj = tiledb::common::job; + +//------------------------------------------------------- +// +//------------------------------------------------------- +/* + * This block is to test the constructors for all the intermediates of a job + * system. Initialization of virtual base classes is error-prone, so we are + * explicitly exercising all of them separately. + */ +namespace test { +using namespace tiledb::common::job; + +struct WhiteboxActivityBase : public ActivityBase { + WhiteboxActivityBase() + : ActivityBase() { + } +}; +using TestActivityBase = WhiteboxActivityBase; +using TestActivity = NullMixin::ActivityMixin; +using TestChildBase = ChildBase; +using TestChild = NullMixin::ChildMixin; +using TestNonchildBase = NonchildBase; +using TestNonchild = NullMixin::NonchildMixin; + +template +struct WhiteboxSupervisionBase : public SupervisionBase { + WhiteboxSupervisionBase(TestActivity& activity) + : SupervisionBase(activity) { + } +}; +using TestSupervisionBase = SupervisionBase; +using TestSupervision = NullMixin::SupervisionMixin; +using TestParentBase = ParentBase; +using TestParent = NullMixin::ParentMixin; +using TestNonparentBase = ParentBase; +using TestNonparent = NullMixin::NonparentMixin; + +TEST_CASE("Default `Supervision` Hierarchy - construct") { + TestActivity act{}; + SECTION("Supervision Base") { + TestSupervisionBase svb{act}; + } + SECTION("Supervision Mixin") { + TestSupervision svm{act}; + } + SECTION("Parent Base") { + TestParentBase pb{act}; + } + SECTION("Parent Mixin") { + TestParent p{act}; + } + SECTION("Nonparent Base") { + TestNonparentBase npb{act}; + } + SECTION("Nonparent Mixin") { + TestNonparent np{act}; + } +} + +TEST_CASE("Default `Activity` Hierarchy - construct") { + SECTION("Activity Base") { + TestActivityBase ab{}; + } + SECTION("Activity Mixin") { + TestActivity am{}; + } + SECTION("Child") { + TestActivity p_act{}; + TestParent p{p_act}; + SECTION("its Base") { + auto cb{TestChildBase{p}}; + } + SECTION("itself") { + TestChild c{p}; + } + } + SECTION("Nonchild Base") { + TestNonchildBase ncb{}; + } + SECTION("Nonchild") { + TestNonchild nc{}; + } +} +} // namespace test + +/* + * Construction tests for the default `JobSystem` + */ +namespace test { + +/* + * This namespace uses the default job system with it's default template + * argument. Internally this is the NullMixin, but we don't reference it + * explicitly in this statement in order to ensure the default works. + */ +using JS = JobSystem<>; + +TEST_CASE("common::JobSystem - construct") { + SECTION("Rooted classes") { + JS::JobRoot root{}; + SECTION("Root itself") { + // Just the initialization + } + SECTION("Root + Branch") { + JS::JobBranch y{root}; + } + SECTION("Root + Leaf") { + JS::JobLeaf y{root}; + } + SECTION("Root + Branch + Leaf") { + JS::JobBranch y{root}; + JS::JobLeaf z{y}; + } + } + SECTION("Isolate class") { + JS::JobIsolate x{}; + } +} +} // namespace test + +/** + * Direct access to the Child class inside the production job system. We use the + * production job system in order to test with `class Context`. + */ +using DirectTestChild = tiledb::sm::job::JobResourceMixin::ChildMixin; + +struct TestJobChild : public DirectTestChild { + explicit TestJobChild(Context& context) + : DirectTestChild(context) { + } + + static std::shared_ptr factory(Context& context) { + auto job{std::make_shared(context)}; + job->register_shared_ptr(job); + return job; + } +}; + +TEST_CASE("job::Child - construct 0") { + Config config{}; + Context context{config}; + /* + * This is never how we'd construct a job that needs to be fully registered, + * since it's not managed by a `shared_ptr`. + */ + auto job{TestJobChild(context)}; +} + +TEST_CASE("job::Child - construct 1") { + Config config{}; + Context context{config}; + auto job{TestJobChild::factory(context)}; +} + +struct TestJobChildFactory { + Config config_; + Context context_; + + TestJobChildFactory() + : config_() + , context_(config_) { + } + + std::shared_ptr operator()() { + return TestJobChild::factory(context_); + } +}; + +TEST_CASE("job::Child - construct 2") { + TestJobChildFactory jf{}; + auto job{jf()}; +} + +TEST_CASE("job::Child - construct and size") { + TestJobChildFactory jf{}; + CHECK(jf.context_.number_of_jobs() == 0); + auto job{jf()}; + CHECK(jf.context_.number_of_jobs() == 1); + job.reset(); + CHECK(jf.context_.number_of_jobs() == 0); +} + + +class TestJobRootBase { + protected: + Config config_{}; + mutable ContextResources resources_{ + config_, std::make_shared("log"), 1, 1, ""}; + StorageManager sm_{resources_, config_}; + TestJobRootBase() = default; +}; + +class TestJobRoot : protected TestJobRootBase, public JobRoot { + using Base = TestJobRootBase; + public: + TestJobRoot() + : TestJobRootBase(), JobRoot(Base::sm_) { + } + ~TestJobRoot() override = default; + ContextResources& resources() const override { + return Base::resources_; + } +}; +static_assert(TestJobRoot::is_parent); +static_assert(!TestJobRoot::is_child); + +TEST_CASE("TestJobRoot - construct") { + TestJobRoot root{}; +} + +class TestJobBranch : public JobBranch { + public: + TestJobBranch(JobParent& parent) + : JobBranch(parent) { + } + ~TestJobBranch() override = default; + ContextResources& resources() const override { + return parent().resources(); + } +}; +static_assert(TestJobBranch::is_parent); +static_assert(TestJobBranch::is_child); + +TEST_CASE("TestJobBranch - construct from root") { + TestJobRoot root{}; + TestJobBranch first{root}; +} + +TEST_CASE("TestJobBranch - construct from branch") { + TestJobRoot root{}; + TestJobBranch first{root}; + // Need a cast because otherwise it means the copy constructor + TestJobBranch second{static_cast(first)}; +} + +class TestJobLeaf : public JobLeaf { + public: + TestJobLeaf(JobParent& parent) + : JobLeaf(parent) { + } +}; +static_assert(!TestJobLeaf::is_parent); +static_assert(TestJobLeaf::is_child); + +TEST_CASE("TestJobLeaf - construct from root") { + TestJobRoot root{}; + TestJobLeaf first{root}; +} + +TEST_CASE("TestJobLeaf - construct from branch") { + TestJobRoot root{}; + TestJobBranch branch{root}; + TestJobLeaf leaf{branch}; +} + +class TestJobIsolateBase { + protected: + Config config_{}; + ContextResources resources_{ + config_, std::make_shared("log"), 1, 1, ""}; + StorageManager sm_{resources_, config_}; + TestJobIsolateBase() = default; +}; + +class TestJobIsolate : protected TestJobIsolateBase, public JobIsolate { + using Base = TestJobIsolateBase; + public: + TestJobIsolate() + : TestJobIsolateBase(), JobIsolate(Base::sm_) { + } +}; +static_assert(!TestJobIsolate::is_parent); +static_assert(!TestJobIsolate::is_child); + +TEST_CASE("TestJobIsolate - construct") { + TestJobIsolate x{}; +} + +//------------------------------------------------------- +// Mixin +//------------------------------------------------------- + +class NotResources {}; + +struct TestMixin : public tiledb::common::job::NullMixin { + using Self = TestMixin; + struct ParentMixin; + + /** + * Mixin class for Activity + */ + struct ActivityMixin : public tcj::ActivityBase { + explicit ActivityMixin() + : ActivityBase(){}; + }; + + /** + * Mixin class for Child + */ + struct ChildMixin : public tcj::ChildBase { + ChildMixin() = delete; + explicit ChildMixin(ParentMixin& parent) + : ChildBase(parent){}; + }; + + /** + * Mixin class for Nonchild + */ + struct NonchildMixin : public tcj::NonchildBase { + explicit NonchildMixin() + : NonchildBase() { + } + }; + + /** + * Mixin class for Supervision + */ + struct SupervisionMixin : public tcj::SupervisionBase { + SupervisionMixin() = delete; + explicit SupervisionMixin(ActivityMixin& activity) + : SupervisionBase(activity){}; + }; + + /** + * Mixin class for Parent + */ + struct ParentMixin : public tcj::ParentBase { + ParentMixin() = delete; + explicit ParentMixin(ActivityMixin& activity) + : ParentBase(activity) { + } + [[nodiscard]] virtual NotResources& resources2() = 0; + }; + + /** + * Mixin class for Nonparent + */ + struct NonparentMixin : public tcj::NonparentBase { + NonparentMixin() = delete; + explicit NonparentMixin(ActivityMixin& activity) + : NonparentBase(activity) { + } + }; +}; +using mixin_job_system = tcj::JobSystem; + +class MixinJobRoot : public mixin_job_system::JobRoot { + public: + MixinJobRoot() + : JobRoot() { + } + NotResources& resources2() override { + static NotResources nothing_; + return nothing_; + } +}; + +class MixinJobBranch : public mixin_job_system::JobBranch { + public: + explicit MixinJobBranch(mixin_job_system::JobParent& p) + : JobBranch(p) { + } + NotResources& resources2() override { + static NotResources nothing_; + return nothing_; + } +}; + +class MixinJobIsolate : public mixin_job_system::JobIsolate { + public: + MixinJobIsolate() + : JobIsolate() { + } +}; + +TEST_CASE("MixinWithSupervision - construct") { + SECTION("Branch") { + MixinJobRoot x{}; + MixinJobBranch y{x}; + } + SECTION("Isolate") { + MixinJobIsolate x{}; + } +}