Files
rocksdb/test_util/sync_point_impl.cc
Xingbo Wang ec31d0074f Accelerate build and test infrastructure (#14478)
Summary:
Reduce `make check` time from ~9.4 minutes to ~3.7 minutes (2.6x) on a 192-core machine, speed up CI by 1.8x across all jobs, and fix ccache hit rates (macOS cmake: 0.51% → 99.80%).

## Changes

### 1. Auto-detect lld linker (`build_detect_platform`)
- Probe for `lld` at configure time; fall back to default `ld.bfd` if unavailable
- Linux-only guard (`TARGET_OS = Linux`)
- **Measured: 12x faster linking** (7.4s → 0.6s per test binary)
- Add `-L/usr/local/lib` for lld library resolution on CI
- Install lld in CI pre-steps action
- Opt out with `ROCKSDB_NO_FAST_LINKER=1`

### 2. Sharded test execution (`Makefile`)

**Before:** `make check` enumerated every individual gtest case via `--gtest_list_tests`, then created a separate shell script for each case with `--gtest_filter=TestName`. For example, `block_based_table_reader_test` has 9,788 parameterized test cases — each spawned its own process, loading a ~50MB binary, initializing gtest, registering all ~10K tests, running exactly ONE, then tearing down. The per-process overhead was ~1.5s, so 9,788 × 1.5s ≈ 15,000s wasted on overhead alone. Across all 302 test binaries this produced 39,467 individual processes.

**After:** Use gtest's built-in sharding (`GTEST_TOTAL_SHARDS`/`GTEST_SHARD_INDEX`) to group ~10 test cases per process. Each shard loads the binary once and runs multiple tests sequentially — eliminating the per-process overhead. The 9,788 cases in `block_based_table_reader_test` become ~980 shards. Total process count drops from 39,467 to ~4,036. Same tests, same coverage, just fewer process spawns.

The shard count adapts to machine size: `min(ceil(test_count / GTEST_SHARD_SIZE), NCORES * 8)`. This ensures large machines (192 cores) get many small shards for parallelism, while small CI runners (4 cores) get fewer larger shards to avoid excessive overhead. Both `GTEST_SHARD_SIZE` (default 10) and `NCORES` can be overridden.

CI sharding uses round-robin distribution across 3 shards to balance heavy tests (db_test, db_compaction_test, etc.) instead of contiguous alphabetical ranges.

Note: gtest continues running all tests after a failure (`GTEST_THROW_ON_FAILURE=0`), so grouping tests does not mask failures — all failures within a shard are reported.

**Measured: 3.6x CPU reduction** (76,121s → 21,033s)

### 3. Fix SyncPoint leaks for test isolation (5 files)
Sharded execution exposed 43 test files that set SyncPoint callbacks but never clean them up. Stale callbacks with captured local variables cause segfaults or data corruption when subsequent tests in the same process trigger them.

**Systematic fix:** Global gtest `TestEventListener` in `testharness.cc` that calls `SyncPoint::DisableProcessing()` + `ClearAllCallBacks()` + `ClearTrace()` + `LoadDependency({})` after every test case. This cleans up all SyncPoint state: callbacks, dependency maps, cleared points, and the point filter. Registered via static initialization — no changes needed to individual test `main()` functions.

**SyncPoint infrastructure fixes:**
- `DisableProcessing()` now calls `cv_.notify_all()` to wake threads blocked in `Process()`. Previously, threads waiting for predecessor sync points that would never fire (because the test ended) would hang forever.
- `Process()` now rechecks `enabled_` after waking from `cv_.wait()`, so threads exit promptly when processing is disabled instead of looping forever.

**Specific fixes (defense-in-depth):**
- `SstFileReaderTest::VerifyNumEntriesCorruption` — leaked `PropertyBlockBuilder::AddTableProperty:Start` callback that corrupted SST entry counts
- `WritePreparedTransactionTest::CommitAndSnapshotDuringCompaction` — leaked `CompactionIterator:AfterInit` callback causing segfault via dangling pointers
- `TransactionTestBase` destructor — cleanup for 26 SyncPoint uses across transaction tests
- `RetriableLogTest` destructor — cleanup for log reader SyncPoint callbacks

### 4. Fix ccache for reliable cross-run caching (`setup-ccache`, `CMakeLists.txt`)
- **`CCACHE_COMPILERCHECK=content`**: Default `mtime` compared compiler binary modification time, which differs on every fresh CI runner → 0% hit rate. `content` hashes compiler output instead — stable across runner instances
- **`CCACHE_SLOPPINESS`**: CMake generates varying `-MF` paths and timestamps. Without sloppiness settings, ccache treated these as different compilations
- **`CMAKE_C/CXX_COMPILER_LAUNCHER`** instead of `RULE_LAUNCH_COMPILE`: Avoids double-wrapping ccache when also injected via PATH. Removes `RULE_LAUNCH_LINK` since ccache cannot cache link operations
- **macOS cmake hit rate: 0.51% → 99.80%**

### 5. Align GTEST_THROW_ON_FAILURE (`Makefile`)
- Set `GTEST_THROW_ON_FAILURE=0` in Makefile default to match CI's `pre-steps/action.yml`
- `=1` caused `std::terminate` in multi-threaded stress tests (e.g., `point_lock_manager_stress_test`, `rate_limiter_test`) when assertions fail — the gtest exception propagates across thread boundaries, triggering undefined behavior and heap-use-after-free under ASAN

### 6. Portability fixes
- `[[maybe_unused]]` instead of `__attribute__((unused))` for MSVC compatibility
- Remove blanket `-fPIC` from `COMMON_FLAGS` — only apply via `PLATFORM_SHARED_CFLAGS` for shared builds, preserving optimal codegen for release/static builds
- `make -s list_all_tests` to suppress make noise in test enumeration

## Results

### Local (192-core devvm, clean build)

| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| Build time (clean) | 2m43s | ~2m | lld linker |
| Test wall clock | 400s | 200s | 2.0x |
| Test CPU total | 76,121s | 21,033s | 3.6x |
| Test failures | 0 | 0 | — |
| **Total make check** | **~9.4min** | **3m41s** | **2.6x** |

### CI (GitHub Actions)

| Metric | Before | After | Improvement |
|--------|--------|-------|-------------|
| Aggregate CI time | 15,220s | 8,425s | 1.8x |
| build-linux-arm | 763s | 221s | 3.5x |
| build-linux-release | 616s | 244s | 2.5x |
| build-windows-vs2022 | 2,868s | 1,205s | 2.4x |
| build-linux-java-static | 858s | 335s | 2.6x |

### ccache hit rates (all jobs)

| Job | Before | After |
|-----|--------|-------|
| macOS cmake (0-3) | 0.51% | **99.80%** |
| build-linux | 99.68% | **99.84%** |
| build-linux-clang18-asan-ubsan | 98.58% | **99.84%** |
| build-linux-clang18-mini-tsan | 98.74% | **99.84%** |
| build-windows-vs2022 | 99.22% | **99.83%** |
| All 27 ccache jobs | — | **>94%** |

## Remaining bottleneck

The critical path is now dominated by genuinely slow integration tests:
- `external_sst_file_test` shards: 97–113s each
- `db_test` shards: 109–124s each
- `db_bloom_filter_test`: 103s (non-parallel)

Further improvement would require test optimization or a `make check-fast` target.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/14478

Test Plan:
- `make check -j192 SKIP_FORMAT_BUCK_CHECKS=1` — all 4,045 shards pass (ASAN+UBSAN), zero failures
- Verified SyncPoint fixes: each crash/corruption reproduces before fix, passes after
- Verified SyncPoint::DisableProcessing wakes blocked threads (previously caused indefinite hangs)
- Verified gtest does NOT stop after failure with `GTEST_THROW_ON_FAILURE=0` — all tests in a shard run and all failures are reported
- CI: all 34 checks pass across two consecutive runs
- ccache: validated hit rates >94% on second run after cache seeding

Reviewed By: hx235

Differential Revision: D97623305

Pulled By: xingbowang

fbshipit-source-id: b299e6d43c8713a9ef2b5659e8bbd74958afe155
2026-03-24 15:39:27 -07:00

159 lines
4.6 KiB
C++

// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
// This source code is licensed under both the GPLv2 (found in the
// COPYING file in the root directory) and Apache 2.0 License
// (found in the LICENSE.Apache file in the root directory).
#include "test_util/sync_point_impl.h"
#ifndef NDEBUG
namespace ROCKSDB_NAMESPACE {
KillPoint* KillPoint::GetInstance() {
static KillPoint kp;
return &kp;
}
void KillPoint::TestKillRandom(std::string kill_point, int odds_weight,
const std::string& srcfile, int srcline) {
if (rocksdb_kill_odds <= 0) {
return;
}
int odds = rocksdb_kill_odds * odds_weight;
for (auto& p : rocksdb_kill_exclude_prefixes) {
if (kill_point.substr(0, p.length()) == p) {
return;
}
}
assert(odds > 0);
if (odds % 7 == 0) {
// class Random uses multiplier 16807, which is 7^5. If odds are
// multiplier of 7, there might be limited values generated.
odds++;
}
auto* r = Random::GetTLSInstance();
bool crash = r->OneIn(odds);
if (crash) {
port::Crash(srcfile, srcline);
}
}
void SyncPoint::Data::LoadDependency(
const std::vector<SyncPointPair>& dependencies) {
std::lock_guard<std::mutex> lock(mutex_);
successors_.clear();
predecessors_.clear();
cleared_points_.clear();
for (const auto& dependency : dependencies) {
successors_[dependency.predecessor].push_back(dependency.successor);
predecessors_[dependency.successor].push_back(dependency.predecessor);
point_filter_.Add(dependency.successor);
point_filter_.Add(dependency.predecessor);
}
cv_.notify_all();
}
void SyncPoint::Data::LoadDependencyAndMarkers(
const std::vector<SyncPointPair>& dependencies,
const std::vector<SyncPointPair>& markers) {
std::lock_guard<std::mutex> lock(mutex_);
successors_.clear();
predecessors_.clear();
cleared_points_.clear();
markers_.clear();
marked_thread_id_.clear();
for (const auto& dependency : dependencies) {
successors_[dependency.predecessor].push_back(dependency.successor);
predecessors_[dependency.successor].push_back(dependency.predecessor);
point_filter_.Add(dependency.successor);
point_filter_.Add(dependency.predecessor);
}
for (const auto& marker : markers) {
successors_[marker.predecessor].push_back(marker.successor);
predecessors_[marker.successor].push_back(marker.predecessor);
markers_[marker.predecessor].push_back(marker.successor);
point_filter_.Add(marker.predecessor);
point_filter_.Add(marker.successor);
}
cv_.notify_all();
}
bool SyncPoint::Data::PredecessorsAllCleared(const std::string& point) {
for (const auto& pred : predecessors_[point]) {
if (cleared_points_.count(pred) == 0) {
return false;
}
}
return true;
}
void SyncPoint::Data::ClearCallBack(const std::string& point) {
std::unique_lock<std::mutex> lock(mutex_);
while (num_callbacks_running_ > 0) {
cv_.wait(lock);
}
callbacks_.erase(point);
}
void SyncPoint::Data::ClearAllCallBacks() {
std::unique_lock<std::mutex> lock(mutex_);
while (num_callbacks_running_ > 0) {
cv_.wait(lock);
}
callbacks_.clear();
}
void SyncPoint::Data::Process(const Slice& point, void* cb_arg) {
if (!enabled_) {
return;
}
// Use a filter to prevent mutex lock if possible.
if (!point_filter_.MayContain(point)) {
return;
}
// Must convert to std::string for remaining work. Take
// heap hit.
std::string point_string(point.ToString());
std::unique_lock<std::mutex> lock(mutex_);
auto thread_id = std::this_thread::get_id();
auto marker_iter = markers_.find(point_string);
if (marker_iter != markers_.end()) {
for (auto& marked_point : marker_iter->second) {
marked_thread_id_.emplace(marked_point, thread_id);
point_filter_.Add(marked_point);
}
}
if (DisabledByMarker(point_string, thread_id)) {
return;
}
while (!PredecessorsAllCleared(point_string)) {
cv_.wait(lock);
// After waking, recheck if processing was disabled (e.g., by
// DisableProcessing() during test cleanup). Without this check,
// threads can wait forever for predecessors that will never fire.
if (!enabled_) {
return;
}
if (DisabledByMarker(point_string, thread_id)) {
return;
}
}
auto callback_pair = callbacks_.find(point_string);
if (callback_pair != callbacks_.end()) {
num_callbacks_running_++;
mutex_.unlock();
callback_pair->second(cb_arg);
mutex_.lock();
num_callbacks_running_--;
}
cleared_points_.insert(point_string);
cv_.notify_all();
}
} // namespace ROCKSDB_NAMESPACE
#endif