Skip to content

Commit

Permalink
Migrate APIs out of StorageManager: object_type, object_move, object_…
Browse files Browse the repository at this point in the history
…remove. (#5017)

Migrate APIs out of `StorageManager`: `object_type`, `object_move`,
`object_remove`.
Also remove some extraneous inclusions from `StorageManager`.

[sc-46737]
[sc-46732]
[sc-46731]

---
TYPE: IMPROVEMENT
DESC: Migrate APIs out of `StorageManager`: `object_type`,
`object_move`, `object_remove`.
  • Loading branch information
bekadavis9 authored May 29, 2024
1 parent f2d32e7 commit afc03ad
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 85 deletions.
3 changes: 2 additions & 1 deletion tiledb/sm/c_api/tiledb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
#include "tiledb/sm/enums/serialization_type.h"
#include "tiledb/sm/filter/filter_pipeline.h"
#include "tiledb/sm/misc/tdb_time.h"
#include "tiledb/sm/object/object.h"
#include "tiledb/sm/query/query.h"
#include "tiledb/sm/query/query_condition.h"
#include "tiledb/sm/rest/rest_client.h"
Expand Down Expand Up @@ -3075,7 +3076,7 @@ int32_t tiledb_object_type(
tiledb_ctx_t* ctx, const char* path, tiledb_object_t* type) {
auto uri = tiledb::sm::URI(path);
tiledb::sm::ObjectType object_type;
throw_if_not_ok(ctx->storage_manager()->object_type(uri, &object_type));
throw_if_not_ok(tiledb::sm::object_type(ctx->resources(), uri, &object_type));

*type = static_cast<tiledb_object_t>(object_type);
return TILEDB_OK;
Expand Down
7 changes: 5 additions & 2 deletions tiledb/sm/consolidator/consolidator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "tiledb/sm/consolidator/fragment_meta_consolidator.h"
#include "tiledb/sm/consolidator/group_meta_consolidator.h"
#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/storage_format/uri/generate_uri.h"
Expand Down Expand Up @@ -147,7 +148,8 @@ void Consolidator::array_consolidate(

// Check if array exists
ObjectType obj_type;
throw_if_not_ok(storage_manager->object_type(array_uri, &obj_type));
throw_if_not_ok(
object_type(storage_manager->resources(), array_uri, &obj_type));

if (obj_type != ObjectType::ARRAY) {
throw ConsolidatorException(
Expand Down Expand Up @@ -210,7 +212,8 @@ void Consolidator::fragments_consolidate(

// Check if array exists
ObjectType obj_type;
throw_if_not_ok(storage_manager->object_type(array_uri, &obj_type));
throw_if_not_ok(
object_type(storage_manager->resources(), array_uri, &obj_type));

if (obj_type != ObjectType::ARRAY) {
throw ConsolidatorException(
Expand Down
2 changes: 1 addition & 1 deletion tiledb/sm/group/group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ void Group::mark_member_for_addition(
"mode");
}
group_details_->mark_member_for_addition(
group_member_uri, relative, name, storage_manager_);
resources_, group_member_uri, relative, name);
}

void Group::mark_member_for_removal(const std::string& name) {
Expand Down
8 changes: 4 additions & 4 deletions tiledb/sm/group/group_details.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "tiledb/sm/group/group_member_v2.h"
#include "tiledb/sm/metadata/metadata.h"
#include "tiledb/sm/misc/tdb_time.h"
#include "tiledb/sm/object/object.h"
#include "tiledb/sm/rest/rest_client.h"
#include "tiledb/sm/tile/generic_tile_io.h"

Expand Down Expand Up @@ -81,19 +82,18 @@ void GroupDetails::delete_member(const shared_ptr<GroupMember> group_member) {
}

void GroupDetails::mark_member_for_addition(
ContextResources& resources,
const URI& group_member_uri,
const bool& relative,
std::optional<std::string>& name,
StorageManager* storage_manager) {
std::optional<std::string>& name) {
std::lock_guard<std::mutex> lck(mtx_);
URI absolute_group_member_uri = group_member_uri;
if (relative) {
absolute_group_member_uri =
group_uri_.join_path(group_member_uri.to_string());
}
ObjectType type = ObjectType::INVALID;
throw_if_not_ok(
storage_manager->object_type(absolute_group_member_uri, &type));
throw_if_not_ok(object_type(resources, absolute_group_member_uri, &type));
if (type == ObjectType::INVALID) {
throw GroupDetailsException(
"Cannot add group member " + absolute_group_member_uri.to_string() +
Expand Down
5 changes: 3 additions & 2 deletions tiledb/sm/group/group_details.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,16 @@ class GroupDetails {
/**
* Add a member to a group, this will be flushed to disk on close
*
* @param resources the context resources
* @param group_member_uri group member uri
* @param relative is this URI relative
* @param name optional name for member
*/
void mark_member_for_addition(
ContextResources& resources,
const URI& group_member_uri,
const bool& relative,
std::optional<std::string>& name,
StorageManager* storage_manager);
std::optional<std::string>& name);

/**
* Remove a member from a group, this will be flushed to disk on close
Expand Down
38 changes: 38 additions & 0 deletions tiledb/sm/object/object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@
*/

#include "tiledb/sm/object/object.h"
#include "tiledb/common/stdx_string.h"
#include "tiledb/sm/enums/object_type.h"
#include "tiledb/sm/filesystem/vfs.h"
#include "tiledb/sm/rest/rest_client.h"
#include "tiledb/storage_format/uri/parse_uri.h"

namespace tiledb::sm {

Expand Down Expand Up @@ -91,4 +94,39 @@ Status is_group(ContextResources& resources, const URI& uri, bool* is_group) {
return Status::Ok();
}

Status object_type(
ContextResources& resources, const URI& uri, ObjectType* type) {
URI dir_uri = uri;
if (uri.is_s3() || uri.is_azure() || uri.is_gcs()) {
// Always add a trailing '/' in the S3/Azure/GCS case so that listing the
// URI as a directory will work as expected. Listing a non-directory object
// is not an error for S3/Azure/GCS.
auto uri_str = uri.to_string();
dir_uri =
URI(utils::parse::ends_with(uri_str, "/") ? uri_str : (uri_str + "/"));
} else if (!uri.is_tiledb()) {
// For non public cloud backends, listing a non-directory is an error.
bool is_dir = false;
throw_if_not_ok(resources.vfs().is_dir(uri, &is_dir));
if (!is_dir) {
*type = ObjectType::INVALID;
return Status::Ok();
}
}
bool exists = is_array(resources, uri);
if (exists) {
*type = ObjectType::ARRAY;
return Status::Ok();
}

throw_if_not_ok(is_group(resources, uri, &exists));
if (exists) {
*type = ObjectType::GROUP;
return Status::Ok();
}

*type = ObjectType::INVALID;
return Status::Ok();
}

} // namespace tiledb::sm
13 changes: 13 additions & 0 deletions tiledb/sm/object/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ using namespace tiledb::common;

namespace tiledb::sm {

enum class ObjectType : uint8_t;

/* ********************************* */
/* API */
/* ********************************* */
Expand All @@ -65,6 +67,17 @@ bool is_array(ContextResources& resources, const URI& uri);
*/
Status is_group(ContextResources& resources, const URI& uri, bool* is_group);

/**
* Returns the tiledb object type
*
* @param resources the context resources.
* @param uri Path to TileDB object resource
* @param type The ObjectType to be retrieved.
* @return Status
*/
Status object_type(
ContextResources& resources, const URI& uri, ObjectType* type);

} // namespace tiledb::sm

#endif
69 changes: 13 additions & 56 deletions tiledb/sm/storage_manager/storage_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,38 +41,28 @@
#include "tiledb/common/heap_memory.h"
#include "tiledb/common/logger.h"
#include "tiledb/common/memory.h"
#include "tiledb/common/memory_tracker.h"
#include "tiledb/common/stdx_string.h"
#include "tiledb/sm/array/array.h"
#include "tiledb/sm/array/array_directory.h"
#include "tiledb/sm/array_schema/array_schema.h"
#include "tiledb/sm/array_schema/array_schema_evolution.h"
#include "tiledb/sm/array_schema/enumeration.h"
#include "tiledb/sm/consolidator/consolidator.h"
#include "tiledb/sm/consolidator/fragment_consolidator.h"
#include "tiledb/sm/enums/array_type.h"
#include "tiledb/sm/enums/layout.h"
#include "tiledb/sm/enums/object_type.h"
#include "tiledb/sm/enums/query_type.h"
#include "tiledb/sm/filesystem/vfs.h"
#include "tiledb/sm/fragment/fragment_info.h"
#include "tiledb/sm/global_state/global_state.h"
#include "tiledb/sm/global_state/unit_test_config.h"
#include "tiledb/sm/group/group.h"
#include "tiledb/sm/group/group_details_v1.h"
#include "tiledb/sm/group/group_details_v2.h"
#include "tiledb/sm/misc/parallel_functions.h"
#include "tiledb/sm/misc/tdb_time.h"
#include "tiledb/sm/misc/utils.h"
#include "tiledb/sm/object/object.h"
#include "tiledb/sm/query/query.h"
#include "tiledb/sm/rest/rest_client.h"
#include "tiledb/sm/stats/global_stats.h"
#include "tiledb/sm/storage_manager/storage_manager.h"
#include "tiledb/sm/tile/generic_tile_io.h"
#include "tiledb/sm/tile/tile.h"
#include "tiledb/storage_format/uri/generate_uri.h"
#include "tiledb/storage_format/uri/parse_uri.h"

#include <algorithm>
#include <iostream>
Expand Down Expand Up @@ -441,7 +431,7 @@ Status StorageManagerCanonical::object_remove(const char* path) const {
std::string("Cannot remove object '") + path + "'; Invalid URI"));

ObjectType obj_type;
RETURN_NOT_OK(object_type(uri, &obj_type));
throw_if_not_ok(object_type(resources_, uri, &obj_type));
if (obj_type == ObjectType::INVALID)
return logger_->status(Status_StorageManagerError(
std::string("Cannot remove object '") + path +
Expand All @@ -463,7 +453,7 @@ Status StorageManagerCanonical::object_move(
std::string("Cannot move object to '") + new_path + "'; Invalid URI"));

ObjectType obj_type;
RETURN_NOT_OK(object_type(old_uri, &obj_type));
throw_if_not_ok(object_type(resources_, old_uri, &obj_type));
if (obj_type == ObjectType::INVALID)
return logger_->status(Status_StorageManagerError(
std::string("Cannot move object '") + old_path +
Expand Down Expand Up @@ -524,41 +514,6 @@ void StorageManagerCanonical::increment_in_progress() {
queries_in_progress_cv_.notify_all();
}

Status StorageManagerCanonical::object_type(
const URI& uri, ObjectType* type) const {
URI dir_uri = uri;
if (uri.is_s3() || uri.is_azure() || uri.is_gcs()) {
// Always add a trailing '/' in the S3/Azure/GCS case so that listing the
// URI as a directory will work as expected. Listing a non-directory object
// is not an error for S3/Azure/GCS.
auto uri_str = uri.to_string();
dir_uri =
URI(utils::parse::ends_with(uri_str, "/") ? uri_str : (uri_str + "/"));
} else if (!uri.is_tiledb()) {
// For non public cloud backends, listing a non-directory is an error.
bool is_dir = false;
throw_if_not_ok(resources_.vfs().is_dir(uri, &is_dir));
if (!is_dir) {
*type = ObjectType::INVALID;
return Status::Ok();
}
}
bool exists = is_array(resources_, uri);
if (exists) {
*type = ObjectType::ARRAY;
return Status::Ok();
}

RETURN_NOT_OK(is_group(resources_, uri, &exists));
if (exists) {
*type = ObjectType::GROUP;
return Status::Ok();
}

*type = ObjectType::INVALID;
return Status::Ok();
}

Status StorageManagerCanonical::object_iter_begin(
ObjectIter** obj_iter, const char* path, WalkOrder order) {
// Sanity check
Expand All @@ -580,7 +535,8 @@ Status StorageManagerCanonical::object_iter_begin(
// Include the uris that are TileDB objects in the iterator state
ObjectType obj_type;
for (auto& uri : uris) {
RETURN_NOT_OK_ELSE(object_type(uri, &obj_type), tdb_delete(*obj_iter));
RETURN_NOT_OK_ELSE(
object_type(resources_, uri, &obj_type), tdb_delete(*obj_iter));
if (obj_type != ObjectType::INVALID) {
(*obj_iter)->objs_.push_back(uri);
if (order == WalkOrder::POSTORDER)
Expand Down Expand Up @@ -612,9 +568,10 @@ Status StorageManagerCanonical::object_iter_begin(
// Include the uris that are TileDB objects in the iterator state
ObjectType obj_type;
for (auto& uri : uris) {
RETURN_NOT_OK(object_type(uri, &obj_type));
if (obj_type != ObjectType::INVALID)
throw_if_not_ok(object_type(resources_, uri, &obj_type));
if (obj_type != ObjectType::INVALID) {
(*obj_iter)->objs_.push_back(uri);
}
}

return Status::Ok();
Expand Down Expand Up @@ -660,7 +617,7 @@ Status StorageManagerCanonical::object_iter_next_postorder(
// Push the new TileDB objects in the front of the iterator's list
ObjectType obj_type;
for (auto it = uris.rbegin(); it != uris.rend(); ++it) {
RETURN_NOT_OK(object_type(*it, &obj_type));
throw_if_not_ok(object_type(resources_, *it, &obj_type));
if (obj_type != ObjectType::INVALID) {
obj_iter->objs_.push_front(*it);
obj_iter->expanded_.push_front(false);
Expand All @@ -672,7 +629,7 @@ Status StorageManagerCanonical::object_iter_next_postorder(
// Prepare the values to be returned
URI front_uri = obj_iter->objs_.front();
obj_iter->next_ = front_uri.to_string();
RETURN_NOT_OK(object_type(front_uri, type));
throw_if_not_ok(object_type(resources_, front_uri, type));
*path = obj_iter->next_.c_str();
*has_next = true;

Expand All @@ -688,7 +645,7 @@ Status StorageManagerCanonical::object_iter_next_preorder(
// Prepare the values to be returned
URI front_uri = obj_iter->objs_.front();
obj_iter->next_ = front_uri.to_string();
RETURN_NOT_OK(object_type(front_uri, type));
throw_if_not_ok(object_type(resources_, front_uri, type));
*path = obj_iter->next_.c_str();
*has_next = true;

Expand All @@ -706,7 +663,7 @@ Status StorageManagerCanonical::object_iter_next_preorder(
// Push the new TileDB objects in the front of the iterator's list
ObjectType obj_type;
for (auto it = uris.rbegin(); it != uris.rend(); ++it) {
RETURN_NOT_OK(object_type(*it, &obj_type));
throw_if_not_ok(object_type(resources_, *it, &obj_type));
if (obj_type != ObjectType::INVALID)
obj_iter->objs_.push_front(*it);
}
Expand Down Expand Up @@ -847,7 +804,7 @@ Status StorageManagerCanonical::group_metadata_consolidate(
}
// Check if group exists
ObjectType obj_type;
RETURN_NOT_OK(object_type(group_uri, &obj_type));
throw_if_not_ok(object_type(resources_, group_uri, &obj_type));

if (obj_type != ObjectType::GROUP) {
return logger_->status(Status_StorageManagerError(
Expand All @@ -873,7 +830,7 @@ void StorageManagerCanonical::group_metadata_vacuum(

// Check if group exists
ObjectType obj_type;
throw_if_not_ok(object_type(group_uri, &obj_type));
throw_if_not_ok(object_type(resources_, group_uri, &obj_type));

if (obj_type != ObjectType::GROUP) {
throw Status_StorageManagerError(
Expand Down
Loading

0 comments on commit afc03ad

Please sign in to comment.