Skip to content

[fix](cloud) Avoid deleting nonexistent delete bitmap files #62967

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

Open
wyxxxcat wants to merge 1 commit into
base: master
Choose a base branch
from
+89 −24
Open

[fix](cloud) Avoid deleting nonexistent delete bitmap files #62967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 26 additions & 19 deletions cloud/src/recycler/recycler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3000,18 +3000,17 @@ int InstanceRecycler::delete_rowset_data(const RowsetMetaCloudPB& rs_meta_pb) {
}
}

// Process delete bitmap - check if it's stored in packed file
bool delete_bitmap_is_packed = false;
// Process delete bitmap - check where it's stored.
DeleteBitmapStorageType delete_bitmap_storage_type = DeleteBitmapStorageType::NOT_FOUND;
if (decrement_delete_bitmap_packed_file_ref_counts(tablet_id, rowset_id,
&delete_bitmap_is_packed) != 0) {
&delete_bitmap_storage_type) != 0) {
LOG_WARNING("failed to decrement delete bitmap packed file ref count")
.tag("instance_id", instance_id_)
.tag("tablet_id", tablet_id)
.tag("rowset_id", rowset_id);
return -1;
}
// Only delete standalone delete bitmap file if not stored in packed file
if (!delete_bitmap_is_packed) {
if (delete_bitmap_storage_type == DeleteBitmapStorageType::STANDALONE_FILE) {
file_paths.push_back(delete_bitmap_path(tablet_id, rowset_id));
}
// TODO(AlexYue): seems could do do batch
Expand Down Expand Up @@ -3257,11 +3256,11 @@ int InstanceRecycler::decrement_packed_file_ref_counts(const doris::RowsetMetaCl
return ret;
}

int InstanceRecycler::decrement_delete_bitmap_packed_file_ref_counts(int64_t tablet_id,
const std::string& rowset_id,
bool* out_is_packed) {
if (out_is_packed) {
*out_is_packed = false;
int InstanceRecycler::decrement_delete_bitmap_packed_file_ref_counts(
int64_t tablet_id, const std::string& rowset_id,
DeleteBitmapStorageType* out_storage_type) {
if (out_storage_type) {
*out_storage_type = DeleteBitmapStorageType::NOT_FOUND;
}

// Get delete bitmap storage info from FDB
Expand Down Expand Up @@ -3305,15 +3304,24 @@ int InstanceRecycler::decrement_delete_bitmap_packed_file_ref_counts(int64_t tab
return -1;
}

// Check if delete bitmap is stored in packed file
if (storage.store_in_fdb()) {
if (out_storage_type) {
*out_storage_type = DeleteBitmapStorageType::IN_FDB;
}
return 0;
}

// Check if delete bitmap is stored in standalone file.
if (!storage.has_packed_slice_location() ||
storage.packed_slice_location().packed_file_path().empty()) {
// Not stored in packed file, nothing to do
if (out_storage_type) {
*out_storage_type = DeleteBitmapStorageType::STANDALONE_FILE;
}
return 0;
}

if (out_is_packed) {
*out_is_packed = true;
if (out_storage_type) {
*out_storage_type = DeleteBitmapStorageType::PACKED_FILE;
}

const auto& packed_loc = storage.packed_slice_location();
Expand Down Expand Up @@ -3652,19 +3660,18 @@ int InstanceRecycler::delete_rowset_data(
continue;
}

// Process delete bitmap - check if it's stored in packed file
bool delete_bitmap_is_packed = false;
// Process delete bitmap - check where it's stored.
DeleteBitmapStorageType delete_bitmap_storage_type = DeleteBitmapStorageType::NOT_FOUND;
if (decrement_delete_bitmap_packed_file_ref_counts(tablet_id, rowset_id,
&delete_bitmap_is_packed) != 0) {
&delete_bitmap_storage_type) != 0) {
LOG_WARNING("failed to decrement delete bitmap packed file ref count")
.tag("instance_id", instance_id_)
.tag("tablet_id", tablet_id)
.tag("rowset_id", rowset_id);
ret = -1;
continue;
}
// Only delete standalone delete bitmap file if not stored in packed file
if (!delete_bitmap_is_packed) {
if (delete_bitmap_storage_type == DeleteBitmapStorageType::STANDALONE_FILE) {
file_paths.push_back(delete_bitmap_path(tablet_id, rowset_id));
}

Expand Down
14 changes: 10 additions & 4 deletions cloud/src/recycler/recycler.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,13 +463,19 @@ class InstanceRecycler {
// Returns 0 for success, -1 for error.
int decrement_packed_file_ref_counts(const doris::RowsetMetaCloudPB& rs_meta_pb);

// Decrement packed file ref count for delete bitmap if it's stored in packed file.
enum class DeleteBitmapStorageType {
NOT_FOUND,
IN_FDB,
STANDALONE_FILE,
PACKED_FILE,
};

// Process delete bitmap storage and decrement packed file ref count when needed.
// Returns 0 for success, -1 for error.
// If delete bitmap is not stored in packed file, this function does nothing and returns 0.
// out_is_packed: if not null, will be set to true if delete bitmap is stored in packed file.
// out_storage_type: if not null, will be set to the delete bitmap storage type.
int decrement_delete_bitmap_packed_file_ref_counts(int64_t tablet_id,
const std::string& rowset_id,
bool* out_is_packed);
DeleteBitmapStorageType* out_storage_type);

int delete_packed_file_and_kv(const std::string& packed_file_path,
const std::string& packed_key,
Expand Down
2 changes: 1 addition & 1 deletion cloud/test/mock_accessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ inline int MockAccessor::delete_all(int64_t expiration_time) {
}

inline int MockAccessor::delete_files(const std::vector<std::string>& paths) {
TEST_SYNC_POINT_RETURN_WITH_VALUE("MockAccessor::delete_files", (int)0);
TEST_SYNC_POINT_RETURN_WITH_VALUE("MockAccessor::delete_files", (int)0, &paths);

for (auto&& path : paths) {
delete_file(path);
Expand Down
52 changes: 52 additions & 0 deletions cloud/test/recycler_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include <exception>
#include <future>
#include <memory>
#include <numeric>
#include <random>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -5571,6 +5572,57 @@ TEST(RecyclerTest, delete_rowset_data) {
}
}

TEST(RecyclerTest, delete_rowset_data_without_delete_bitmap_meta) {
auto txn_kv = std::make_shared<MemTxnKv>();
ASSERT_EQ(txn_kv->init(), 0);

constexpr std::string_view resource_id = "delete_rowset_data_without_delete_bitmap_meta";
InstanceInfoPB instance;
instance.set_instance_id(instance_id);
auto obj_info = instance.add_obj_info();
obj_info->set_id(std::string(resource_id));
obj_info->set_ak(config::test_s3_ak);
obj_info->set_sk(config::test_s3_sk);
obj_info->set_endpoint(config::test_s3_endpoint);
obj_info->set_region(config::test_s3_region);
obj_info->set_bucket(config::test_s3_bucket);
obj_info->set_prefix(std::string(resource_id));

InstanceRecycler recycler(txn_kv, instance, thread_group,
std::make_shared<TxnLazyCommitter>(txn_kv));
ASSERT_EQ(recycler.init(), 0);
auto accessor = recycler.accessor_map_.begin()->second;

doris::TabletSchemaCloudPB schema;
schema.set_schema_version(1);
schema.set_inverted_index_storage_format(InvertedIndexStorageFormatPB::V1);

auto rowset = create_rowset(std::string(resource_id), 10002, 10001, 1, schema);
ASSERT_EQ(create_recycle_rowset(txn_kv.get(), accessor.get(), rowset, RecycleRowsetPB::COMPACT,
true),
0);

auto sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
SyncPoint::get_instance()->clear_all_call_backs();
};
std::vector<std::string> deleted_paths;
sp->set_call_back("MockAccessor::delete_files", [&](auto&& args) {
auto* paths = try_any_cast<const std::vector<std::string>*>(args[0]);
deleted_paths.insert(deleted_paths.end(), paths->begin(), paths->end());
});
sp->enable_processing();

ASSERT_EQ(recycler.delete_rowset_data(rowset), 0);
ASSERT_EQ(deleted_paths.size(), 1)
<< " size: " << deleted_paths.size() << " content: "
<< std::accumulate(deleted_paths.begin(), deleted_paths.end(), std::string(),
[](const std::string& str, const std::string& it) {
return str.empty() ? it : str + ", " + it;
});
EXPECT_EQ(deleted_paths[0], segment_path(rowset.tablet_id(), rowset.rowset_id_v2(), 0));
}

TEST(RecyclerTest, delete_rowset_data_packed_file_single_rowset) {
auto txn_kv = std::make_shared<MemTxnKv>();
ASSERT_EQ(txn_kv->init(), 0);
Expand Down
Loading