From 22a71d477e6535e8b7d2811f0a7e89edd2026155 Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Tue, 27 Feb 2024 09:57:56 +0200 Subject: [PATCH] Move `StorageManager::store_data_to_generic_tile` to `GenericTileIO`. (#4743) Move StorageManager::store_data_to_generic_tile out of StorageManager and into GenericTileIO. --- TYPE: NO_HISTORY DESC: Move StorageManager::store_data_to_generic_tile to GenericTileIO. --- .../deletes_and_updates.cc | 8 +++++-- tiledb/sm/storage_manager/storage_manager.cc | 18 ++++----------- .../storage_manager_canonical.h | 11 --------- tiledb/sm/tile/generic_tile_io.cc | 11 +++++++++ tiledb/sm/tile/generic_tile_io.h | 23 ++++++++++++++----- 5 files changed, 38 insertions(+), 33 deletions(-) diff --git a/tiledb/sm/query/deletes_and_updates/deletes_and_updates.cc b/tiledb/sm/query/deletes_and_updates/deletes_and_updates.cc index f5cdfdc5fbd..1211d58105b 100644 --- a/tiledb/sm/query/deletes_and_updates/deletes_and_updates.cc +++ b/tiledb/sm/query/deletes_and_updates/deletes_and_updates.cc @@ -36,6 +36,7 @@ #include "tiledb/sm/fragment/fragment_identifier.h" #include "tiledb/sm/query/deletes_and_updates/serialization.h" #include "tiledb/sm/storage_manager/storage_manager.h" +#include "tiledb/sm/tile/generic_tile_io.h" #include "tiledb/storage_format/uri/generate_uri.h" using namespace tiledb; @@ -163,8 +164,11 @@ Status DeletesAndUpdates::dowork() { constants::update_file_suffix; auto uri = commit_uri.join_path(new_fragment_str); - RETURN_NOT_OK(storage_manager_->store_data_to_generic_tile( - serialized_condition, uri, *array_->encryption_key())); + GenericTileIO::store_data( + storage_manager_->resources(), + uri, + serialized_condition, + *array_->encryption_key()); return Status::Ok(); } diff --git a/tiledb/sm/storage_manager/storage_manager.cc b/tiledb/sm/storage_manager/storage_manager.cc index eb2d9350d66..4886c2c22b4 100644 --- a/tiledb/sm/storage_manager/storage_manager.cc +++ b/tiledb/sm/storage_manager/storage_manager.cc @@ -1283,8 +1283,7 @@ Status StorageManagerCanonical::store_group_detail( if (!group_detail_dir_exists) RETURN_NOT_OK(vfs()->create_dir(group_detail_folder_uri)); - RETURN_NOT_OK( - store_data_to_generic_tile(tile, group_detail_uri, encryption_key)); + GenericTileIO::store_data(resources_, group_detail_uri, tile, encryption_key); return Status::Ok(); } @@ -1322,7 +1321,7 @@ Status StorageManagerCanonical::store_array_schema( if (!schema_dir_exists) RETURN_NOT_OK(vfs()->create_dir(array_schema_dir_uri)); - RETURN_NOT_OK(store_data_to_generic_tile(tile, schema_uri, encryption_key)); + GenericTileIO::store_data(resources_, schema_uri, tile, encryption_key); // Create the `__enumerations` directory under `__schema` if it doesn't // exist. This might happen if someone tries to add an enumeration to an @@ -1355,8 +1354,7 @@ Status StorageManagerCanonical::store_array_schema( enmr->serialize(serializer); auto abs_enmr_uri = array_enumerations_dir_uri.join_path(enmr->path_name()); - RETURN_NOT_OK( - store_data_to_generic_tile(tile, abs_enmr_uri, encryption_key)); + GenericTileIO::store_data(resources_, abs_enmr_uri, tile, encryption_key); } return Status::Ok(); @@ -1390,19 +1388,11 @@ Status StorageManagerCanonical::store_metadata( // Create a metadata file name URI metadata_uri = metadata->get_uri(uri); - RETURN_NOT_OK(store_data_to_generic_tile(tile, metadata_uri, encryption_key)); + GenericTileIO::store_data(resources_, metadata_uri, tile, encryption_key); return Status::Ok(); } -Status StorageManagerCanonical::store_data_to_generic_tile( - WriterTile& tile, const URI& uri, const EncryptionKey& encryption_key) { - GenericTileIO tile_io(resources_, uri); - uint64_t nbytes = 0; - tile_io.write_generic(&tile, encryption_key, &nbytes); - return vfs()->close_file(uri); -} - void StorageManagerCanonical::wait_for_zero_in_progress() { std::unique_lock lck(queries_in_progress_mtx_); queries_in_progress_cv_.wait( diff --git a/tiledb/sm/storage_manager/storage_manager_canonical.h b/tiledb/sm/storage_manager/storage_manager_canonical.h index 50c37451643..4309f946e32 100644 --- a/tiledb/sm/storage_manager/storage_manager_canonical.h +++ b/tiledb/sm/storage_manager/storage_manager_canonical.h @@ -591,17 +591,6 @@ class StorageManagerCanonical { Status store_metadata( const URI& uri, const EncryptionKey& encryption_key, Metadata* metadata); - /** - * Stores data into persistent storage. - * - * @param tile Tile to store. - * @param uri The object URI. - * @param encryption_key The encryption key to use. - * @return Status - */ - Status store_data_to_generic_tile( - WriterTile& tile, const URI& uri, const EncryptionKey& encryption_key); - [[nodiscard]] inline ContextResources& resources() const { return resources_; } diff --git a/tiledb/sm/tile/generic_tile_io.cc b/tiledb/sm/tile/generic_tile_io.cc index 4292999719b..c2edc076f40 100644 --- a/tiledb/sm/tile/generic_tile_io.cc +++ b/tiledb/sm/tile/generic_tile_io.cc @@ -175,6 +175,17 @@ GenericTileIO::GenericTileHeader GenericTileIO::read_generic_tile_header( return header; } +void GenericTileIO::store_data( + ContextResources& resources, + const URI& uri, + WriterTile& tile, + const EncryptionKey& encryption_key) { + GenericTileIO tile_io(resources, uri); + uint64_t nbytes = 0; + tile_io.write_generic(&tile, encryption_key, &nbytes); + throw_if_not_ok(resources.vfs().close_file(uri)); +} + void GenericTileIO::write_generic( WriterTile* tile, const EncryptionKey& encryption_key, uint64_t* nbytes) { // Create a header diff --git a/tiledb/sm/tile/generic_tile_io.h b/tiledb/sm/tile/generic_tile_io.h index d99f4640bf5..a177f875ab0 100644 --- a/tiledb/sm/tile/generic_tile_io.h +++ b/tiledb/sm/tile/generic_tile_io.h @@ -120,7 +120,7 @@ class GenericTileIO { * @param uri The object URI. * @param offset The offset into the file to read from. * @param encryption_key The encryption key to use. - * @return Status, Tile with the data. + * @return Tile with the data. */ static shared_ptr load( ContextResources& resources, @@ -142,7 +142,7 @@ class GenericTileIO { * @param file_offset The offset in the file to read from. * @param encryption_key The encryption key to use. * @param config The storage manager's config. - * @return Status, Tile + * @return Tile */ shared_ptr read_generic( uint64_t file_offset, @@ -156,13 +156,25 @@ class GenericTileIO { * @param resources The ContextResources instance to use for reading. * @param uri The URI of the generic tile. * @param file_offset The offset where the header read will begin. - * @param encryption_key If the array is encrypted, the private encryption - * key. For unencrypted arrays, pass `nullptr`. - * @return Status, Header + * @return Header */ static GenericTileHeader read_generic_tile_header( ContextResources& resources, const URI& uri, uint64_t file_offset); + /** + * Writes a generic tile to a file. + * + * @param resources The ContextResources instance to use for writing. + * @param uri The URI of the generic tile. + * @param tile The tile to write. + * @param encryption_key The encryption key to use. + */ + static void store_data( + ContextResources& resources, + const URI& uri, + WriterTile& tile, + const EncryptionKey& encryption_key); + /** * Writes a tile generically to the file. This means that a header will be * prepended to the file before writing the tile contents. The reason is @@ -172,7 +184,6 @@ class GenericTileIO { * @param tile The tile to be written. * @param encryption_key The encryption key to use. * @param nbytes The total number of bytes written to the file. - * @return Status */ void write_generic( WriterTile* tile, const EncryptionKey& encryption_key, uint64_t* nbytes);