Skip to content

Commit

Permalink
Do not create empty .vac files in metadata consolidation
Browse files Browse the repository at this point in the history
  • Loading branch information
ypatia committed Feb 13, 2025
1 parent 1ca277c commit ef07fe9
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 3 deletions.
45 changes: 45 additions & 0 deletions test/src/unit-capi-consolidation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7895,3 +7895,48 @@ TEST_CASE_METHOD(
remove_sparse_array();
}
}

TEST_CASE_METHOD(
ConsolidationFx,
"C API: Test consolidating empty array metadata",
"[capi][consolidation][array-meta][empty][sc-62900][non-rest]") {
remove_dense_array();
create_dense_array();

SECTION("Consolidate empty array metadata") {
// Configuration for consolidating array metadata
tiledb_config_t* config = nullptr;
tiledb_error_t* error = nullptr;
REQUIRE(tiledb_config_alloc(&config, &error) == TILEDB_OK);
REQUIRE(error == nullptr);
int rc = tiledb_config_set(
config, "sm.consolidation.mode", "array_meta", &error);
REQUIRE(rc == TILEDB_OK);
REQUIRE(error == nullptr);

// Consolidate without having added any array meta
rc = tiledb_array_consolidate(ctx_, dense_array_uri_.c_str(), config);
CHECK(rc == TILEDB_OK);

// We must have 0 vacuum files after consolidation.
std::vector<std::string> array_meta_vac_files;
get_array_meta_vac_files_dense(array_meta_vac_files);
CHECK(array_meta_vac_files.size() == 0);

tiledb_array_t* array;
rc = tiledb_array_alloc(ctx_, dense_array_uri_.c_str(), &array);
REQUIRE(rc == TILEDB_OK);

// Open for Delete, so that array metadata are loaded from disk, and check
// there is no failure
rc = tiledb_array_open(ctx_, array, TILEDB_DELETE);
REQUIRE(rc == TILEDB_OK);

// Vacuum consolidated array metadata (empty) and check there is no failure
rc = tiledb_config_set(config, "sm.vacuum.mode", "array_meta", &error);
REQUIRE(rc == TILEDB_OK);
REQUIRE(error == nullptr);
rc = tiledb_array_vacuum(ctx_, dense_array_uri_.c_str(), config);
CHECK(rc == TILEDB_OK);
}
}
15 changes: 14 additions & 1 deletion test/src/unit-capi-group.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,19 @@ TEST_CASE_METHOD(
tiledb::sm::URI group_uri(temp_dir + "group");
REQUIRE(tiledb_group_create(ctx_, group_uri.c_str()) == TILEDB_OK);

// Consolidate empty metadata
tiledb_config_t* cfg;
tiledb_error_t* err = nullptr;
REQUIRE(tiledb_config_alloc(&cfg, &err) == TILEDB_OK);
int rc = tiledb_group_consolidate_metadata(ctx_, group_uri.c_str(), cfg);
REQUIRE(rc == TILEDB_OK);

// Check no .vac file is created
std::vector<std::string> group_empty_meta_vac_files;
get_meta_vac_files(group_uri, group_empty_meta_vac_files);
CHECK(group_empty_meta_vac_files.size() == 0);
tiledb_config_free(&cfg);

write_group_metadata(group_uri.c_str());
std::this_thread::sleep_for(std::chrono::milliseconds(1));
auto start = tiledb::sm::utils::time::timestamp_now_ms();
Expand All @@ -1100,7 +1113,7 @@ TEST_CASE_METHOD(
CHECK(group_meta_files.size() == 4);

uint64_t file_size = 0;
int rc = tiledb_vfs_file_size(
rc = tiledb_vfs_file_size(
ctx_, vfs_, group_meta_vac_files[0].c_str(), &file_size);
REQUIRE(rc == TILEDB_OK);

Expand Down
7 changes: 6 additions & 1 deletion tiledb/sm/consolidator/array_meta_consolidator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ Status ArrayMetaConsolidator::consolidate(
encryption_key,
key_length));

// Check if there's actually more than 1 file to consolidate
auto& metadata_r = array_for_reads.metadata();
if (metadata_r.loaded_metadata_uris().size() <= 1) {
return Status::Ok();
}

// Open array for writing
Array array_for_writes(resources_, array_uri);
RETURN_NOT_OK_ELSE(
Expand All @@ -88,7 +94,6 @@ Status ArrayMetaConsolidator::consolidate(
throw_if_not_ok(array_for_reads.close()));

// Copy-assign the read metadata into the metadata of the array for writes
auto& metadata_r = array_for_reads.metadata();
array_for_writes.opened_array()->metadata() = metadata_r;
URI new_uri = metadata_r.get_uri(array_uri);
const auto& to_vacuum = metadata_r.loaded_metadata_uris();
Expand Down
7 changes: 6 additions & 1 deletion tiledb/sm/consolidator/group_meta_consolidator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,13 @@ Status GroupMetaConsolidator::consolidate(
group_for_reads.close();
}

// Copy-assign the read metadata into the metadata of the group for writes
// Check if there's actually more than 1 file to consolidate
auto metadata_r = group_for_reads.metadata();
if (metadata_r->loaded_metadata_uris().size() <= 1) {
return Status::Ok();
}

// Copy-assign the read metadata into the metadata of the group for writes
*(group_for_writes.metadata()) = *metadata_r;
URI new_uri = metadata_r->get_uri(group_uri);
const auto& to_vacuum = metadata_r->loaded_metadata_uris();
Expand Down

0 comments on commit ef07fe9

Please sign in to comment.