Skip to content

Commit

Permalink
Add requested sizes to memory exceptions and logs. (#5346)
Browse files Browse the repository at this point in the history 
This adds some more context to exceptions and logging when hitting cases
where we have exceeded memory budgets.

---
TYPE: NO_HISTORY
DESC: Add requested sizes to memory exceptions and logs.
  • Loading branch information
@shaunrd0
shaunrd0 authored Oct 15, 2024
1 parent ae4c9a1 commit 39c665a
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 123 deletions.
74 changes: 35 additions & 39 deletions test/src/unit-dense-reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ struct CDenseFx {
std::string expected_error = std::string());
void reset_config();
void update_config();
void check_last_error(std::string expected_error);

CDenseFx();
~CDenseFx();
Expand Down Expand Up @@ -189,6 +190,16 @@ void CDenseFx::update_config() {
tiledb_config_free(&config);
}

void CDenseFx::check_last_error(std::string expected_error) {
tiledb_error_t* err = NULL;
tiledb_ctx_get_last_error(ctx_, &err);
// Retrieve the error message by invoking `tiledb_error_message`.
const char* msg;
tiledb_error_message(err, &msg);
CHECK_THAT(
std::string(msg), Catch::Matchers::ContainsSubstring(expected_error));
}

void CDenseFx::create_default_array_1d() {
int domain[] = {1, 200};
int tile_extent = 10;
Expand Down Expand Up @@ -516,13 +527,7 @@ void CDenseFx::read(
CHECK(rc == TILEDB_OK);
} else {
CHECK(rc == TILEDB_ERR);
tiledb_error_t* err = NULL;
tiledb_ctx_get_last_error(ctx_, &err);

// Retrieve the error message by invoking `tiledb_error_message`.
const char* msg;
tiledb_error_message(err, &msg);
CHECK(expected_error == std::string(msg));
check_last_error(expected_error);
}

// Check the internal loop count against expected value.
Expand Down Expand Up @@ -644,13 +649,7 @@ void CDenseFx::read_strings(
CHECK(rc == TILEDB_OK);
} else {
CHECK(rc == TILEDB_ERR);
tiledb_error_t* err = NULL;
tiledb_ctx_get_last_error(ctx_, &err);

// Retrieve the error message by invoking `tiledb_error_message`.
const char* msg;
tiledb_error_message(err, &msg);
CHECK(expected_error == std::string(msg));
check_last_error(expected_error);
}

if (rc == TILEDB_OK) {
Expand Down Expand Up @@ -820,16 +819,7 @@ void CDenseFx::read_fixed_strings(
CHECK(rc == TILEDB_OK);
} else {
CHECK(rc == TILEDB_ERR);

if (rc == TILEDB_ERR) {
tiledb_error_t* err = NULL;
tiledb_ctx_get_last_error(ctx_, &err);

// Retrieve the error message by invoking `tiledb_error_message`.
const char* msg;
tiledb_error_message(err, &msg);
CHECK(expected_error == std::string(msg));
}
check_last_error(expected_error);
}

if (rc == TILEDB_OK) {
Expand Down Expand Up @@ -919,7 +909,8 @@ TEST_CASE_METHOD(
data_r,
&data_r_size,
0,
"DenseReader: Cannot load tile offsets, increase memory budget");
"DenseReader: Cannot load tile offsets");
check_last_error("increase memory budget");
}

TEST_CASE_METHOD(
Expand Down Expand Up @@ -979,16 +970,16 @@ TEST_CASE_METHOD(
tile_upper_memory_limit_ = "50";
update_config();

std::string error_expected =
use_qc ?
"DenseReader: Cannot process a single tile because of query "
"condition, increase memory budget" :
"DenseReader: Cannot process a single tile, increase memory budget";
std::string error_expected = use_qc ?
"DenseReader: Cannot process a single tile "
"because of query condition" :
"DenseReader: Cannot process a single tile";

// Try to read.
int data_r[20] = {0};
uint64_t data_r_size = sizeof(data_r);
read(subarray, data_r, &data_r_size, use_qc, error_expected);
check_last_error("increase memory budget");
}

TEST_CASE_METHOD(
Expand Down Expand Up @@ -1063,10 +1054,9 @@ TEST_CASE_METHOD(
update_config();

std::string error_expected =
use_qc ?
"DenseReader: Cannot process a single tile because of query "
"condition, increase memory budget" :
"DenseReader: Cannot process a single tile, increase memory budget";
use_qc ? "DenseReader: Cannot process a single tile because of query "
"condition" :
"DenseReader: Cannot process a single tile";

// Try to read.
char data_r[NUM_CELLS * 2] = {0};
Expand All @@ -1081,6 +1071,7 @@ TEST_CASE_METHOD(
&offsets_r_size,
use_qc,
error_expected);
check_last_error("increase memory budget");
}

TEST_CASE_METHOD(
Expand Down Expand Up @@ -1243,7 +1234,8 @@ TEST_CASE_METHOD(
true,
true,
"DenseReader: Cannot process a single tile because of query "
"condition, increase memory budget");
"condition");
check_last_error("increase memory budget");
}

TEST_CASE_METHOD(
Expand Down Expand Up @@ -1300,7 +1292,8 @@ TEST_CASE_METHOD(
0,
false,
false,
"DenseReader: Cannot process a single tile, increase memory budget");
"DenseReader: Cannot process a single tile");
check_last_error("increase memory budget");

// Now read with QC set for a1 only.
read_fixed_strings(
Expand All @@ -1314,7 +1307,8 @@ TEST_CASE_METHOD(
0,
true,
false,
"DenseReader: Cannot process a single tile, increase memory budget");
"DenseReader: Cannot process a single tile");
check_last_error("increase memory budget");

// Now read with QC set for a2 only.
read_fixed_strings(
Expand All @@ -1328,7 +1322,8 @@ TEST_CASE_METHOD(
0,
false,
true,
"DenseReader: Cannot process a single tile, increase memory budget");
"DenseReader: Cannot process a single tile");
check_last_error("increase memory budget");

// Now read with QC set for a1 and a2, should fail.
read_fixed_strings(
Expand All @@ -1343,7 +1338,8 @@ TEST_CASE_METHOD(
true,
true,
"DenseReader: Cannot process a single tile because of query "
"condition, increase memory budget");
"condition");
check_last_error("increase memory budget");
}

#define LARGE_NUM_CELLS 50
Expand Down
11 changes: 9 additions & 2 deletions tiledb/sm/query/readers/dense_reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,10 @@ Status DenseReader::dense_read() {
if (total_tile_offsets_sizes >
memory_budget_ - array_memory_tracker_->get_memory_usage()) {
throw DenseReaderException(
"Cannot load tile offsets, increase memory budget");
"Cannot load tile offsets requiring " +
std::to_string(total_tile_offsets_sizes) +
" bytes, increase memory budget (" + std::to_string(memory_budget_) +
")");
}

auto&& [names, var_names] = field_names_to_process(qc_loaded_attr_names_set_);
Expand Down Expand Up @@ -959,7 +962,11 @@ DenseReader::compute_result_space_tiles(
// If a single tile doesn't fit in the available memory, we can't proceed.
if (total_memory > available_memory) {
throw DenseReaderException(
"Cannot process a single tile, increase memory budget");
"Cannot process a single tile requiring " +
std::to_string(total_memory) + " bytes with " +
std::to_string(available_memory) +
" available, increase memory budget (" +
std::to_string(memory_budget_) + ")");
}
}

Expand Down
7 changes: 5 additions & 2 deletions tiledb/sm/query/readers/ordered_dim_label_reader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ uint64_t OrderedDimLabelReader::create_result_tiles() {

// Process ranges one by one.
for (uint64_t r = 0; r < ranges_.size(); r++) {
// Add tiles for each fragments.
// Add tiles for each fragment.
for (unsigned f = 0; f < fragment_metadata_.size(); f++) {
// Add the tiles for the start/end range.
for (uint8_t range_index = 0; range_index < 2; range_index++) {
Expand Down Expand Up @@ -524,7 +524,10 @@ uint64_t OrderedDimLabelReader::create_result_tiles() {
} else {
if (r == 0) {
throw OrderedDimLabelReaderException(
"Can't process a single range, increase memory budget");
"Can't process a single range requiring " +
std::to_string(tile_size) +
" bytes, increase memory budget(" +
std::to_string(memory_budget_) + ")");
}
return r;
}
Expand Down
Loading

0 comments on commit 39c665a

Please sign in to comment.