diff --git a/tiledb/api/c_api/group/group_api.cc b/tiledb/api/c_api/group/group_api.cc index 3f2246b4b02..91b66589787 100644 --- a/tiledb/api/c_api/group/group_api.cc +++ b/tiledb/api/c_api/group/group_api.cc @@ -114,8 +114,7 @@ capi_return_t tiledb_group_open( tiledb_group_handle_t* group, tiledb_query_type_t query_type) { ensure_group_is_valid(group); - throw_if_not_ok( - group->group().open(static_cast(query_type))); + group->group().open(static_cast(query_type)); return TILEDB_OK; } @@ -123,7 +122,7 @@ capi_return_t tiledb_group_open( capi_return_t tiledb_group_close(tiledb_group_handle_t* group) { ensure_group_is_valid(group); - throw_if_not_ok(group->group().close()); + group->group().close(); return TILEDB_OK; } @@ -270,8 +269,7 @@ capi_return_t tiledb_group_add_member( name_optional = name; } - throw_if_not_ok( - group->group().mark_member_for_addition(uri, relative, name_optional)); + group->group().mark_member_for_addition(uri, relative, name_optional); return TILEDB_OK; } @@ -281,7 +279,7 @@ capi_return_t tiledb_group_remove_member( ensure_group_is_valid(group); ensure_name_argument_is_valid(name_or_uri); - throw_if_not_ok(group->group().mark_member_for_removal(name_or_uri)); + group->group().mark_member_for_removal(name_or_uri); return TILEDB_OK; } @@ -463,8 +461,7 @@ capi_return_t tiledb_group_get_query_type( ensure_output_pointer_is_valid(query_type); // Get query_type - tiledb::sm::QueryType type; - throw_if_not_ok(group->group().get_query_type(&type)); + tiledb::sm::QueryType type = group->group().get_query_type(); *query_type = static_cast(type); diff --git a/tiledb/sm/consolidator/group_meta_consolidator.cc b/tiledb/sm/consolidator/group_meta_consolidator.cc index b5121b29a15..89677e8b8ee 100644 --- a/tiledb/sm/consolidator/group_meta_consolidator.cc +++ b/tiledb/sm/consolidator/group_meta_consolidator.cc @@ -70,15 +70,18 @@ Status GroupMetaConsolidator::consolidate( auto group_uri = URI(group_name); Group group_for_reads( storage_manager_->resources(), group_uri, storage_manager_); - RETURN_NOT_OK(group_for_reads.open( - QueryType::READ, config_.timestamp_start_, config_.timestamp_end_)); + group_for_reads.open( + QueryType::READ, config_.timestamp_start_, config_.timestamp_end_); // Open group for writing Group group_for_writes( storage_manager_->resources(), group_uri, storage_manager_); - RETURN_NOT_OK_ELSE( - group_for_writes.open(QueryType::WRITE), - throw_if_not_ok(group_for_reads.close())); + + try { + group_for_writes.open(QueryType::WRITE); + } catch (...) { + group_for_reads.close(); + } // Copy-assign the read metadata into the metadata of the group for writes auto metadata_r = group_for_reads.metadata(); @@ -95,8 +98,8 @@ Status GroupMetaConsolidator::consolidate( auto data = ss.str(); // Close groups - throw_if_not_ok(group_for_reads.close()); - throw_if_not_ok(group_for_writes.close()); + group_for_reads.close(); + group_for_writes.close(); // Write vacuum file throw_if_not_ok(resources_.vfs().write(vac_uri, data.c_str(), data.size())); diff --git a/tiledb/sm/group/group.cc b/tiledb/sm/group/group.cc index d274a03e224..47e04ff83f2 100644 --- a/tiledb/sm/group/group.cc +++ b/tiledb/sm/group/group.cc @@ -83,16 +83,16 @@ Group::Group( memory_tracker_->set_type(MemoryTrackerType::GROUP); } -Status Group::open( +void Group::open( QueryType query_type, uint64_t timestamp_start, uint64_t timestamp_end) { // Checks if (is_open_) { - return Status_GroupError("Cannot open group; Group already open"); + throw GroupStatusException("Cannot open group; Group already open"); } if (query_type != QueryType::READ && query_type != QueryType::WRITE && query_type != QueryType::MODIFY_EXCLUSIVE) { - return Status_GroupError("Cannot open group; Unsupported query type"); + throw GroupStatusException("Cannot open group; Unsupported query type"); } if (timestamp_end == UINT64_MAX) { @@ -126,7 +126,7 @@ Status Group::open( encryption_type_from_cfg = config_.get("sm.encryption_type", &found); assert(found); auto [st, et] = encryption_type_enum(encryption_type_from_cfg); - RETURN_NOT_OK(st); + throw_if_not_ok(st); encryption_type = et.value(); if (!EncryptionKey::is_valid_key_length( @@ -138,11 +138,11 @@ Status Group::open( } if (remote_ && encryption_type != EncryptionType::NO_ENCRYPTION) - return Status_GroupError( + throw GroupStatusException( "Cannot open group; encrypted remote groups are not supported."); // Copy the key bytes. - RETURN_NOT_OK( + throw_if_not_ok( encryption_key_->set_key(encryption_type, encryption_key, key_length)); metadata_.clear(); @@ -151,14 +151,14 @@ Status Group::open( if (remote_) { auto rest_client = storage_manager_->rest_client(); if (rest_client == nullptr) { - return Status_GroupError( + throw GroupStatusException( "Cannot open group; remote group with no REST client."); } // Set initial group details to be deserialized into group_details_ = tdb::make_shared(HERE(), group_uri_); - RETURN_NOT_OK(rest_client->post_group_from_rest(group_uri_, this)); + throw_if_not_ok(rest_client->post_group_from_rest(group_uri_, this)); } else if (query_type == QueryType::READ) { group_dir_ = make_shared( HERE(), @@ -187,23 +187,18 @@ Status Group::open( query_type_ = query_type; is_open_ = true; - - return Status::Ok(); } -Status Group::open(QueryType query_type) { - bool found = false; - RETURN_NOT_OK(config_.get( - "sm.group.timestamp_start", ×tamp_start_, &found)); - assert(found); - RETURN_NOT_OK( - config_.get("sm.group.timestamp_end", ×tamp_end_, &found)); - assert(found); +void Group::open(QueryType query_type) { + timestamp_start_ = + config_.get("sm.group.timestamp_start", Config::must_find); + timestamp_end_ = + config_.get("sm.group.timestamp_end", Config::must_find); - return Group::open(query_type, timestamp_start_, timestamp_end_); + Group::open(query_type, timestamp_start_, timestamp_end_); } -Status Group::close_for_writes() { +void Group::close_for_writes() { // Flush the group metadata throw_if_not_ok( unsafe_metadata()->store(resources_, group_uri(), *encryption_key())); @@ -218,13 +213,12 @@ Status Group::close_for_writes() { group_detail_uri, *encryption_key())); } - return Status::Ok(); } -Status Group::close() { +void Group::close() { // Check if group is open if (!is_open_) - return Status::Ok(); + return; if (remote_) { // Update group metadata for write queries if metadata was written by the @@ -237,28 +231,28 @@ Status Group::close() { metadata_loaded_ = true; auto rest_client = storage_manager_->rest_client(); if (rest_client == nullptr) - return Status_GroupError( + throw GroupStatusException( "Error closing group; remote group with no REST client."); - RETURN_NOT_OK( + throw_if_not_ok( rest_client->put_group_metadata_to_rest(group_uri_, this)); } if (!members_to_modify().empty()) { auto rest_client = storage_manager_->rest_client(); if (rest_client == nullptr) - return Status_GroupError( + throw GroupStatusException( "Error closing group; remote group with no REST client."); - RETURN_NOT_OK(rest_client->patch_group_to_rest(group_uri_, this)); + throw_if_not_ok(rest_client->patch_group_to_rest(group_uri_, this)); } } // Storage manager does not own the group schema for remote groups. } else { if (query_type_ == QueryType::READ) { - RETURN_NOT_OK(close_for_reads()); + close_for_reads(); } else if ( query_type_ == QueryType::WRITE || query_type_ == QueryType::MODIFY_EXCLUSIVE) { try { - throw_if_not_ok(close_for_writes()); + close_for_writes(); } catch (StatusException& exc) { std::string msg = exc.what(); msg += " : Was storage for the group moved or deleted before closing?"; @@ -271,7 +265,6 @@ Status Group::close() { metadata_loaded_ = false; is_open_ = false; clear(); - return Status::Ok(); } bool Group::is_open() const { @@ -290,15 +283,13 @@ const shared_ptr Group::group_details() const { return group_details_; } -Status Group::get_query_type(QueryType* query_type) const { +QueryType Group::get_query_type() const { // Error if the group is not open - if (!is_open_) - return LOG_STATUS( - Status_GroupError("Cannot get query_type; Group is not open")); - - *query_type = query_type_; + if (!is_open_) { + throw GroupStatusException("Cannot get query_type; Group is not open"); + } - return Status::Ok(); + return query_type_; } void Group::delete_group(const URI& uri, bool recursive) { @@ -334,7 +325,7 @@ void Group::delete_group(const URI& uri, bool recursive) { Array::delete_array(resources_, member_uri); } else if (member->type() == ObjectType::GROUP) { Group group_rec(resources_, member_uri, storage_manager_); - throw_if_not_ok(group_rec.open(QueryType::MODIFY_EXCLUSIVE)); + group_rec.open(QueryType::MODIFY_EXCLUSIVE); group_rec.delete_group(member_uri, true); } } @@ -366,7 +357,7 @@ void Group::delete_group(const URI& uri, bool recursive) { group_details_->clear(); // Close the deleted group - throw_if_not_ok(this->close()); + this->close(); } void Group::delete_metadata(const char* key) { @@ -551,46 +542,44 @@ void Group::delete_member(const shared_ptr group_member) { group_details_->delete_member(group_member); } -Status Group::mark_member_for_addition( +void Group::mark_member_for_addition( const URI& group_member_uri, const bool& relative, std::optional& name) { std::lock_guard lck(mtx_); // Check if group is open if (!is_open_) { - return Status_GroupError("Cannot add member; Group is not open"); + throw GroupStatusException("Cannot add member; Group is not open"); } // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) { - return Status_GroupError( + throw GroupStatusException( "Cannot get member; Group was not opened in write or modify_exclusive " "mode"); } group_details_->mark_member_for_addition( group_member_uri, relative, name, storage_manager_); - return Status::Ok(); } -Status Group::mark_member_for_removal(const std::string& name) { +void Group::mark_member_for_removal(const std::string& name) { std::lock_guard lck(mtx_); // Check if group is open if (!is_open_) { - return Status_GroupError( + throw GroupStatusException( "Cannot mark member for removal; Group is not open"); } // Check mode if (query_type_ != QueryType::WRITE && query_type_ != QueryType::MODIFY_EXCLUSIVE) { - return Status_GroupError( + throw GroupStatusException( "Cannot get member; Group was not opened in write or modify_exclusive " "mode"); } group_details_->mark_member_for_removal(name); - return Status::Ok(); } const std::vector>& Group::members_to_modify() const { @@ -628,12 +617,12 @@ uint64_t Group::member_count() const { std::lock_guard lck(mtx_); // Check if group is open if (!is_open_) { - throw Status_GroupError("Cannot get member count; Group is not open"); + throw GroupStatusException("Cannot get member count; Group is not open"); } // Check mode if (query_type_ != QueryType::READ) { - throw Status_GroupError( + throw GroupStatusException( "Cannot get member; Group was not opened in read mode"); } @@ -646,12 +635,12 @@ tuple> Group::member_by_index( // Check if group is open if (!is_open_) { - throw Status_GroupError("Cannot get member by index; Group is not open"); + throw GroupStatusException("Cannot get member by index; Group is not open"); } // Check mode if (query_type_ != QueryType::READ) { - throw Status_GroupError( + throw GroupStatusException( "Cannot get member; Group was not opened in read mode"); } @@ -664,12 +653,12 @@ Group::member_by_name(const std::string& name) { // Check if group is open if (!is_open_) { - throw Status_GroupError("Cannot get member by name; Group is not open"); + throw GroupStatusException("Cannot get member by name; Group is not open"); } // Check mode if (query_type_ != QueryType::READ) { - throw Status_GroupError( + throw GroupStatusException( "Cannot get member; Group was not opened in read mode"); } @@ -701,12 +690,12 @@ std::string Group::dump( } Group group_rec(resources_, member_uri, storage_manager_); - auto st = group_rec.open(QueryType::READ); - if (st.ok()) { + try { + group_rec.open(QueryType::READ); ss << std::endl; ss << group_rec.dump(indent_size, num_indents + 2, recursive, false); - throw_if_not_ok(group_rec.close()); - } else { + group_rec.close(); + } catch (GroupNotFoundException&) { // If the group no longer exists in storage it will be listed but we // won't be able to dump its members ss << " (does not exist)" << std::endl; diff --git a/tiledb/sm/group/group.h b/tiledb/sm/group/group.h index 5b1f2598d08..0ba2209a190 100644 --- a/tiledb/sm/group/group.h +++ b/tiledb/sm/group/group.h @@ -35,7 +35,6 @@ #include -#include "tiledb/common/status.h" #include "tiledb/sm/config/config.h" #include "tiledb/sm/crypto/encryption_key.h" #include "tiledb/sm/enums/query_type.h" @@ -89,11 +88,10 @@ class Group { * * @param query_type The query type. This should always be READ. It * is here only for sanity check. - * @return Status * * @note Applicable only to reads. */ - Status open(QueryType query_type); + void open(QueryType query_type); /** * Opens the group. @@ -102,31 +100,25 @@ class Group { * @param timestamp_start The start timestamp at which to open the group * @param timestamp_end The end timestamp at which to open the group * @param query_type The query type. This should always be READ. It - * @return Status * */ - Status open( + void open( QueryType query_type, uint64_t timestamp_start, uint64_t timestamp_end); /** * Closes a group opened for reads. - * - * @return Status */ - inline Status close_for_reads() { + inline void close_for_reads() { // Closing a group opened for reads does nothing at present. - return Status::Ok(); } /** * Closes a group opened for writes. - * - * @return Status */ - Status close_for_writes(); + void close_for_writes(); /** Closes the group and frees all memory. */ - Status close(); + void close(); /** * Clear a group @@ -280,9 +272,8 @@ class Group { * @param group_member_uri group member uri * @param relative is this URI relative * @param name optional name for member - * @return Status */ - Status mark_member_for_addition( + void mark_member_for_addition( const URI& group_member_uri, const bool& relative, std::optional& name); @@ -293,9 +284,8 @@ class Group { * @param name Name of member to remove. If the member has no name, * this parameter should be set to the URI of the member. In that case, only * the unnamed member with the given URI will be removed. - * @return Status */ - Status mark_member_for_removal(const std::string& name); + void mark_member_for_removal(const std::string& name); /** * Get the vector of members to modify, used in serialization only @@ -371,7 +361,7 @@ class Group { bool is_remote() const; /** Retrieves the query type. Errors if the group is not open. */ - Status get_query_type(QueryType* query_type) const; + QueryType get_query_type() const; /** * Dump a string representation of a group diff --git a/tiledb/sm/group/group_directory.cc b/tiledb/sm/group/group_directory.cc index 3e01e5a1d23..202d82b5d6d 100644 --- a/tiledb/sm/group/group_directory.cc +++ b/tiledb/sm/group/group_directory.cc @@ -163,7 +163,7 @@ Status GroupDirectory::load() { } if (!is_group) { - throw GroupDirectoryException("Cannot open group; Group does not exist."); + throw GroupNotFoundException("Cannot open group; Group does not exist."); } // The URI manager has been loaded successfully diff --git a/tiledb/sm/group/group_directory.h b/tiledb/sm/group/group_directory.h index 7dba31ead41..39cc386bc92 100644 --- a/tiledb/sm/group/group_directory.h +++ b/tiledb/sm/group/group_directory.h @@ -44,6 +44,13 @@ using namespace tiledb::common; namespace tiledb::sm { +class GroupNotFoundException : public StatusException { + public: + explicit GroupNotFoundException(const std::string& message) + : StatusException("Group", message) { + } +}; + /** Mode for the GroupDirectory class. */ enum class GroupDirectoryMode { READ, // Read mode. diff --git a/tiledb/sm/group/group_member.h b/tiledb/sm/group/group_member.h index 879a9241f6d..f166df4bfe3 100644 --- a/tiledb/sm/group/group_member.h +++ b/tiledb/sm/group/group_member.h @@ -45,11 +45,6 @@ using namespace tiledb::common; namespace tiledb::sm { -/** Return an Group error class Status with a given message **/ -inline Status Status_GroupError(const std::string& msg) { - return {"[TileDB::Group] Error", msg}; -} - class GroupMember { public: GroupMember( diff --git a/tiledb/sm/serialization/group.cc b/tiledb/sm/serialization/group.cc index bbb2e4e4b88..cd7dcdb235f 100644 --- a/tiledb/sm/serialization/group.cc +++ b/tiledb/sm/serialization/group.cc @@ -282,7 +282,7 @@ Status group_update_from_capnp( if (group_update_details_reader.hasMembersToRemove()) { for (auto uri : group_update_details_reader.getMembersToRemove()) { - throw_if_not_ok(group->mark_member_for_removal(uri.cStr())); + group->mark_member_for_removal(uri.cStr()); } }