fix(replication): prevent full-sync checkpoint race with cron purge (…

[advisedy]

Fixes #3100

Problem

TSan reported an oversized allocation:

ThreadSanitizer: requested allocation size 0x100000000000000
exceeds maximum supported size of 0x10000000000

Root cause:

Before the fix, GetFullReplDataInfo released the lock before calling GetChildren, while Server::cron called rocksdb::DestroyDB on the live checkpoint directory without holding the lock at all. This allowed the following interleaving:

meta:  unlock checkpoint_mu_
purge: DestroyDB starts deleting files
meta:  GetChildren → [] (dir exists but files are already gone)

GetFullReplDataInfo builds the file list by appending entries and then calls files->pop_back(). If the directory is empty, files is an empty string, and pop_back on an empty string is undefined behavior. (https://en.cppreference.com/cpp/string/basic_string/pop_back)

On libstdc++, the common manifestation is a size underflow that produces a very large number. The subsequent files + CRLF then allocates memory based on this corrupted size, triggering the TSan error.

eaxmple:

#include <cstdio>
#include <string>

int main() {
  std::string files;
  files.pop_back();
  auto s = files + "\r\n";
  printf("size=%zu\n", files.size());
  return 0;
}

Running with GCC 11 + ASan on Linux:

AddressSanitizer: requested allocation size 0x100000000000000
(0x100000000001000 after adjustments for alignment, red zones etc.)
exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0x7ffffee8b1e7 in operator new(unsigned long)
        ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x7ffffecf8109 in void
        std::__cxx11::basic_string::_M_construct<char*>(...)
        (/lib/x86_64-linux-gnu/libstdc++.so.6+0x14f109)
    ...
SUMMARY: AddressSanitizer: allocation-size-too-big

The corrupted size (0x100000000000000) and the threshold (0x10000000000) match the TSan report in the issue.

While investigating, we also found that the same race condition, besides the "empty list" extreme case, could cause the replica to receive an incomplete file list (GetChildren returned only a subset of files).

---

Fix

No pop_back on empty list：

In GetFullReplDataInfo, if files is empty, return NotOK immediately instead of calling pop_back.

Introduce TryPurgeCheckpoint — rename then destroy：

Added Storage::TryPurgeCheckpoint, which changes the purge logic to:

1. Under checkpoint_mu_, atomically rename checkpoint_dir to checkpoint_dir.trash via RenameFile;
2. Clear checkpoint_info_;
3. After releasing the lock, call DestroyDB(trash).

This way, the meta listing either sees a complete live directory or no directory at all.

---

Tests

Two new C++ gtests added (storage_test.cc):

• GetFullReplDataInfoRejectsEmptyCheckpoint: Creates a checkpoint → clears the directory → asserts NotOK.
• TryPurgeCheckpointPolicyAndAtomicRemove: Verifies idle purge, rename atomicity, and checkpoint_info_ reset.

[Copilot]

Pull request overview

This PR fixes a replication full-sync race where the server cron could purge a live RocksDB checkpoint directory while replication metadata collection was building the checkpoint file list, leading to undefined behavior (std::string::pop_back() on empty) and oversized allocations reported by TSan.

Changes:

• Add an explicit empty-checkpoint guard in ReplDataManager::GetFullReplDataInfo to avoid pop_back() on an empty file list.
• Introduce Storage::TryPurgeCheckpoint() to purge checkpoints via an under-lock rename to *.trash followed by DestroyDB() outside the lock.
• Switch Server::cron() checkpoint cleanup to use TryPurgeCheckpoint, and add new C++ unit tests covering empty-checkpoint rejection and purge behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
tests/cppunit/storage_test.cc	Adds new gtests for empty checkpoint handling and checkpoint purge behavior.
src/storage/storage.h	Declares the new TryPurgeCheckpoint API and documents its rename-then-destroy behavior.
src/storage/storage.cc	Implements TryPurgeCheckpoint and adds a safety check for empty checkpoint listings in full-sync metadata generation.
src/server/server.cc	Replaces direct DestroyDB(checkpoint_dir) purge logic with Storage::TryPurgeCheckpoint.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[jihuayu]

Hello @advisedy. Thanks for your contribution. If you used an LLM, please follow our guidelines for AI-assisted contributions: https://kvrocks.apache.org/community/contributing#guidelines-for-ai-assisted-contributions
This does not mean that using an LLM is prohibited. It simply helps us assess the reliability of the code.

[advisedy]

Hi @jihuayu, thanks for the reminder. Here's my AI usage:

The main fix logic and code were written by myself.
I used AI for test and code review suggestions.
The additional test suggested by Copilot just now.

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 50%)

See analysis details on SonarQube Cloud

[advisedy]

Hi, @jihuayu . just friendly ping. CI has passed. Could you please take a look when you have time?