diff --git a/tiledb/api/c_api/array/array_api_internal.h b/tiledb/api/c_api/array/array_api_internal.h index 8b66a5b3dc0..a747ed4017d 100644 --- a/tiledb/api/c_api/array/array_api_internal.h +++ b/tiledb/api/c_api/array/array_api_internal.h @@ -98,15 +98,6 @@ struct tiledb_array_handle_t array_->delete_array(uri); } - void delete_fragments( - tiledb::sm::ContextResources& resources, - const tiledb::sm::URI& uri, - uint64_t ts_start, - uint64_t ts_end, - std::optional array_dir = std::nullopt) { - array_->delete_fragments(resources, uri, ts_start, ts_end, array_dir); - } - void delete_fragments( const tiledb::sm::URI& uri, uint64_t timestamp_start, diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc index 2c35059ffa1..acc5c5f2ce8 100644 --- a/tiledb/sm/array/array.cc +++ b/tiledb/sm/array/array.cc @@ -658,42 +658,33 @@ Status Array::close() { } void Array::delete_fragments( - ContextResources& resources, - const URI& uri, - uint64_t timestamp_start, - uint64_t timestamp_end, - std::optional array_dir) { - // Get the fragment URIs to be deleted - if (array_dir == std::nullopt) { - array_dir = ArrayDirectory(resources, uri, timestamp_start, timestamp_end); - } - auto filtered_fragment_uris = array_dir->filtered_fragment_uris(true); + ContextResources& resources, ArrayDirectory& array_dir) { + auto filtered_fragment_uris = array_dir.filtered_fragment_uris(true); const auto& fragment_uris = filtered_fragment_uris.fragment_uris(); // Retrieve commit uris to delete and ignore std::vector commit_uris_to_delete; std::vector commit_uris_to_ignore; for (auto& fragment : fragment_uris) { - auto commit_uri = array_dir->get_commit_uri(fragment.uri_); + auto commit_uri = array_dir.get_commit_uri(fragment.uri_); commit_uris_to_delete.emplace_back(commit_uri); - if (array_dir->consolidated_commit_uris_set().count(commit_uri.c_str()) != - 0) { + if (array_dir.consolidated_commit_uris_set().count(commit_uri) != 0) { commit_uris_to_ignore.emplace_back(commit_uri); } } // Write ignore file if (commit_uris_to_ignore.size() != 0) { - array_dir->write_commit_ignore_file(commit_uris_to_ignore); + array_dir.write_commit_ignore_file(commit_uris_to_ignore); } // Delete fragments and commits - auto vfs = &(resources.vfs()); + auto& vfs = resources.vfs(); throw_if_not_ok(parallel_for( &resources.compute_tp(), 0, fragment_uris.size(), [&](size_t i) { - vfs->remove_dir(fragment_uris[i].uri_); - if (vfs->is_file(commit_uris_to_delete[i])) { - vfs->remove_file(commit_uris_to_delete[i]); + vfs.remove_dir(fragment_uris[i].uri_); + if (vfs.is_file(commit_uris_to_delete[i])) { + vfs.remove_file(commit_uris_to_delete[i]); } return Status::Ok(); })); @@ -714,7 +705,9 @@ void Array::delete_fragments( rest_client->post_delete_fragments_to_rest( uri, this, timestamp_start, timestamp_end); } else { - Array::delete_fragments(resources_, uri, timestamp_start, timestamp_end); + auto array_dir = + ArrayDirectory(resources_, uri, timestamp_start, timestamp_end); + Array::delete_fragments(resources_, array_dir); } } @@ -724,8 +717,7 @@ void Array::delete_array(ContextResources& resources, const URI& uri) { ArrayDirectory(resources, uri, 0, std::numeric_limits::max()); // Delete fragments and commits - Array::delete_fragments( - resources, uri, 0, std::numeric_limits::max(), array_dir); + Array::delete_fragments(resources, array_dir); // Delete array metadata, fragment metadata and array schema files // Note: metadata files may not be present, try to delete anyway diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h index 71e9361cb55..e7ec800f288 100644 --- a/tiledb/sm/array/array.h +++ b/tiledb/sm/array/array.h @@ -100,7 +100,7 @@ class OpenedArray { uint64_t timestamp_end_opened_at, bool is_remote) : resources_(resources) - , array_dir_(ArrayDirectory(resources, array_uri)) + , array_dir_(std::in_place, resources, array_uri) , array_schema_latest_(nullptr) , metadata_(memory_tracker) , metadata_loaded_(false) @@ -124,8 +124,8 @@ class OpenedArray { } /** Sets the array directory. */ - inline void set_array_directory(const ArrayDirectory&& dir) { - array_dir_ = dir; + inline void set_array_directory(ArrayDirectory&& dir) { + array_dir_.emplace(std::move(dir)); } /** Returns the latest array schema. */ @@ -320,7 +320,7 @@ class Array { } /** Set the array directory. */ - inline void set_array_directory(const ArrayDirectory&& dir) { + inline void set_array_directory(ArrayDirectory&& dir) { opened_array_->set_array_directory(std::move(dir)); } @@ -463,23 +463,10 @@ class Array { * between the provided timestamps. * * @param resources The context resources. - * @param uri The uri of the Array whose fragments are to be deleted. - * @param timestamp_start The start timestamp at which to delete fragments. - * @param timestamp_end The end timestamp at which to delete fragments. * @param array_dir An optional ArrayDirectory from which to delete fragments. - * - * @section Maturity Notes - * This is legacy code, ported from StorageManager during its removal process. - * Its existence supports the non-static `delete_fragments` API below, - * performing the actual deletion of fragments. This function is slated for - * removal and should be directly integrated into the function below. */ static void delete_fragments( - ContextResources& resources, - const URI& uri, - uint64_t timestamp_start, - uint64_t timstamp_end, - std::optional array_dir = std::nullopt); + ContextResources& resources, ArrayDirectory& array_dir); /** * Handles local and remote deletion of fragments between the provided diff --git a/tiledb/sm/array/array_directory.cc b/tiledb/sm/array/array_directory.cc index 2dd9156447d..6d515f0fb12 100644 --- a/tiledb/sm/array/array_directory.cc +++ b/tiledb/sm/array/array_directory.cc @@ -253,8 +253,8 @@ const std::vector& ArrayDirectory::commit_uris_to_vacuum() const { return commit_uris_to_vacuum_; } -const std::unordered_set& -ArrayDirectory::consolidated_commit_uris_set() const { +const std::unordered_set& ArrayDirectory::consolidated_commit_uris_set() + const { return consolidated_commit_uris_set_; } @@ -309,7 +309,7 @@ void ArrayDirectory::delete_fragments_list( for (auto& timestamped_uri : uris) { auto commit_uri = get_commit_uri(timestamped_uri); commit_uris_to_delete.emplace_back(commit_uri); - if (consolidated_commit_uris_set().count(commit_uri.c_str()) != 0) { + if (consolidated_commit_uris_set().count(commit_uri) != 0) { commit_uris_to_ignore.emplace_back(commit_uri); } } @@ -483,7 +483,7 @@ ArrayDirectory::filtered_fragment_uris(const bool full_overlap_only) const { if (mode_ == ArrayDirectoryMode::VACUUM_FRAGMENTS) { for (auto& uri : fragment_uris_to_vacuum.value()) { auto commit_uri = get_commit_uri(uri); - if (consolidated_commit_uris_set_.count(commit_uri.c_str()) == 0) { + if (consolidated_commit_uris_set_.count(commit_uri) == 0) { commit_uris_to_vacuum.emplace_back(commit_uri); } else { commit_uris_to_ignore.emplace_back(commit_uri); @@ -672,8 +672,7 @@ ArrayDirectory::load_commits_dir_uris_v12_or_higher( for (size_t i = 0; i < commits_dir_uris.size(); ++i) { if (stdx::string::ends_with( commits_dir_uris[i].to_string(), constants::write_file_suffix)) { - if (consolidated_commit_uris_set_.count(commits_dir_uris[i].c_str()) == - 0) { + if (consolidated_commit_uris_set_.count(commits_dir_uris[i]) == 0) { auto name = commits_dir_uris[i].last_path_part(); name = name.substr(0, name.size() - constants::write_file_suffix.size()); @@ -694,8 +693,7 @@ ArrayDirectory::load_commits_dir_uris_v12_or_higher( // Add the delete tile location if it overlaps the open start/end times if (timestamps_overlap(timestamp_range, false)) { - if (consolidated_commit_uris_set_.count(commits_dir_uris[i].c_str()) == - 0) { + if (consolidated_commit_uris_set_.count(commits_dir_uris[i]) == 0) { const auto base_uri_size = uri_.to_string().size(); delete_and_update_tiles_location_.emplace_back( commits_dir_uris[i], @@ -716,10 +714,7 @@ ArrayDirectory::list_fragment_metadata_dir_uris_v12_or_higher() { return ls(uri_.join_path(constants::array_fragment_meta_dir_name)); } -tuple< - Status, - optional>, - optional>> +tuple>, optional>> ArrayDirectory::load_consolidated_commit_uris( const std::vector& commits_dir_uris) { auto timer_se = stats_->start_timer("load_consolidated_commit_uris"); @@ -745,7 +740,7 @@ ArrayDirectory::load_consolidated_commit_uris( // Load all commit URIs. This is done in serial for now as it can be optimized // by vacuuming. - std::unordered_set uris_set; + std::unordered_set uris_set; std::vector> meta_files; for (uint64_t i = 0; i < commits_dir_uris.size(); i++) { auto& uri = commits_dir_uris[i]; @@ -763,7 +758,7 @@ ArrayDirectory::load_consolidated_commit_uris( std::stringstream ss(names); for (std::string condition_marker; std::getline(ss, condition_marker);) { if (ignore_set.count(condition_marker) == 0) { - uris_set.emplace(uri_.to_string() + condition_marker); + uris_set.emplace(uri_.append_string(condition_marker)); } // If we have a delete, process the condition tile @@ -807,7 +802,7 @@ ArrayDirectory::load_consolidated_commit_uris( uint64_t count = 0; bool all_in_set = true; for (std::string uri_str; std::getline(ss, uri_str);) { - if (uris_set.count(uri_.to_string() + uri_str) > 0) { + if (uris_set.count(uri_.append_string(uri_str)) > 0) { count++; } else { all_in_set = false; @@ -879,7 +874,7 @@ void ArrayDirectory::load_commits_uris_to_consolidate( const std::vector& array_dir_uris, const std::vector& commits_dir_uris, const std::vector& consolidated_uris, - const std::unordered_set& consolidated_uris_set) { + const std::unordered_set& consolidated_uris_set) { // Make a set of existing commit URIs. std::unordered_set uris_set; for (auto& uri : array_dir_uris) { @@ -902,7 +897,7 @@ void ArrayDirectory::load_commits_uris_to_consolidate( // Add the ok file URIs not already in the list. for (auto& uri : array_dir_uris) { if (stdx::string::ends_with(uri.to_string(), constants::ok_file_suffix)) { - if (consolidated_uris_set.count(uri.c_str()) == 0) { + if (consolidated_uris_set.count(uri) == 0) { commit_uris_to_consolidate_.emplace_back(uri); } } @@ -914,7 +909,7 @@ void ArrayDirectory::load_commits_uris_to_consolidate( uri.to_string(), constants::write_file_suffix) || stdx::string::ends_with( uri.to_string(), constants::delete_file_suffix)) { - if (consolidated_uris_set.count(uri.c_str()) == 0) { + if (consolidated_uris_set.count(uri) == 0) { commit_uris_to_consolidate_.emplace_back(uri); } } @@ -1258,7 +1253,7 @@ bool ArrayDirectory::is_vacuum_file(const URI& uri) const { Status ArrayDirectory::is_fragment( const URI& uri, const std::unordered_set& ok_uris_set, - const std::unordered_set& consolidated_uris_set, + const std::unordered_set& consolidated_uris_set, int* is_fragment) const { // If the fragment ID does not have a name, ignore it. if (!FragmentID::has_fragment_name(uri)) { @@ -1292,7 +1287,7 @@ Status ArrayDirectory::is_fragment( // Check set membership in consolidated uris if (consolidated_uris_set.count( - uri.to_string() + constants::ok_file_suffix) != 0) { + uri.append_string(constants::ok_file_suffix)) != 0) { *is_fragment = 1; return Status::Ok(); } diff --git a/tiledb/sm/array/array_directory.h b/tiledb/sm/array/array_directory.h index ad48b62cf2a..565166cd606 100644 --- a/tiledb/sm/array/array_directory.h +++ b/tiledb/sm/array/array_directory.h @@ -311,6 +311,10 @@ class ArrayDirectory { uint64_t timestamp_end, ArrayDirectoryMode mode = ArrayDirectoryMode::READ); + DISABLE_COPY_AND_COPY_ASSIGN(ArrayDirectory); + + ArrayDirectory(ArrayDirectory&&) = default; + /** Destructor. */ ~ArrayDirectory() = default; @@ -436,7 +440,7 @@ class ArrayDirectory { const std::vector& commit_uris_to_vacuum() const; /** Returns the consolidated commit URI set. */ - const std::unordered_set& consolidated_commit_uris_set() const; + const std::unordered_set& consolidated_commit_uris_set() const; /** Returns the URIs of the consolidated commit files to vacuum. */ const std::vector& consolidated_commits_uris_to_vacuum() const; @@ -511,7 +515,7 @@ class ArrayDirectory { } /** Accessor to consolidated_commit_uris_set_ */ - inline std::unordered_set& consolidated_commit_uris_set() { + inline std::unordered_set& consolidated_commit_uris_set() { return consolidated_commit_uris_set_; } @@ -589,7 +593,7 @@ class ArrayDirectory { std::vector unfiltered_fragment_uris_; /** Consolidated commit URI set. */ - std::unordered_set consolidated_commit_uris_set_; + std::unordered_set consolidated_commit_uris_set_; /** The URIs of all the array schema files. */ std::vector array_schema_uris_; @@ -710,7 +714,7 @@ class ArrayDirectory { const std::vector& array_dir_uris, const std::vector& commits_dir_uris, const std::vector& consolidated_uris, - const std::unordered_set& consolidated_uris_set); + const std::unordered_set& consolidated_uris_set); /** * Loads the consolidated commit URI from the commit directory and the files @@ -718,10 +722,7 @@ class ArrayDirectory { * * @return Status, consolidated uris, set of all consolidated uris. */ - tuple< - Status, - optional>, - optional>> + tuple>, optional>> load_consolidated_commit_uris(const std::vector& commits_dir_uris); /** Loads the array metadata URIs. */ @@ -821,7 +822,7 @@ class ArrayDirectory { Status is_fragment( const URI& uri, const std::unordered_set& ok_uris_set, - const std::unordered_set& consolidated_uris_set, + const std::unordered_set& consolidated_uris_set, int32_t* is_fragment) const; /** diff --git a/tiledb/sm/consolidator/consolidator.cc b/tiledb/sm/consolidator/consolidator.cc index 1b713b9cec3..1cd376a3356 100644 --- a/tiledb/sm/consolidator/consolidator.cc +++ b/tiledb/sm/consolidator/consolidator.cc @@ -252,7 +252,7 @@ void Consolidator::fragments_consolidate( void Consolidator::write_consolidated_commits_file( format_version_t write_version, - ArrayDirectory array_dir, + const ArrayDirectory& array_dir, const std::vector& commit_uris, ContextResources& resources) { // Compute the file name. diff --git a/tiledb/sm/consolidator/consolidator.h b/tiledb/sm/consolidator/consolidator.h index dcbdc37ad2a..af82fec8124 100644 --- a/tiledb/sm/consolidator/consolidator.h +++ b/tiledb/sm/consolidator/consolidator.h @@ -215,7 +215,7 @@ class Consolidator { */ static void write_consolidated_commits_file( format_version_t write_version, - ArrayDirectory array_dir, + const ArrayDirectory& array_dir, const std::vector& commit_uris, ContextResources& resources); diff --git a/tiledb/sm/filesystem/uri.cc b/tiledb/sm/filesystem/uri.cc index 13a4a20f3d0..c69299fbb00 100644 --- a/tiledb/sm/filesystem/uri.cc +++ b/tiledb/sm/filesystem/uri.cc @@ -50,32 +50,13 @@ namespace tiledb::sm { /* CONSTRUCTORS & DESTRUCTORS */ /* ********************************* */ -URI::URI() { - uri_ = ""; -} - -URI::URI(char* path) - : URI((path == nullptr) ? std::string("") : std::string(path)) { -} +URI::URI() = default; URI::URI(const char* path) - : URI((path == nullptr) ? std::string("") : std::string(path)) { + : URI(path == nullptr ? std::string_view() : std::string_view(path)) { } -URI::URI(std::string_view path) { - if (path.empty()) - uri_ = ""; - else if (URI::is_file(path)) - uri_ = VFS::abs_path(path); - else if ( - URI::is_s3(path) || URI::is_azure(path) || URI::is_gcs(path) || - URI::is_memfs(path) || URI::is_tiledb(path)) - uri_ = path; - else - uri_ = ""; -} - -URI::URI(std::string_view path, const bool& get_abs) { +URI::URI(std::string_view path, bool get_abs) { if (path.empty()) { uri_ = ""; } else if (URI::is_file(path)) { @@ -111,9 +92,9 @@ URI URI::add_trailing_slash() const { if (uri_.empty()) { return URI("/"); } else if (uri_.back() != '/') { - return URI(uri_ + '/'); + return URI(CreateRaw, uri_ + '/'); } else { - return URI(uri_); + return *this; } } @@ -121,10 +102,10 @@ URI URI::remove_trailing_slash() const { if (!uri_.empty() && uri_.back() == '/') { std::string uri_str = uri_; uri_str.pop_back(); - return URI(uri_str); + return URI(CreateRaw, std::move(uri_str)); } - return URI(uri_); + return *this; } bool URI::empty() const { @@ -343,30 +324,30 @@ std::optional URI::get_fragment_name() const { } if (slash_pos != std::string::npos) { - return URI(uri_.substr(0, slash_pos)); + return URI(CreateRaw, uri_.substr(0, slash_pos)); } - return URI(uri_); + return *this; } URI URI::join_path(const std::string& path) const { // Check for empty strings. if (path.empty()) { - return URI(uri_); + return *this; } else if (uri_.empty()) { return URI(path); } if (uri_.back() == '/') { if (path.front() == '/') { - return URI(uri_ + path.substr(1, path.size())); + return URI(CreateRaw, uri_ + path.substr(1, path.size())); } - return URI(uri_ + path); + return URI(CreateRaw, uri_ + path); } else { if (path.front() == '/') { - return URI(uri_ + path); + return URI(CreateRaw, uri_ + path); } else { - return URI(uri_ + "/" + path); + return URI(CreateRaw, uri_ + "/" + path); } } } @@ -375,6 +356,10 @@ URI URI::join_path(const URI& uri) const { return join_path(uri.to_string()); } +URI URI::append_string(const std::string& str) const { + return URI(CreateRaw, uri_ + str); +} + std::string URI::last_path_part() const { return uri_.substr(uri_.find_last_of('/') + 1); } @@ -385,7 +370,7 @@ std::string URI::last_two_path_parts() const { URI URI::parent_path() const { auto pos = this->remove_trailing_slash().to_string().find_last_of('/'); - return URI(uri_.substr(0, pos + 1)); + return URI(CreateRaw, uri_.substr(0, pos + 1)); } std::string URI::to_path(const std::string& uri) { @@ -432,16 +417,14 @@ bool URI::operator==(const URI& uri) const { return uri_ == uri.uri_; } -bool URI::operator!=(const URI& uri) const { - return !operator==(uri); +std::strong_ordering URI::operator<=>(const URI& uri) const { + return uri_ <=> uri.uri_; } -bool URI::operator<(const URI& uri) const { - return uri_ < uri.uri_; -} +} // namespace tiledb::sm -bool URI::operator>(const URI& uri) const { - return uri_ > uri.uri_; +namespace std { +size_t hash::operator()(const tiledb::sm::URI& val) const { + return hash()(val.to_string()); } - -} // namespace tiledb::sm +} // namespace std diff --git a/tiledb/sm/filesystem/uri.h b/tiledb/sm/filesystem/uri.h index 89356bc65ee..97a1aa3c871 100644 --- a/tiledb/sm/filesystem/uri.h +++ b/tiledb/sm/filesystem/uri.h @@ -33,6 +33,7 @@ #ifndef TILEDB_URI_H #define TILEDB_URI_H +#include #include #include "tiledb/common/status.h" @@ -85,26 +86,10 @@ class URI { /** * Constructor. * - * @param path String that gets converted into an absolute path and stored - * as a URI. - */ - explicit URI(char* path); - - /** - * Constructor. - * - * @param path String that gets converted into an absolute path and stored - * as a URI. + * @param path The URI's path. + * @param get_abs Whether to convert path to absolute. */ - explicit URI(std::string_view path); - - /** - * Constructor. - * - * @param path - * @param get_abs should local files become absolute - */ - explicit URI(std::string_view path, const bool& get_abs); + explicit URI(std::string_view path, bool get_abs = true); /** * Constructor. Throws if the given path is invalid (nullptr or empty). @@ -291,6 +276,14 @@ class URI { */ URI join_path(const URI& uri) const; + /** + * Appends a string to the input URI. + * + * @param str The string to append. + * @return The resulting URI. + */ + URI append_string(const std::string& str) const; + /** Returns the last part of the URI (i.e., excluding the parent). */ std::string last_path_part() const; @@ -331,17 +324,25 @@ class URI { bool operator==(const URI& uri) const; /** For comparing URIs alphanumerically. */ - bool operator!=(const URI& uri) const; - - /** For comparing URIs alphanumerically. */ - bool operator<(const URI& uri) const; - - /** For comparing URIs alphanumerically. */ - bool operator>(const URI& uri) const; + std::strong_ordering operator<=>(const URI& uri) const; operator std::string_view() const noexcept; private: + struct CreateRawMarker {}; + + static constexpr CreateRawMarker CreateRaw{}; + + /** + * Creates a URI from a string, without making any modifications. + * + * @param create_raw CreateRawMarker dummy value. + * @param uri Rvalue reference to string. + */ + URI(const CreateRawMarker, std::string&& uri) + : uri_(std::move(uri)) { + } + /* ********************************* */ /* PRIVATE ATTRIBUTES */ /* ********************************* */ @@ -386,15 +387,17 @@ struct TimestampedURI { } }; +} // namespace tiledb::sm + +namespace std { +template <> /** - * URI hash operator. + * Specialization of std::hash for URI. This lets it being used as a key for + * hash tables. */ -struct URIHasher { - std::size_t operator()(const URI& uri) const { - return std::hash()(uri.to_string()); - } +struct hash { + size_t operator()(const tiledb::sm::URI& val) const; }; - -} // namespace tiledb::sm +} // namespace std #endif // TILEDB_URI_H diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc index 9a4ef23182e..158d54ff792 100644 --- a/tiledb/sm/serialization/array.cc +++ b/tiledb/sm/serialization/array.cc @@ -172,7 +172,7 @@ Status array_to_capnp( if (array->use_refactored_query_submit()) { // Serialize array directory (load if not loaded already) - const auto array_directory = array->load_array_directory(); + auto& array_directory = array->load_array_directory(); auto array_directory_builder = array_builder->initArrayDirectory(); array_directory_to_capnp(array_directory, &array_directory_builder);