diff --git a/cloud/src/recycler/recycler.cpp b/cloud/src/recycler/recycler.cpp index 0417ddaa2f46c5..e7ea909906b74c 100644 --- a/cloud/src/recycler/recycler.cpp +++ b/cloud/src/recycler/recycler.cpp @@ -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 @@ -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 @@ -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(); @@ -3652,10 +3660,10 @@ 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) @@ -3663,8 +3671,7 @@ int InstanceRecycler::delete_rowset_data( 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)); } diff --git a/cloud/src/recycler/recycler.h b/cloud/src/recycler/recycler.h index ab03460093a837..e04a69e0ec57b1 100644 --- a/cloud/src/recycler/recycler.h +++ b/cloud/src/recycler/recycler.h @@ -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, diff --git a/cloud/test/mock_accessor.h b/cloud/test/mock_accessor.h index 6c187b7ca995cf..0e29c383899ddf 100644 --- a/cloud/test/mock_accessor.h +++ b/cloud/test/mock_accessor.h @@ -151,7 +151,7 @@ inline int MockAccessor::delete_all(int64_t expiration_time) { } inline int MockAccessor::delete_files(const std::vector& 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); diff --git a/cloud/test/recycler_test.cpp b/cloud/test/recycler_test.cpp index 8a657314288128..ffe2401862bb07 100644 --- a/cloud/test/recycler_test.cpp +++ b/cloud/test/recycler_test.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -5571,6 +5572,57 @@ TEST(RecyclerTest, delete_rowset_data) { } } +TEST(RecyclerTest, delete_rowset_data_without_delete_bitmap_meta) { + auto txn_kv = std::make_shared(); + 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(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 deleted_paths; + sp->set_call_back("MockAccessor::delete_files", [&](auto&& args) { + auto* paths = try_any_cast*>(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(); ASSERT_EQ(txn_kv->init(), 0);