Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the UUID generator. #4082

Closed
wants to merge 6 commits into from
Closed

Refactor the UUID generator. #4082

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0f105ba
Unstatus uuid.
teo-tsirpanis May 5, 2023
aee4d6d
Unify the UUID generator across platforms.
teo-tsirpanis Apr 27, 2023
631d6df
Separate generating and formatting the UUID.
teo-tsirpanis Apr 27, 2023
b2cd4f9
Fix formatting.
teo-tsirpanis May 8, 2023
0d6eb69
Merge branch 'dev' into uuid-refactor
teo-tsirpanis Nov 3, 2023
4168c64
Fix compile errors.
teo-tsirpanis Nov 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions tiledb/sm/array/array_directory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,10 +550,6 @@ URI ArrayDirectory::get_vacuum_uri(const URI& fragment_uri) const {

std::string ArrayDirectory::compute_new_fragment_name(
const URI& first, const URI& last, format_version_t format_version) const {
// Get uuid
std::string uuid;
throw_if_not_ok(uuid::generate_uuid(&uuid, false));

// For creating the new fragment URI

// Get timestamp ranges
Expand All @@ -563,8 +559,8 @@ std::string ArrayDirectory::compute_new_fragment_name(

// Create new URI
std::stringstream ss;
ss << "/__" << t_first.first << "_" << t_last.second << "_" << uuid << "_"
<< format_version;
ss << "/__" << t_first.first << "_" << t_last.second << "_"
<< uuid::generate_uuid(false) << "_" << format_version;

return ss.str();
}
Expand Down
20 changes: 5 additions & 15 deletions tiledb/sm/array_schema/array_schema.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ ArraySchema::ArraySchema(ArrayType array_type)
constants::cell_validity_compression_level));

// Generate URI and name for ArraySchema
throw_if_not_ok(generate_uri());
generate_uri();
}

ArraySchema::ArraySchema(
Expand Down Expand Up @@ -1385,36 +1385,26 @@ void ArraySchema::clear() {
timestamp_range_ = std::make_pair(0, 0);
}

Status ArraySchema::generate_uri() {
std::string uuid;
RETURN_NOT_OK(uuid::generate_uuid(&uuid, false));

void ArraySchema::generate_uri() {
auto timestamp = utils::time::timestamp_now_ms();
timestamp_range_ = std::make_pair(timestamp, timestamp);
std::stringstream ss;
ss << "__" << timestamp_range_.first << "_" << timestamp_range_.second << "_"
<< uuid;
<< uuid::generate_uuid(false);
name_ = ss.str();
uri_ =
array_uri_.join_path(constants::array_schema_dir_name).join_path(name_);

return Status::Ok();
}

Status ArraySchema::generate_uri(
void ArraySchema::generate_uri(
const std::pair<uint64_t, uint64_t>& timestamp_range) {
std::string uuid;
RETURN_NOT_OK(uuid::generate_uuid(&uuid, false));

timestamp_range_ = timestamp_range;
std::stringstream ss;
ss << "__" << timestamp_range_.first << "_" << timestamp_range_.second << "_"
<< uuid;
<< uuid::generate_uuid(false);
name_ = ss.str();
uri_ =
array_uri_.join_path(constants::array_schema_dir_name).join_path(name_);

return Status::Ok();
}

} // namespace sm
Expand Down
4 changes: 2 additions & 2 deletions tiledb/sm/array_schema/array_schema.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,10 @@ class ArraySchema {
void set_name(const std::string& name);

/** Generates a new array schema URI. */
Status generate_uri();
void generate_uri();

/** Generates a new array schema URI with specified timestamp range. */
Status generate_uri(const std::pair<uint64_t, uint64_t>& timestamp_range);
void generate_uri(const std::pair<uint64_t, uint64_t>& timestamp_range);

private:
/* ********************************* */
Expand Down
4 changes: 2 additions & 2 deletions tiledb/sm/array_schema/array_schema_evolution.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ ArraySchemaEvolution::evolve_schema(
if (std::get<0>(timestamp_range_) != 0) {
RETURN_NOT_OK_TUPLE(
schema.get()->set_timestamp_range(timestamp_range_), nullopt);
RETURN_NOT_OK_TUPLE(schema->generate_uri(timestamp_range_), nullopt);
schema->generate_uri(timestamp_range_);
} else {
// Generate new schema URI
RETURN_NOT_OK_TUPLE(schema->generate_uri(), nullopt);
schema->generate_uri();
}

return {Status::Ok(), schema};
Expand Down
6 changes: 2 additions & 4 deletions tiledb/sm/group/group.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,14 +726,12 @@ std::string Group::dump(
/* ********************************* */

tuple<Status, optional<std::string>> Group::generate_name() const {
std::string uuid;
RETURN_NOT_OK_TUPLE(uuid::generate_uuid(&uuid, false), std::nullopt);

const auto& version = group_details_->version();
auto timestamp =
(timestamp_end_ != 0) ? timestamp_end_ : utils::time::timestamp_now_ms();
std::stringstream ss;
ss << "__" << timestamp << "_" << timestamp << "_" << uuid;
ss << "__" << timestamp << "_" << timestamp << "_"
<< uuid::generate_uuid(false);
if (version > 1) {
ss << "_" << version;
}
Expand Down
8 changes: 2 additions & 6 deletions tiledb/sm/group/group_directory.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ Status GroupDirectory::load() {

tuple<Status, optional<std::string>> GroupDirectory::compute_new_fragment_name(
const URI& first, const URI& last, format_version_t format_version) const {
// Get uuid
std::string uuid;
RETURN_NOT_OK_TUPLE(uuid::generate_uuid(&uuid, false), nullopt);

// For creating the new fragment URI

// Get timestamp ranges
Expand All @@ -186,8 +182,8 @@ tuple<Status, optional<std::string>> GroupDirectory::compute_new_fragment_name(

// Create new URI
std::stringstream ss;
ss << "/__" << t_first.first << "_" << t_last.second << "_" << uuid << "_"
<< format_version;
ss << "/__" << t_first.first << "_" << t_last.second << "_"
<< uuid::generate_uuid(false) << "_" << format_version;

return {Status::Ok(), ss.str()};
}
Expand Down
5 changes: 1 addition & 4 deletions tiledb/sm/metadata/metadata.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,9 @@ Status Metadata::get_uri(const URI& array_uri, URI* meta_uri) {
}

Status Metadata::generate_uri(const URI& array_uri) {
std::string uuid;
RETURN_NOT_OK(uuid::generate_uuid(&uuid, false));

std::stringstream ss;
ss << "__" << timestamp_range_.first << "_" << timestamp_range_.second << "_"
<< uuid;
<< uuid::generate_uuid(false);
uri_ = array_uri.join_path(constants::array_metadata_dir_name)
.join_path(ss.str());

Expand Down
11 changes: 5 additions & 6 deletions tiledb/sm/misc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,14 @@ conclude(object_library)
#
commence(object_library uuid)
this_target_sources(uuid.cc)
this_target_object_libraries(baseline)
if(WIN32)
if(MSVC)
find_library(RPCRT4_LIBRARY rpcrt4)
message(STATUS "Found Win32 lib bcrypt: ${RPCRT4_LIBRARY}")
this_target_link_libraries(${RPCRT4_LIBRARY})
find_library(BCRYPT_LIBRARY bcrypt)
message(STATUS "Found Win32 lib bcrypt: ${BCRYPT_LIBRARY}")
this_target_link_libraries(${BCRYPT_LIBRARY})
else()
message(STATUS "Linking to Win32 lib rpcrt4")
this_target_link_libraries(-lrpcrt4)
message(STATUS "Linking to Win32 lib bcrypt")
this_target_link_libraries(-lbcrypt)
endif()
else()
find_package(OpenSSL_EP REQUIRED)
Expand Down
2 changes: 1 addition & 1 deletion tiledb/sm/misc/test/compile_uuid_main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@
#include "../uuid.h"

int main() {
(void)tiledb::sm::uuid::generate_uuid(nullptr, false);
(void)tiledb::sm::uuid::generate_uuid(false);
return 0;
}
15 changes: 11 additions & 4 deletions tiledb/sm/misc/test/unit_uuid.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,26 @@ std::mutex catch2_macro_mutex;
REQUIRE(a); \
}

// A thread-safe variant of the REQUIRE_NOTHROW macro.
#define REQUIRE_NOTHROW_SAFE(a) \
{ \
std::lock_guard<std::mutex> lock(catch2_macro_mutex); \
REQUIRE_NOTHROW(a); \
}

void cancel_all_tasks(StorageManager*) {
}

TEST_CASE("UUID: Test generate", "[uuid]") {
SECTION("- Serial") {
std::string uuid0, uuid1, uuid2;
REQUIRE(uuid::generate_uuid(&uuid0).ok());
REQUIRE_NOTHROW(uuid0 = uuid::generate_uuid());
REQUIRE(uuid0.length() == 36);
REQUIRE(uuid::generate_uuid(&uuid1).ok());
REQUIRE_NOTHROW(uuid1 = uuid::generate_uuid());
REQUIRE(uuid1.length() == 36);
REQUIRE(uuid0 != uuid1);

REQUIRE(uuid::generate_uuid(&uuid2, false).ok());
REQUIRE_NOTHROW(uuid2 = uuid::generate_uuid(false));
REQUIRE(uuid2.length() == 32);
}

Expand All @@ -72,7 +79,7 @@ TEST_CASE("UUID: Test generate", "[uuid]") {
for (unsigned i = 0; i < nthreads; i++) {
threads.emplace_back([&uuids, i]() {
std::string& uuid = uuids[i];
REQUIRE_SAFE(uuid::generate_uuid(&uuid).ok());
REQUIRE_NOTHROW_SAFE(uuid = uuid::generate_uuid());
REQUIRE_SAFE(uuid.length() == 36);
});
}
Expand Down
Loading