diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index ae6b8bb280e..179ad5229be 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -587,7 +587,7 @@ capi_return_t tiledb_group_vacuum_metadata( ensure_group_uri_argument_is_valid(group_uri); auto cfg = (config == nullptr) ? ctx->config() : config->config(); - ctx->storage_manager()->group_metadata_vacuum(group_uri, cfg); + sm::Group::vacuum_metadata(ctx->resources(), 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 2fa240c46c2..05708ac1f9e 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 @@ -61,10 +61,6 @@ class StorageManagerStub { inline Status cancel_all_tasks() { return Status{}; } - inline Status group_metadata_vacuum(const char*, const Config&) { - throw std::logic_error( - "StorageManagerStub does not support group metadata vacuum"); - } inline Status set_tag(const std::string&, const std::string&) { return Status{}; } diff --git a/tiledb/common/exception/status.h b/tiledb/common/exception/status.h index bc7eaae277e..5fe546a643a 100644 --- a/tiledb/common/exception/status.h +++ b/tiledb/common/exception/status.h @@ -5,7 +5,7 @@ * * The BSD License * - * @copyright Copyright (c) 2017-2022 TileDB, Inc. + * @copyright Copyright (c) 2017-2024 TileDB, Inc. * Copyright (c) 2011 The LevelDB Authors. All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -60,8 +60,7 @@ #include "tiledb/common/common-std.h" #include "tiledb/common/heap_memory.h" -namespace tiledb { -namespace common { +namespace tiledb::common { #define RETURN_NOT_OK(s) \ do { \ @@ -250,10 +249,6 @@ inline Status Status_Ok() { inline Status Status_Error(const std::string& msg) { return {"Error", msg}; }; -/** Return a StorageManager error class Status with a given message **/ -inline Status Status_StorageManagerError(const std::string& msg) { - return {"[TileDB::StorageManager] Error", msg}; -} /** Return a FragmentMetadata error class Status with a given message **/ inline Status Status_FragmentMetadataError(const std::string& msg) { return {"[TileDB::FragmentMetadata] Error", msg}; @@ -433,7 +428,6 @@ inline Status Status_TaskError(const std::string& msg) { inline Status Status_RangeError(const std::string& msg) { return {"[TileDB::Range] Error", msg}; } -} // namespace common -} // namespace tiledb +} // namespace tiledb::common #endif // TILEDB_STATUS_H diff --git a/tiledb/common/logger.cc b/tiledb/common/logger.cc index 2c7e5235662..6b025044957 100644 --- a/tiledb/common/logger.cc +++ b/tiledb/common/logger.cc @@ -5,7 +5,7 @@ * * The MIT License * - * @copyright Copyright (c) 2017-2021 TileDB, Inc. + * @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 @@ -147,10 +147,6 @@ Status Logger::status(const Status& st) { return st; } -void Logger::status_no_return_value(const Status& st) { - logger_->error(st.message()); -} - void Logger::trace(const std::string& msg) { logger_->trace(msg); } diff --git a/tiledb/common/logger.h b/tiledb/common/logger.h index 62fc53a3716..dde199fb4b5 100644 --- a/tiledb/common/logger.h +++ b/tiledb/common/logger.h @@ -5,7 +5,7 @@ * * The MIT License * - * @copyright Copyright (c) 2017-2021 TileDB, Inc. + * @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 @@ -48,8 +48,7 @@ namespace spdlog { class logger; } -namespace tiledb { -namespace common { +namespace tiledb::common { /** Definition of class Logger. */ class Logger { @@ -315,13 +314,6 @@ class Logger { */ Status status(const Status& st); - /** - * Log a message from a Status object without returning it. - * - * @param st The Status object to log - */ - void status_no_return_value(const Status& st); - /** * Log an error and exit with a non-zero status. * @@ -457,8 +449,7 @@ inline Status logger_format_from_string( return Status::Ok(); } -} // namespace common -} // namespace tiledb +} // namespace tiledb::common // Also include the public-permissible logger functions here. #include "tiledb/common/logger_public.h" diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc index 1deb57ecf1b..dc7ca4fc6e8 100644 --- a/tiledb/sm/array/array.cc +++ b/tiledb/sm/array/array.cc @@ -585,12 +585,11 @@ Status Array::open( array_dir_timestamp_end_, ArrayDirectoryMode::SCHEMA_ONLY)); } - auto&& [st, array_schema_latest, array_schemas] = open_for_writes(); - throw_if_not_ok(st); // Set schemas - set_array_schema_latest(array_schema_latest.value()); - set_array_schemas_all(std::move(array_schemas.value())); + auto&& [array_schema_latest, array_schemas] = open_for_writes(); + set_array_schema_latest(array_schema_latest); + set_array_schemas_all(std::move(array_schemas)); // Set the timestamp opened_array_->metadata().reset(timestamp_end_opened_at()); @@ -606,12 +605,11 @@ Status Array::open( array_dir_timestamp_end_, ArrayDirectoryMode::READ)); } - auto&& [st, latest, array_schemas] = open_for_writes(); - throw_if_not_ok(st); // Set schemas - set_array_schema_latest(latest.value()); - set_array_schemas_all(std::move(array_schemas.value())); + auto&& [latest, array_schemas] = open_for_writes(); + set_array_schema_latest(latest); + set_array_schemas_all(std::move(array_schemas)); auto version = array_schema_latest().version(); if (query_type == QueryType::DELETE && @@ -1842,19 +1840,15 @@ Array::open_for_reads_without_fragments() { } tuple< - Status, - optional>, - optional>>> + shared_ptr, + std::unordered_map>> Array::open_for_writes() { auto timer_se = resources_.stats().start_timer("array_open_write_load_schemas"); // Checks - if (!resources_.vfs().supports_uri_scheme(array_uri_)) - return { - resources_.logger()->status(Status_StorageManagerError( - "Cannot open array; URI scheme unsupported.")), - nullopt, - nullopt}; + if (!resources_.vfs().supports_uri_scheme(array_uri_)) { + throw ArrayException("Cannot open array; URI scheme unsupported."); + } // Load array schemas auto&& [array_schema_latest, array_schemas_all] = @@ -1867,31 +1861,21 @@ Array::open_for_writes() { auto version = array_schema_latest->version(); if constexpr (is_experimental_build) { if (version != constants::format_version) { - std::stringstream err; - err << "Cannot open array for writes; Array format version ("; - err << version; - err << ") is not the library format version ("; - err << constants::format_version << ")"; - return { - resources_.logger()->status(Status_StorageManagerError(err.str())), - nullopt, - nullopt}; + throw ArrayException( + "Cannot open array for writes; Array format version (" + + std::to_string(version) + ") is not the library format version (" + + std::to_string(constants::format_version) + ")"); } } else { if (version > constants::format_version) { - std::stringstream err; - err << "Cannot open array for writes; Array format version ("; - err << version; - err << ") is newer than library format version ("; - err << constants::format_version << ")"; - return { - resources_.logger()->status(Status_StorageManagerError(err.str())), - nullopt, - nullopt}; + throw ArrayException( + "Cannot open array for writes; Array format version (" + + std::to_string(version) + ") is newer than library format version (" + + std::to_string(constants::format_version) + ")"); } } - return {Status::Ok(), array_schema_latest, array_schemas_all}; + return {array_schema_latest, array_schemas_all}; } void Array::clear_last_max_buffer_sizes() { @@ -2326,12 +2310,10 @@ void Array::ensure_array_is_valid_for_delete(const URI& uri) { void ensure_supported_schema_version_for_read(format_version_t version) { // We do not allow reading from arrays written by newer version of TileDB if (version > constants::format_version) { - std::stringstream err; - err << "Cannot open array for reads; Array format version ("; - err << version; - err << ") is newer than library format version ("; - err << constants::format_version << ")"; - throw Status_StorageManagerError(err.str()); + throw ArrayException( + "Cannot open array for reads; Array format version (" + + std::to_string(version) + ") is newer than library format version (" + + std::to_string(constants::format_version) + ")"); } } diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h index 2e2ccad56e3..dc955493a85 100644 --- a/tiledb/sm/array/array.h +++ b/tiledb/sm/array/array.h @@ -1200,19 +1200,18 @@ class Array { std::unordered_map>> open_for_reads_without_fragments(); - /** Opens an array for writes. + /** + * Opens an array for writes. * * @param array The array to open. - * @return tuple of Status, latest ArraySchema and map of all array schemas - * Status Ok on success, else error + * @return tuple of latest ArraySchema and map of all array schemas * ArraySchema The array schema to be retrieved after the * array is opened. * ArraySchemaMap Map of all array schemas found keyed by name */ tuple< - Status, - optional>, - optional>>> + shared_ptr, + std::unordered_map>> open_for_writes(); /** Clears the cached max buffer sizes and subarray. */ diff --git a/tiledb/sm/consolidator/array_meta_consolidator.cc b/tiledb/sm/consolidator/array_meta_consolidator.cc index 69a62057163..6cf4f6f110e 100644 --- a/tiledb/sm/consolidator/array_meta_consolidator.cc +++ b/tiledb/sm/consolidator/array_meta_consolidator.cc @@ -120,7 +120,7 @@ Status ArrayMetaConsolidator::consolidate( void ArrayMetaConsolidator::vacuum(const char* array_name) { if (array_name == nullptr) { - throw Status_StorageManagerError( + throw std::invalid_argument( "Cannot vacuum array metadata; Array name cannot be null"); } diff --git a/tiledb/sm/consolidator/commits_consolidator.cc b/tiledb/sm/consolidator/commits_consolidator.cc index c5ab1a31758..2544007ff71 100644 --- a/tiledb/sm/consolidator/commits_consolidator.cc +++ b/tiledb/sm/consolidator/commits_consolidator.cc @@ -103,7 +103,7 @@ Status CommitsConsolidator::consolidate( void CommitsConsolidator::vacuum(const char* array_name) { if (array_name == nullptr) { - throw Status_StorageManagerError( + throw std::invalid_argument( "Cannot vacuum array metadata; Array name cannot be null"); } diff --git a/tiledb/sm/consolidator/fragment_meta_consolidator.cc b/tiledb/sm/consolidator/fragment_meta_consolidator.cc index 39f0f1d7818..19c7a9bae44 100644 --- a/tiledb/sm/consolidator/fragment_meta_consolidator.cc +++ b/tiledb/sm/consolidator/fragment_meta_consolidator.cc @@ -181,7 +181,7 @@ Status FragmentMetaConsolidator::consolidate( void FragmentMetaConsolidator::vacuum(const char* array_name) { if (array_name == nullptr) { - throw Status_StorageManagerError( + throw std::invalid_argument( "Cannot vacuum fragment metadata; Array name cannot be null"); } diff --git a/tiledb/sm/consolidator/group_meta_consolidator.cc b/tiledb/sm/consolidator/group_meta_consolidator.cc index 89677e8b8ee..7436a2a462f 100644 --- a/tiledb/sm/consolidator/group_meta_consolidator.cc +++ b/tiledb/sm/consolidator/group_meta_consolidator.cc @@ -110,7 +110,7 @@ Status GroupMetaConsolidator::consolidate( void GroupMetaConsolidator::vacuum(const char* group_name) { if (group_name == nullptr) { - throw Status_StorageManagerError( + throw std::invalid_argument( "Cannot vacuum group metadata; Group name cannot be null"); } diff --git a/tiledb/sm/group/group.cc b/tiledb/sm/group/group.cc index c9803b67675..e2cca0923f5 100644 --- a/tiledb/sm/group/group.cc +++ b/tiledb/sm/group/group.cc @@ -599,6 +599,28 @@ Status Group::consolidate_metadata( group_name, EncryptionType::NO_ENCRYPTION, nullptr, 0); } +void Group::vacuum_metadata( + ContextResources& resources, const char* group_name, const Config& config) { + // Check group URI + URI group_uri(group_name); + if (group_uri.is_invalid()) { + throw GroupException("Cannot vacuum group metadata; Invalid URI"); + } + + // Check if group exists + ObjectType obj_type; + throw_if_not_ok(object_type(resources, group_uri, &obj_type)); + + if (obj_type != ObjectType::GROUP) { + throw GroupException("Cannot vacuum group metadata; Group does not exist"); + } + + StorageManager sm(resources, resources.logger(), config); + auto consolidator = + Consolidator::create(ConsolidationMode::GROUP_META, config, &sm); + consolidator->vacuum(group_name); +} + const EncryptionKey* Group::encryption_key() const { return encryption_key_.get(); } diff --git a/tiledb/sm/group/group.h b/tiledb/sm/group/group.h index 046c9dc1fd2..1480178bb5b 100644 --- a/tiledb/sm/group/group.h +++ b/tiledb/sm/group/group.h @@ -254,6 +254,21 @@ class Group { const char* group_name, const Config& config); + /** + * Vacuums the consolidated metadata files of a group. + * + * @param resources The context resources. + * @param group_name The name of the group whose metadata will be + * vacuumed. + * @param config Configuration parameters for vacuuming + * (`nullptr` means default, which will use the config associated with + * this instance). + */ + static void vacuum_metadata( + ContextResources& resources, + 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/query/writers/unordered_writer.cc b/tiledb/sm/query/writers/unordered_writer.cc index 1c058cf3ae8..b67b9b1b8a6 100644 --- a/tiledb/sm/query/writers/unordered_writer.cc +++ b/tiledb/sm/query/writers/unordered_writer.cc @@ -273,7 +273,7 @@ Status UnorderedWriter::check_coord_dups() const { return Status::Ok(); }); - RETURN_NOT_OK_ELSE(status, logger_->status_no_return_value(status)); + RETURN_NOT_OK_ELSE(status, logger_->error(status.message())); return Status::Ok(); } diff --git a/tiledb/sm/query/writers/writer_base.cc b/tiledb/sm/query/writers/writer_base.cc index aeb122a1164..be6a4ada95e 100644 --- a/tiledb/sm/query/writers/writer_base.cc +++ b/tiledb/sm/query/writers/writer_base.cc @@ -345,7 +345,7 @@ Status WriterBase::calculate_hilbert_values( return Status::Ok(); }); - RETURN_NOT_OK_ELSE(status, logger_->status_no_return_value(status)); + RETURN_NOT_OK_ELSE(status, logger_->error(status.message())); return Status::Ok(); } diff --git a/tiledb/sm/storage_manager/context.cc b/tiledb/sm/storage_manager/context.cc index 7377d6c9011..8e6a38a7811 100644 --- a/tiledb/sm/storage_manager/context.cc +++ b/tiledb/sm/storage_manager/context.cc @@ -5,7 +5,7 @@ * * The MIT License * - * @copyright Copyright (c) 2017-2021 TileDB, Inc. + * @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 @@ -37,8 +37,14 @@ using namespace tiledb::common; -namespace tiledb { -namespace sm { +namespace tiledb::sm { + +class ContextException : public StatusException { + public: + explicit ContextException(const std::string& message) + : StatusException("Context", message) { + } +}; static common::Logger::Level get_log_level(const Config& config); @@ -67,7 +73,7 @@ Context::Context(const Config& config) /* * Logger class is not yet C.41-compliant */ - throw_if_not_ok(init_loggers(config)); + init_loggers(config); } /* ****************************** */ @@ -111,9 +117,10 @@ 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_->status_no_return_value(Status_StorageManagerError( + logger_->error( + "[Context::get_config_thread_count] " "Config parameter \"sm.num_async_threads\" has been removed; use " - "config parameter \"sm.compute_concurrency_level\".")); + "config parameter \"sm.compute_concurrency_level\"."); } uint64_t num_reader_threads{0}; @@ -121,9 +128,10 @@ 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_->status_no_return_value(Status_StorageManagerError( + logger_->error( + "[Context::get_config_thread_count] " "Config parameter \"sm.num_reader_threads\" has been removed; use " - "config parameter \"sm.compute_concurrency_level\".")); + "config parameter \"sm.compute_concurrency_level\"."); } uint64_t num_writer_threads{0}; @@ -131,9 +139,10 @@ 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_->status_no_return_value(Status_StorageManagerError( + logger_->error( + "[Context::get_config_thread_count] " "Config parameter \"sm.num_writer_threads\" has been removed; use " - "config parameter \"sm.compute_concurrency_level\".")); + "config parameter \"sm.compute_concurrency_level\"."); } uint64_t num_vfs_threads{0}; @@ -141,9 +150,10 @@ 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_->status_no_return_value(Status_StorageManagerError( + logger_->error( + "[Context::get_config_thread_count] " "Config parameter \"sm.num_vfs_threads\" has been removed; use " - "config parameter \"sm.io_concurrency_level\".")); + "config parameter \"sm.io_concurrency_level\"."); } // The "sm.num_tbb_threads" has been deprecated. Users may still be setting @@ -156,9 +166,10 @@ Status Context::get_config_thread_count( if (found) { config_thread_count = std::max(config_thread_count, static_cast(num_tbb_threads)); - logger_->status_no_return_value(Status_StorageManagerError( + logger_->error( + "[Context::get_config_thread_count] " "Config parameter \"sm.num_tbb_threads\" has been removed; use " - "config parameter \"sm.io_concurrency_level\".")); + "config parameter \"sm.io_concurrency_level\"."); } config_thread_count_ret = static_cast(config_thread_count); @@ -210,16 +221,16 @@ size_t Context::get_io_thread_count(const Config& config) { std::max(config_thread_count, io_concurrency_level)); } -Status Context::init_loggers(const Config& config) { +void Context::init_loggers(const Config& config) { // temporarily set level to error so that possible errors reading // configuration are visible to the user logger_->set_level(Logger::Level::ERR); const char* format_conf; - RETURN_NOT_OK(config.get("config.logging_format", &format_conf)); + throw_if_not_ok(config.get("config.logging_format", &format_conf)); assert(format_conf != nullptr); Logger::Format format = Logger::Format::DEFAULT; - RETURN_NOT_OK(logger_format_from_string(format_conf, &format)); + throw_if_not_ok(logger_format_from_string(format_conf, &format)); global_logger(format); logger_->set_format(static_cast(format)); @@ -227,18 +238,16 @@ Status Context::init_loggers(const Config& config) { // set logging level from config bool found = false; uint32_t level = static_cast(Logger::Level::ERR); - RETURN_NOT_OK(config.get("config.logging_level", &level, &found)); + throw_if_not_ok(config.get("config.logging_level", &level, &found)); assert(found); if (level > static_cast(Logger::Level::TRACE)) { - return logger_->status(Status_StorageManagerError( + throw ContextException( "Cannot set logger level; Unsupported level:" + std::to_string(level) + - "set in configuration")); + "set in configuration"); } global_logger().set_level(static_cast(level)); logger_->set_level(static_cast(level)); - - return Status::Ok(); } common::Logger::Level get_log_level(const Config& config) { @@ -260,5 +269,4 @@ common::Logger::Level get_log_level(const Config& config) { } } -} // namespace sm -} // namespace tiledb +} // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/context.h b/tiledb/sm/storage_manager/context.h index 5ef63c33ccf..61002a156ec 100644 --- a/tiledb/sm/storage_manager/context.h +++ b/tiledb/sm/storage_manager/context.h @@ -202,9 +202,8 @@ class Context { * Initializes global and local logger. * * @param config The configuration parameters. - * @return Status */ - Status init_loggers(const Config& config); + void init_loggers(const Config& config); }; } // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/storage_manager.cc b/tiledb/sm/storage_manager/storage_manager.cc index 30bec13652d..0bf601483aa 100644 --- a/tiledb/sm/storage_manager/storage_manager.cc +++ b/tiledb/sm/storage_manager/storage_manager.cc @@ -136,11 +136,8 @@ Status StorageManagerCanonical::async_push_query(Query* query) { &resources_.compute_tp(), [this, query]() { // Process query. - Status st = query_submit(query); - if (!st.ok()) { - resources_.logger()->status_no_return_value(st); - } - return st; + throw_if_not_ok(query_submit(query)); + return Status::Ok(); }, [query]() { // Task was cancelled. This is safe to perform in a separate thread, @@ -404,27 +401,4 @@ Status StorageManagerCanonical::set_default_tags() { return Status::Ok(); } -void StorageManagerCanonical::group_metadata_vacuum( - const char* group_name, const Config& config) { - // Check group URI - URI group_uri(group_name); - if (group_uri.is_invalid()) { - throw Status_StorageManagerError( - "Cannot vacuum group metadata; Invalid URI"); - } - - // Check if group exists - ObjectType obj_type; - throw_if_not_ok(object_type(resources_, group_uri, &obj_type)); - - if (obj_type != ObjectType::GROUP) { - throw Status_StorageManagerError( - "Cannot vacuum group metadata; Group does not exist"); - } - - auto consolidator = - Consolidator::create(ConsolidationMode::GROUP_META, config, this); - consolidator->vacuum(group_name); -} - } // namespace tiledb::sm diff --git a/tiledb/sm/storage_manager/storage_manager_canonical.h b/tiledb/sm/storage_manager/storage_manager_canonical.h index def3a5f6bfc..f024abd2f76 100644 --- a/tiledb/sm/storage_manager/storage_manager_canonical.h +++ b/tiledb/sm/storage_manager/storage_manager_canonical.h @@ -250,17 +250,6 @@ class StorageManagerCanonical { return resources_; } - /** - * Vacuums the consolidated metadata files of a group. - * - * @param group_name The name of the group whose metadata will be - * vacuumed. - * @param config Configuration parameters for vacuuming - * (`nullptr` means default, which will use the config associated with - * this instance). - */ - void group_metadata_vacuum(const char* group_name, const Config& config); - private: /* ********************************* */ /* PRIVATE DATATYPES */