[fix](packed-file) fix issues in packed file recycler checker (apache#59153)

luwei16 · liaoxin01 · commit dd3b3efc9e0c · 2026-01-08T20:30:53.000+08:00
diff --git a/be/src/cloud/config.cpp b/be/src/cloud/config.cpp
@@ -140,8 +140,7 @@ DEFINE_mInt64(packed_file_time_threshold_ms, "100");        // 100ms
 DEFINE_mInt64(packed_file_try_lock_timeout_ms, "5");        // 5ms
 DEFINE_mInt64(packed_file_small_file_count_threshold, "100");
 DEFINE_mInt64(small_file_threshold_bytes, "1048576");      // 1MB
-DEFINE_mInt64(uploaded_file_retention_seconds, "60");      // 1 minute
-DEFINE_mInt64(index_retention_seconds, "60");              // 1 minute
+DEFINE_mInt64(uploaded_file_retention_seconds, "1800");    // 1 minute
 DEFINE_mInt64(packed_file_cleanup_interval_seconds, "60"); // 1 minute
 
 DEFINE_mBool(enable_standby_passive_compaction, "true");
diff --git a/be/src/cloud/config.h b/be/src/cloud/config.h
@@ -184,7 +184,6 @@ DECLARE_mInt64(packed_file_try_lock_timeout_ms);
 DECLARE_mInt64(packed_file_small_file_count_threshold);
 DECLARE_mInt64(small_file_threshold_bytes);
 DECLARE_mInt64(uploaded_file_retention_seconds);
-DECLARE_mInt64(index_retention_seconds);
 DECLARE_mInt64(packed_file_cleanup_interval_seconds);
 
 DECLARE_mBool(enable_standby_passive_compaction);
diff --git a/be/src/io/fs/packed_file_manager.cpp b/be/src/io/fs/packed_file_manager.cpp
@@ -85,24 +85,24 @@ Status append_packed_info_trailer(FileWriter* writer, const std::string& packed_
                                      packed_file_path);
     }
 
-    cloud::PackedFileDebugInfoPB debug_pb;
-    debug_pb.mutable_packed_file_info()->CopyFrom(packed_file_info);
+    cloud::PackedFileFooterPB footer_pb;
+    footer_pb.mutable_packed_file_info()->CopyFrom(packed_file_info);
 
-    std::string serialized_debug_info;
-    if (!debug_pb.SerializeToString(&serialized_debug_info)) {
-        return Status::InternalError("Failed to serialize packed file debug info for {}",
+    std::string serialized_footer;
+    if (!footer_pb.SerializeToString(&serialized_footer)) {
+        return Status::InternalError("Failed to serialize packed file footer info for {}",
                                      packed_file_path);
     }
 
-    if (serialized_debug_info.size() >
+    if (serialized_footer.size() >
         std::numeric_limits<uint32_t>::max() - kPackedFileTrailerSuffixSize) {
-        return Status::InternalError("PackedFileDebugInfoPB too large for {}", packed_file_path);
+        return Status::InternalError("PackedFileFooterPB too large for {}", packed_file_path);
     }
 
     std::string trailer;
-    trailer.reserve(serialized_debug_info.size() + kPackedFileTrailerSuffixSize);
-    trailer.append(serialized_debug_info);
-    put_fixed32_le(&trailer, static_cast<uint32_t>(serialized_debug_info.size()));
+    trailer.reserve(serialized_footer.size() + kPackedFileTrailerSuffixSize);
+    trailer.append(serialized_footer);
+    put_fixed32_le(&trailer, static_cast<uint32_t>(serialized_footer.size()));
     put_fixed32_le(&trailer, kPackedFileTrailerVersion);
 
     return writer->append(Slice(trailer));
diff --git a/be/src/io/fs/packed_file_trailer.cpp b/be/src/io/fs/packed_file_trailer.cpp
@@ -25,7 +25,7 @@
 
 namespace doris::io {
 
-Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileDebugInfoPB* debug_pb,
+Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileFooterPB* debug_pb,
                                  uint32_t* version) {
     if (debug_pb == nullptr || version == nullptr) {
         return Status::InvalidArgument("Output parameters must not be null");
@@ -39,14 +39,14 @@ Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileDebugIn
     const uint32_t trailer_size = decode_fixed32_le(suffix_ptr);
     const uint32_t trailer_version = decode_fixed32_le(suffix_ptr + sizeof(uint32_t));
 
-    // Preferred format: [PackedFileDebugInfoPB][length][version]
+    // Preferred format: [PackedFileFooterPB][length][version]
     if (trailer_size > 0 && trailer_size <= data.size() - kPackedFileTrailerSuffixSize) {
         const size_t payload_offset = data.size() - kPackedFileTrailerSuffixSize - trailer_size;
         std::string_view payload(data.data() + payload_offset, trailer_size);
         if (payload.size() > static_cast<size_t>(std::numeric_limits<int>::max())) {
             return Status::InternalError("Packed file trailer payload too large");
         }
-        cloud::PackedFileDebugInfoPB parsed_pb;
+        cloud::PackedFileFooterPB parsed_pb;
         if (parsed_pb.ParseFromArray(payload.data(), static_cast<int>(payload.size()))) {
             debug_pb->Swap(&parsed_pb);
             *version = trailer_version;
@@ -80,8 +80,8 @@ Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileDebugIn
     return Status::OK();
 }
 
-Status read_packed_file_trailer(const std::string& file_path,
-                                cloud::PackedFileDebugInfoPB* debug_pb, uint32_t* version) {
+Status read_packed_file_trailer(const std::string& file_path, cloud::PackedFileFooterPB* debug_pb,
+                                uint32_t* version) {
     if (debug_pb == nullptr || version == nullptr) {
         return Status::InvalidArgument("Output parameters must not be null");
     }
diff --git a/be/src/io/fs/packed_file_trailer.h b/be/src/io/fs/packed_file_trailer.h
@@ -29,10 +29,10 @@ namespace doris::io {
 constexpr uint32_t kPackedFileTrailerVersion = 1;
 constexpr size_t kPackedFileTrailerSuffixSize = sizeof(uint32_t) * 2;
 
-Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileDebugInfoPB* debug_pb,
+Status parse_packed_file_trailer(std::string_view data, cloud::PackedFileFooterPB* debug_pb,
                                  uint32_t* version);
 
-Status read_packed_file_trailer(const std::string& file_path,
-                                cloud::PackedFileDebugInfoPB* debug_pb, uint32_t* version);
+Status read_packed_file_trailer(const std::string& file_path, cloud::PackedFileFooterPB* debug_pb,
+                                uint32_t* version);
 
 } // namespace doris::io
diff --git a/be/src/tools/packed_file_tool.cpp b/be/src/tools/packed_file_tool.cpp
@@ -38,7 +38,7 @@ int main(int argc, char** argv) {
         return -1;
     }
 
-    doris::cloud::PackedFileDebugInfoPB debug_info;
+    doris::cloud::PackedFileFooterPB debug_info;
     uint32_t version = 0;
     doris::Status st = doris::io::read_packed_file_trailer(FLAGS_file, &debug_info, &version);
     if (!st.ok()) {
diff --git a/be/test/io/fs/packed_file_manager_test.cpp b/be/test/io/fs/packed_file_manager_test.cpp
@@ -608,14 +608,14 @@ TEST_F(PackedFileManagerTest, AppendPackedFileInfoToFileTail) {
     ASSERT_NE(writer, nullptr);
 
     const auto& data = writer->written_data();
-    cloud::PackedFileDebugInfoPB parsed_debug;
+    cloud::PackedFileFooterPB parsed_footer;
     uint32_t version = 0;
-    auto st = parse_packed_file_trailer(data, &parsed_debug, &version);
+    auto st = parse_packed_file_trailer(data, &parsed_footer, &version);
     ASSERT_TRUE(st.ok()) << st;
     ASSERT_EQ(version, kPackedFileTrailerVersion);
-    ASSERT_TRUE(parsed_debug.has_packed_file_info());
+    ASSERT_TRUE(parsed_footer.has_packed_file_info());
 
-    const auto& parsed_info = parsed_debug.packed_file_info();
+    const auto& parsed_info = parsed_footer.packed_file_info();
     ASSERT_EQ(parsed_info.slices_size(), 1);
     EXPECT_EQ(parsed_info.slices(0).path(), "trailer_path");
     EXPECT_EQ(parsed_info.slices(0).offset(), 0);
diff --git a/be/test/io/packed_file_trailer_test.cpp b/be/test/io/packed_file_trailer_test.cpp
@@ -57,21 +57,21 @@ TEST(PackedFileTrailerTest, ReadNewFormatTrailer) {
     slice->set_offset(10);
     slice->set_size(20);
 
-    cloud::PackedFileDebugInfoPB debug_pb;
-    debug_pb.mutable_packed_file_info()->CopyFrom(info);
+    cloud::PackedFileFooterPB footer_pb;
+    footer_pb.mutable_packed_file_info()->CopyFrom(info);
 
-    std::string serialized_debug;
-    ASSERT_TRUE(debug_pb.SerializeToString(&serialized_debug));
+    std::string serialized_footer;
+    ASSERT_TRUE(footer_pb.SerializeToString(&serialized_footer));
 
     std::string file_content = "data";
-    file_content.append(serialized_debug);
-    put_fixed32_le(&file_content, static_cast<uint32_t>(serialized_debug.size()));
+    file_content.append(serialized_footer);
+    put_fixed32_le(&file_content, static_cast<uint32_t>(serialized_footer.size()));
     put_fixed32_le(&file_content, kPackedFileTrailerVersion);
 
     auto path = unique_temp_file();
     write_file(path, file_content);
 
-    cloud::PackedFileDebugInfoPB parsed;
+    cloud::PackedFileFooterPB parsed;
     uint32_t version = 0;
     Status st = read_packed_file_trailer(path, &parsed, &version);
     ASSERT_TRUE(st.ok()) << st;
@@ -101,7 +101,7 @@ TEST(PackedFileTrailerTest, ReadLegacyTrailer) {
     auto path = unique_temp_file();
     write_file(path, file_content);
 
-    cloud::PackedFileDebugInfoPB parsed;
+    cloud::PackedFileFooterPB parsed;
     uint32_t version = 0;
     Status st = read_packed_file_trailer(path, &parsed, &version);
     ASSERT_TRUE(st.ok()) << st;
diff --git a/cloud/src/common/config.h b/cloud/src/common/config.h
@@ -120,6 +120,7 @@ CONF_mInt32(packed_file_txn_retry_times, "10");
 // randomized interval to reduce conflict storms in FoundationDB, default 5-50ms
 CONF_mInt64(packed_file_txn_retry_sleep_min_ms, "5");
 CONF_mInt64(packed_file_txn_retry_sleep_max_ms, "50");
+CONF_mInt32(recycle_txn_delete_max_retry_times, "10");
 
 // force recycler to recycle all useless object.
 // **just for TEST**
diff --git a/cloud/src/recycler/checker.cpp b/cloud/src/recycler/checker.cpp
@@ -991,21 +991,25 @@ int InstanceChecker::do_inverted_check() {
         }
 
         for (auto file = list_iter->next(); file.has_value(); file = list_iter->next()) {
+            const auto& path = file->path;
+            if (path == "data/packed_file" || path.starts_with("data/packed_file/")) {
+                continue; // packed_file has dedicated check logic
+            }
             ++num_scanned;
-            int ret = check_segment_file(file->path);
+            int ret = check_segment_file(path);
             if (ret != 0) {
                 LOG(WARNING) << "failed to check segment file, uri=" << accessor->uri()
-                             << " path=" << file->path;
+                             << " path=" << path;
                 if (ret == 1) {
                     ++num_file_leak;
                 } else {
                     check_ret = -1;
                 }
             }
-            ret = check_inverted_index_file(file->path);
+            ret = check_inverted_index_file(path);
             if (ret != 0) {
                 LOG(WARNING) << "failed to check index file, uri=" << accessor->uri()
-                             << " path=" << file->path;
+                             << " path=" << path;
                 if (ret == 1) {
                     ++num_file_leak;
                 } else {
@@ -2749,6 +2753,18 @@ int InstanceChecker::do_packed_file_check() {
 
     // Step 1: Scan all rowset metas to collect packed_slice_locations references
     // Use efficient range scan instead of iterating through each tablet_id
+    auto collect_packed_refs = [&](const doris::RowsetMetaCloudPB& rs_meta) {
+        const auto& index_map = rs_meta.packed_slice_locations();
+        for (const auto& [small_file_path, index_pb] : index_map) {
+            if (!index_pb.has_packed_file_path() || index_pb.packed_file_path().empty()) {
+                continue;
+            }
+            const std::string& packed_file_path = index_pb.packed_file_path();
+            expected_ref_counts[packed_file_path]++;
+            packed_file_small_files[packed_file_path].insert(small_file_path);
+        }
+    };
+
     {
         std::string start_key = meta_rowset_key({instance_id_, 0, 0});
         std::string end_key = meta_rowset_key({instance_id_, INT64_MAX, 0});
@@ -2788,16 +2804,57 @@ int InstanceChecker::do_packed_file_check() {
 
                 num_scanned_rowsets++;
 
-                // Check packed_slice_locations in rowset meta
-                const auto& index_map = rs_meta.packed_slice_locations();
-                for (const auto& [small_file_path, index_pb] : index_map) {
-                    if (!index_pb.has_packed_file_path() || index_pb.packed_file_path().empty()) {
-                        continue;
-                    }
-                    const std::string& packed_file_path = index_pb.packed_file_path();
-                    expected_ref_counts[packed_file_path]++;
-                    packed_file_small_files[packed_file_path].insert(small_file_path);
+                collect_packed_refs(rs_meta);
+            }
+            start_key.push_back('\x00'); // Update to next smallest key for iteration
+        } while (it->more() && !stopped());
+    }
+
+    // Rowsets in recycle keys may still hold packed file references while ref count
+    // updates are pending, so include them when calculating expected references.
+    {
+        std::string start_key = recycle_rowset_key({instance_id_, 0, ""});
+        std::string end_key = recycle_rowset_key({instance_id_, INT64_MAX, "\xff"});
+
+        std::unique_ptr<RangeGetIterator> it;
+        do {
+            if (stopped()) {
+                return -1;
+            }
+
+            std::unique_ptr<Transaction> txn;
+            TxnErrorCode err = txn_kv_->create_txn(&txn);
+            if (err != TxnErrorCode::TXN_OK) {
+                LOG(WARNING) << "failed to create txn for recycle rowset scan in packed file check";
+                return -1;
+            }
+
+            err = txn->get(start_key, end_key, &it);
+            if (err != TxnErrorCode::TXN_OK) {
+                LOG(WARNING) << "failed to scan recycle rowset metas, err=" << err;
+                check_ret = -1;
+                break;
+            }
+
+            while (it->has_next() && !stopped()) {
+                auto [k, v] = it->next();
+                if (!it->has_next()) {
+                    start_key = k;
+                }
+
+                RecycleRowsetPB recycle_rowset;
+                if (!recycle_rowset.ParseFromArray(v.data(), v.size())) {
+                    LOG(WARNING) << "malformed recycle rowset, key=" << hex(k);
+                    check_ret = -1;
+                    continue;
                 }
+
+                if (!recycle_rowset.has_rowset_meta()) {
+                    continue;
+                }
+
+                num_scanned_rowsets++;
+                collect_packed_refs(recycle_rowset.rowset_meta());
             }
             start_key.push_back('\x00'); // Update to next smallest key for iteration
         } while (it->more() && !stopped());
diff --git a/cloud/src/recycler/recycler.cpp b/cloud/src/recycler/recycler.cpp
@@ -5268,8 +5268,8 @@ int InstanceRecycler::recycle_expired_txn_label() {
             concurrent_delete_executor.add([&]() {
                 int ret = delete_recycle_txn_kv(k);
                 if (ret == 1) {
-                    constexpr int MAX_RETRY = 10;
-                    for (size_t i = 1; i <= MAX_RETRY; ++i) {
+                    const int max_retry = std::max(1, config::recycle_txn_delete_max_retry_times);
+                    for (int i = 1; i <= max_retry; ++i) {
                         LOG(WARNING) << "txn conflict, retry times=" << i << " key=" << hex(k);
                         ret = delete_recycle_txn_kv(k);
                         // clang-format off
diff --git a/cloud/src/recycler/recycler.h b/cloud/src/recycler/recycler.h
@@ -430,7 +430,6 @@ class InstanceRecycler {
 
     // return 0 for success otherwise error
     int decrement_packed_file_ref_counts(const doris::RowsetMetaCloudPB& rs_meta_pb);
-    friend class RecyclerTest_delete_rowset_data_packed_file_respects_recycled_tablet_Test;
 
     int delete_packed_file_and_kv(const std::string& packed_file_path,
                                   const std::string& packed_key,
diff --git a/gensrc/proto/cloud.proto b/gensrc/proto/cloud.proto
@@ -2093,9 +2093,9 @@ message PackedFileInfoPB {
     optional string resource_id = 9;
 }
 
-// Wrapper for packed file debug information. It keeps PackedFileInfoPB extensible for tools
+// Wrapper for packed file footer information. It keeps PackedFileInfoPB extensible for tools
 // reading packed file trailers.
-message PackedFileDebugInfoPB {
+message PackedFileFooterPB {
     optional PackedFileInfoPB packed_file_info = 1;
 }
 
diff --git a/regression-test/suites/cloud_p0/recycler/test_recycler.groovy b/regression-test/suites/cloud_p0/recycler/test_recycler.groovy
@@ -51,6 +51,8 @@ suite("test_recycler") {
         }
     }
 
+    sql """ drop table IF EXISTS __internal_schema.audit_log force """
+
     do {
         triggerRecycle(token, instanceId)
         Thread.sleep(10000)

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ int main(int argc, char** argv) {`
`38`	`38`	`return -1;`
`39`	`39`	`}`
`40`	`40`
`41`		`- doris::cloud::PackedFileDebugInfoPB debug_info;`
	`41`	`+ doris::cloud::PackedFileFooterPB debug_info;`
`42`	`42`	`uint32_t version = 0;`
`43`	`43`	`doris::Status st = doris::io::read_packed_file_trailer(FLAGS_file, &debug_info, &version);`
`44`	`44`	`if (!st.ok()) {`