Skip to content

Commit 1326ed4

Browse files
authored
Remove Status_StorageManagerError and Migrate group_metadata_vacuum out of StorageManager. (#5034)
Remove `Status_StorageManagerError` and its occurrences. Related subsequent fixes: * Remove `Logger::status_no_return_value` and replace occurrences with `Logger::error`. * Remove `Status` and `std::optional` from returned tuple of `Array::open_for_writes`. * de-`Status` `Context::init_loggers`. * Migrate `StorageManagerCanonical::group_metadata_vacuum` -> `static Group::vacuum_metadata`. [sc-48630] [sc-48639] --- TYPE: NO_HISTORY DESC: Remove `Status_StorageManagerError` and Migrate `group_metadata_vacuum` out of `StorageManager`.
1 parent bc4ec51 commit 1326ed4

File tree

19 files changed

+115
-150
lines changed

19 files changed

+115
-150
lines changed

tiledb/api/c_api/group/group_api.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ capi_return_t tiledb_group_vacuum_metadata(
587587
ensure_group_uri_argument_is_valid(group_uri);
588588

589589
auto cfg = (config == nullptr) ? ctx->config() : config->config();
590-
ctx->storage_manager()->group_metadata_vacuum(group_uri, cfg);
590+
sm::Group::vacuum_metadata(ctx->resources(), group_uri, cfg);
591591

592592
return TILEDB_OK;
593593
}

tiledb/api/c_api_test_support/storage_manager_stub/storage_manager_override.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,6 @@ class StorageManagerStub {
6161
inline Status cancel_all_tasks() {
6262
return Status{};
6363
}
64-
inline Status group_metadata_vacuum(const char*, const Config&) {
65-
throw std::logic_error(
66-
"StorageManagerStub does not support group metadata vacuum");
67-
}
6864
inline Status set_tag(const std::string&, const std::string&) {
6965
return Status{};
7066
}

tiledb/common/exception/status.h

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The BSD License
77
*
8-
* @copyright Copyright (c) 2017-2022 TileDB, Inc.
8+
* @copyright Copyright (c) 2017-2024 TileDB, Inc.
99
* Copyright (c) 2011 The LevelDB Authors. All rights reserved.
1010
*
1111
* Redistribution and use in source and binary forms, with or without
@@ -60,8 +60,7 @@
6060
#include "tiledb/common/common-std.h"
6161
#include "tiledb/common/heap_memory.h"
6262

63-
namespace tiledb {
64-
namespace common {
63+
namespace tiledb::common {
6564

6665
#define RETURN_NOT_OK(s) \
6766
do { \
@@ -250,10 +249,6 @@ inline Status Status_Ok() {
250249
inline Status Status_Error(const std::string& msg) {
251250
return {"Error", msg};
252251
};
253-
/** Return a StorageManager error class Status with a given message **/
254-
inline Status Status_StorageManagerError(const std::string& msg) {
255-
return {"[TileDB::StorageManager] Error", msg};
256-
}
257252
/** Return a FragmentMetadata error class Status with a given message **/
258253
inline Status Status_FragmentMetadataError(const std::string& msg) {
259254
return {"[TileDB::FragmentMetadata] Error", msg};
@@ -433,7 +428,6 @@ inline Status Status_TaskError(const std::string& msg) {
433428
inline Status Status_RangeError(const std::string& msg) {
434429
return {"[TileDB::Range] Error", msg};
435430
}
436-
} // namespace common
437-
} // namespace tiledb
431+
} // namespace tiledb::common
438432

439433
#endif // TILEDB_STATUS_H

tiledb/common/logger.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The MIT License
77
*
8-
* @copyright Copyright (c) 2017-2021 TileDB, Inc.
8+
* @copyright Copyright (c) 2017-2024 TileDB, Inc.
99
*
1010
* Permission is hereby granted, free of charge, to any person obtaining a copy
1111
* of this software and associated documentation files (the "Software"), to deal
@@ -147,10 +147,6 @@ Status Logger::status(const Status& st) {
147147
return st;
148148
}
149149

150-
void Logger::status_no_return_value(const Status& st) {
151-
logger_->error(st.message());
152-
}
153-
154150
void Logger::trace(const std::string& msg) {
155151
logger_->trace(msg);
156152
}

tiledb/common/logger.h

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* The MIT License
77
*
8-
* @copyright Copyright (c) 2017-2021 TileDB, Inc.
8+
* @copyright Copyright (c) 2017-2024 TileDB, Inc.
99
*
1010
* Permission is hereby granted, free of charge, to any person obtaining a copy
1111
* of this software and associated documentation files (the "Software"), to deal
@@ -48,8 +48,7 @@ namespace spdlog {
4848
class logger;
4949
}
5050

51-
namespace tiledb {
52-
namespace common {
51+
namespace tiledb::common {
5352

5453
/** Definition of class Logger. */
5554
class Logger {
@@ -315,13 +314,6 @@ class Logger {
315314
*/
316315
Status status(const Status& st);
317316

318-
/**
319-
* Log a message from a Status object without returning it.
320-
*
321-
* @param st The Status object to log
322-
*/
323-
void status_no_return_value(const Status& st);
324-
325317
/**
326318
* Log an error and exit with a non-zero status.
327319
*
@@ -457,8 +449,7 @@ inline Status logger_format_from_string(
457449
return Status::Ok();
458450
}
459451

460-
} // namespace common
461-
} // namespace tiledb
452+
} // namespace tiledb::common
462453

463454
// Also include the public-permissible logger functions here.
464455
#include "tiledb/common/logger_public.h"

tiledb/sm/array/array.cc

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -585,12 +585,11 @@ Status Array::open(
585585
array_dir_timestamp_end_,
586586
ArrayDirectoryMode::SCHEMA_ONLY));
587587
}
588-
auto&& [st, array_schema_latest, array_schemas] = open_for_writes();
589-
throw_if_not_ok(st);
590588

591589
// Set schemas
592-
set_array_schema_latest(array_schema_latest.value());
593-
set_array_schemas_all(std::move(array_schemas.value()));
590+
auto&& [array_schema_latest, array_schemas] = open_for_writes();
591+
set_array_schema_latest(array_schema_latest);
592+
set_array_schemas_all(std::move(array_schemas));
594593

595594
// Set the timestamp
596595
opened_array_->metadata().reset(timestamp_end_opened_at());
@@ -606,12 +605,11 @@ Status Array::open(
606605
array_dir_timestamp_end_,
607606
ArrayDirectoryMode::READ));
608607
}
609-
auto&& [st, latest, array_schemas] = open_for_writes();
610-
throw_if_not_ok(st);
611608

612609
// Set schemas
613-
set_array_schema_latest(latest.value());
614-
set_array_schemas_all(std::move(array_schemas.value()));
610+
auto&& [latest, array_schemas] = open_for_writes();
611+
set_array_schema_latest(latest);
612+
set_array_schemas_all(std::move(array_schemas));
615613

616614
auto version = array_schema_latest().version();
617615
if (query_type == QueryType::DELETE &&
@@ -1842,19 +1840,15 @@ Array::open_for_reads_without_fragments() {
18421840
}
18431841

18441842
tuple<
1845-
Status,
1846-
optional<shared_ptr<ArraySchema>>,
1847-
optional<std::unordered_map<std::string, shared_ptr<ArraySchema>>>>
1843+
shared_ptr<ArraySchema>,
1844+
std::unordered_map<std::string, shared_ptr<ArraySchema>>>
18481845
Array::open_for_writes() {
18491846
auto timer_se =
18501847
resources_.stats().start_timer("array_open_write_load_schemas");
18511848
// Checks
1852-
if (!resources_.vfs().supports_uri_scheme(array_uri_))
1853-
return {
1854-
resources_.logger()->status(Status_StorageManagerError(
1855-
"Cannot open array; URI scheme unsupported.")),
1856-
nullopt,
1857-
nullopt};
1849+
if (!resources_.vfs().supports_uri_scheme(array_uri_)) {
1850+
throw ArrayException("Cannot open array; URI scheme unsupported.");
1851+
}
18581852

18591853
// Load array schemas
18601854
auto&& [array_schema_latest, array_schemas_all] =
@@ -1867,31 +1861,21 @@ Array::open_for_writes() {
18671861
auto version = array_schema_latest->version();
18681862
if constexpr (is_experimental_build) {
18691863
if (version != constants::format_version) {
1870-
std::stringstream err;
1871-
err << "Cannot open array for writes; Array format version (";
1872-
err << version;
1873-
err << ") is not the library format version (";
1874-
err << constants::format_version << ")";
1875-
return {
1876-
resources_.logger()->status(Status_StorageManagerError(err.str())),
1877-
nullopt,
1878-
nullopt};
1864+
throw ArrayException(
1865+
"Cannot open array for writes; Array format version (" +
1866+
std::to_string(version) + ") is not the library format version (" +
1867+
std::to_string(constants::format_version) + ")");
18791868
}
18801869
} else {
18811870
if (version > constants::format_version) {
1882-
std::stringstream err;
1883-
err << "Cannot open array for writes; Array format version (";
1884-
err << version;
1885-
err << ") is newer than library format version (";
1886-
err << constants::format_version << ")";
1887-
return {
1888-
resources_.logger()->status(Status_StorageManagerError(err.str())),
1889-
nullopt,
1890-
nullopt};
1871+
throw ArrayException(
1872+
"Cannot open array for writes; Array format version (" +
1873+
std::to_string(version) + ") is newer than library format version (" +
1874+
std::to_string(constants::format_version) + ")");
18911875
}
18921876
}
18931877

1894-
return {Status::Ok(), array_schema_latest, array_schemas_all};
1878+
return {array_schema_latest, array_schemas_all};
18951879
}
18961880

18971881
void Array::clear_last_max_buffer_sizes() {
@@ -2326,12 +2310,10 @@ void Array::ensure_array_is_valid_for_delete(const URI& uri) {
23262310
void ensure_supported_schema_version_for_read(format_version_t version) {
23272311
// We do not allow reading from arrays written by newer version of TileDB
23282312
if (version > constants::format_version) {
2329-
std::stringstream err;
2330-
err << "Cannot open array for reads; Array format version (";
2331-
err << version;
2332-
err << ") is newer than library format version (";
2333-
err << constants::format_version << ")";
2334-
throw Status_StorageManagerError(err.str());
2313+
throw ArrayException(
2314+
"Cannot open array for reads; Array format version (" +
2315+
std::to_string(version) + ") is newer than library format version (" +
2316+
std::to_string(constants::format_version) + ")");
23352317
}
23362318
}
23372319

tiledb/sm/array/array.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1200,19 +1200,18 @@ class Array {
12001200
std::unordered_map<std::string, shared_ptr<ArraySchema>>>
12011201
open_for_reads_without_fragments();
12021202

1203-
/** Opens an array for writes.
1203+
/**
1204+
* Opens an array for writes.
12041205
*
12051206
* @param array The array to open.
1206-
* @return tuple of Status, latest ArraySchema and map of all array schemas
1207-
* Status Ok on success, else error
1207+
* @return tuple of latest ArraySchema and map of all array schemas
12081208
* ArraySchema The array schema to be retrieved after the
12091209
* array is opened.
12101210
* ArraySchemaMap Map of all array schemas found keyed by name
12111211
*/
12121212
tuple<
1213-
Status,
1214-
optional<shared_ptr<ArraySchema>>,
1215-
optional<std::unordered_map<std::string, shared_ptr<ArraySchema>>>>
1213+
shared_ptr<ArraySchema>,
1214+
std::unordered_map<std::string, shared_ptr<ArraySchema>>>
12161215
open_for_writes();
12171216

12181217
/** Clears the cached max buffer sizes and subarray. */

tiledb/sm/consolidator/array_meta_consolidator.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ Status ArrayMetaConsolidator::consolidate(
120120

121121
void ArrayMetaConsolidator::vacuum(const char* array_name) {
122122
if (array_name == nullptr) {
123-
throw Status_StorageManagerError(
123+
throw std::invalid_argument(
124124
"Cannot vacuum array metadata; Array name cannot be null");
125125
}
126126

tiledb/sm/consolidator/commits_consolidator.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Status CommitsConsolidator::consolidate(
103103

104104
void CommitsConsolidator::vacuum(const char* array_name) {
105105
if (array_name == nullptr) {
106-
throw Status_StorageManagerError(
106+
throw std::invalid_argument(
107107
"Cannot vacuum array metadata; Array name cannot be null");
108108
}
109109

tiledb/sm/consolidator/fragment_meta_consolidator.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ Status FragmentMetaConsolidator::consolidate(
181181

182182
void FragmentMetaConsolidator::vacuum(const char* array_name) {
183183
if (array_name == nullptr) {
184-
throw Status_StorageManagerError(
184+
throw std::invalid_argument(
185185
"Cannot vacuum fragment metadata; Array name cannot be null");
186186
}
187187

0 commit comments

Comments
 (0)