Skip to content

Commit

Permalink
Avoid empty HTTP requests to load Enumerations (#4369)
Browse files Browse the repository at this point in the history
Previously, we weren't checking if we actually had any enumerations to load which resulted in REST requests for an empty list of enumerations. That just wastes time and resources for no reason so this change avoids the request whn there's no work to do.

---
TYPE: IMPROVEMENT
DESC: Avoid empty Enumeration REST requests.
  • Loading branch information
davisp authored Sep 22, 2023
1 parent c524631 commit 7d17d10
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 32 deletions.
68 changes: 36 additions & 32 deletions tiledb/sm/array/array.cc
Original file line number Diff line number Diff line change
Expand Up @@ -599,43 +599,47 @@ std::vector<shared_ptr<const Enumeration>> Array::get_enumerations(
enmrs_to_load.insert(enmr_name);
}

std::vector<shared_ptr<const Enumeration>> loaded;
// Only attempt to load enumerations if we have at least one Enumeration
// to load.
if (enmrs_to_load.size() > 0) {
std::vector<shared_ptr<const Enumeration>> loaded;

if (remote_) {
auto rest_client = resources_.rest_client();
if (rest_client == nullptr) {
throw ArrayException(
"Error loading enumerations; "
"Remote array with no REST client.");
}
if (remote_) {
auto rest_client = resources_.rest_client();
if (rest_client == nullptr) {
throw ArrayException(
"Error loading enumerations; "
"Remote array with no REST client.");
}

std::vector<std::string> names_to_load;
for (auto& enmr_name : enmrs_to_load) {
names_to_load.push_back(enmr_name);
}
std::vector<std::string> names_to_load;
for (auto& enmr_name : enmrs_to_load) {
names_to_load.push_back(enmr_name);
}

loaded = rest_client->post_enumerations_from_rest(
array_uri_,
timestamp_start_,
timestamp_end_opened_at_,
this,
names_to_load);
} else {
// Create a vector of paths to be loaded.
std::vector<std::string> paths_to_load;
for (auto& enmr_name : enmrs_to_load) {
auto path = array_schema_latest_->get_enumeration_path_name(enmr_name);
paths_to_load.push_back(path);
}
loaded = rest_client->post_enumerations_from_rest(
array_uri_,
timestamp_start_,
timestamp_end_opened_at_,
this,
names_to_load);
} else {
// Create a vector of paths to be loaded.
std::vector<std::string> paths_to_load;
for (auto& enmr_name : enmrs_to_load) {
auto path = array_schema_latest_->get_enumeration_path_name(enmr_name);
paths_to_load.push_back(path);
}

// Load the enumerations from storage
loaded = array_dir_.load_enumerations_from_paths(
paths_to_load, get_encryption_key());
}
// Load the enumerations from storage
loaded = array_dir_.load_enumerations_from_paths(
paths_to_load, get_encryption_key());
}

// Store the loaded enumerations in the schema
for (auto& enmr : loaded) {
array_schema_latest_->store_enumeration(enmr);
// Store the loaded enumerations in the schema
for (auto& enmr : loaded) {
array_schema_latest_->store_enumeration(enmr);
}
}

// Return the requested list of enumerations
Expand Down
7 changes: 7 additions & 0 deletions tiledb/sm/array/array_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,13 @@ std::vector<shared_ptr<const Enumeration>>
ArrayDirectory::load_enumerations_from_paths(
const std::vector<std::string>& enumeration_paths,
const EncryptionKey& encryption_key) const {
// This should never be called with an empty list of enumeration paths, but
// there's no reason to not check an early return case here given that code
// changes.
if (enumeration_paths.size() == 0) {
return {};
}

std::vector<shared_ptr<const Enumeration>> ret(enumeration_paths.size());
auto& tp = resources_.get().io_tp();
throw_if_not_ok(parallel_for(&tp, 0, enumeration_paths.size(), [&](size_t i) {
Expand Down
7 changes: 7 additions & 0 deletions tiledb/sm/rest/rest_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,13 @@ RestClient::post_enumerations_from_rest(
"Error getting enumerations from REST; array is null.");
}

// This should never be called with an empty list of enumeration names, but
// there's no reason to not check an early return case here given that code
// changes.
if (enumeration_names.size() == 0) {
return {};
}

Buffer buf;
serialization::serialize_load_enumerations_request(
array->config(), enumeration_names, serialization_type_, buf);
Expand Down

0 comments on commit 7d17d10

Please sign in to comment.