(kvrocks) branch unstable updated: fix(storage): correct CRC verification for multi-chunk replication files (#3660)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

git-hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new f507a6338 fix(storage): correct CRC verification for multi-chunk replication files (#3660)
f507a6338 is described below

commit f507a6338869f56c2215d6a58b1a5af83faf1f39
Author: weimeng <we...@163.com>
AuthorDate: Sat Oct 10 19:09:22 2026 +0800

    fix(storage): correct CRC verification for multi-chunk replication files (#3660)
    
    Fix a bug in `ReplDataManager::FileExists` where CRC verification only
    checked
    the last chunk of a file instead of the entire file content.
    
    ## Root Cause
    
    In `src/storage/storage.cc`, `rocksdb::crc32c::Extend` was called with
    `0` as
    the initial CRC value inside the read loop:
    
    ```cpp
    tmp_crc = rocksdb::crc32c::Extend(0, slice.data(), slice.size());
    ```
    
    ## AI Usage Disclosure
    - The bug fix was implemented manually.
    - AI tools(Qoder) were used to assist in generating unit test
    scaffolding; all tests were reviewed and verified by the author.
    
    ---------
    
    Co-authored-by: weimeng <we...@didiglobal.com>
    Co-authored-by: 纪华裕 <ji...@gmail.com>
---
 src/storage/storage.cc        |  2 +-
 tests/cppunit/storage_test.cc | 79 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/src/storage/storage.cc b/src/storage/storage.cc
index f328b04f3..c79ed972a 100644
--- a/src/storage/storage.cc
+++ b/src/storage/storage.cc
@@ -1474,7 +1474,7 @@ bool Storage::ReplDataManager::FileExists(Storage *storage, const std::string &d
 
     if (slice.size() == 0) return false;
 
-    tmp_crc = rocksdb::crc32c::Extend(0, slice.data(), slice.size());
+    tmp_crc = rocksdb::crc32c::Extend(tmp_crc, slice.data(), slice.size());
     size -= slice.size();
   }
 
diff --git a/tests/cppunit/storage_test.cc b/tests/cppunit/storage_test.cc
index 0c6e60ff5..881f8418b 100644
--- a/tests/cppunit/storage_test.cc
+++ b/tests/cppunit/storage_test.cc
@@ -18,6 +18,7 @@
  *
  */
 
+#include <common/rocksdb_crc32c.h>
 #include <config/config.h>
 #include <gtest/gtest.h>
 #include <status.h>
@@ -257,6 +258,84 @@ TEST(Storage, TryPurgeCheckpoint) {
   ASSERT_TRUE(s.IsOK()) << s.Msg();
   EXPECT_TRUE(storage->ExistCheckpoint());
 
+  std::filesystem::remove_all(test_dir, ec);
+  ASSERT_FALSE(ec);
+}
+
+TEST(Storage, ReplDataManagerFileExistsCRC) {
+  std::error_code ec;
+  const std::string test_dir = "test_repl_crc_check";
+
+  Config config;
+  config.db_dir = test_dir + "/db";
+  config.checkpoint_dir = test_dir + "/checkpoint";
+  config.slot_id_encoded = false;
+
+  std::filesystem::remove_all(test_dir, ec);
+  ASSERT_FALSE(ec);
+  std::filesystem::create_directories(test_dir + "/sync", ec);
+  ASSERT_FALSE(ec);
+
+  auto storage = std::make_unique<engine::Storage>(&config);
+  auto s = storage->Open();
+  ASSERT_TRUE(s.IsOK()) << s.Msg();
+
+  auto write_file_and_get_crc = [&](const std::string &file_name, size_t file_size) -> uint32_t {
+    std::string path = test_dir + "/sync/" + file_name;
+    std::string content(file_size, '\0');
+    for (size_t i = 0; i < file_size; i++) {
+      content[i] = static_cast<char>(i & 0xFF);
+    }
+    std::ofstream ofs(path, std::ios::binary);
+    ofs.write(content.data(), static_cast<std::streamsize>(content.size()));
+    ofs.close();
+
+    uint32_t crc = 0;
+    crc = rocksdb::crc32c::Extend(crc, content.data(), content.size());
+    return crc;
+  };
+
+  // Small file (<= 4096 bytes, single chunk)
+  {
+    std::string file_name = "small_file";
+    uint32_t crc = write_file_and_get_crc(file_name, 1024);
+    EXPECT_TRUE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, crc));
+  }
+
+  // Large file (> 4096 bytes, multiple chunks) — this is the case the bug affected
+  {
+    std::string file_name = "large_file";
+    uint32_t crc = write_file_and_get_crc(file_name, 16384);
+    EXPECT_TRUE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, crc));
+  }
+
+  // File size exactly at chunk boundary
+  {
+    std::string file_name = "boundary_file";
+    uint32_t crc = write_file_and_get_crc(file_name, 4096);
+    EXPECT_TRUE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, crc));
+  }
+
+  // File size just past chunk boundary
+  {
+    std::string file_name = "just_over_file";
+    uint32_t crc = write_file_and_get_crc(file_name, 4097);
+    EXPECT_TRUE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, crc));
+  }
+
+  // Wrong CRC should return false
+  {
+    std::string file_name = "large_file";
+    uint32_t wrong_crc = 0xDEADBEEF;
+    EXPECT_FALSE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, wrong_crc));
+  }
+
+  // CRC of 0 should skip verification and return true
+  {
+    std::string file_name = "large_file";
+    EXPECT_TRUE(engine::Storage::ReplDataManager::FileExists(storage.get(), test_dir + "/sync", file_name, 0));
+  }
+
   std::filesystem::remove_all(test_dir, ec);
   ASSERT_FALSE(ec);
 }
\ No newline at end of file