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 <[email protected]>
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 <[email protected]>
Co-authored-by: 纪华裕 <[email protected]>
---
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
