Add fast path optimization for multi-threaded writes#209
Add fast path optimization for multi-threaded writes#209genezhang wants to merge 4 commits intoeBay:masterfrom
Conversation
- Skip writeMutex for common writes (auto-assigned seqnum, no overwrite) - Thread safety via atomic CAS and lock-free skiplist - ~50-100% write throughput improvement - Add multi-threaded stress test suite
greensky00
left a comment
There was a problem hiding this comment.
Thanks @genezhang for submitting this PR.
The failure of many_logs_test is caused by this change, especially because of ref count leak. Please see my detailed comments.
src/log_mgr.cc
Outdated
| } | ||
|
|
||
| // Fast path: if log file is valid and writable, write directly | ||
| if (lf_info && !lf_info->isRemoved() && lf_info->file->isValidToWrite()) { |
There was a problem hiding this comment.
There is a potential ref count leak: if lf_info is not null, but other two conditions are not met, we don't enter this if-clause and nobody will call lf_info->done().
The correct code should be as follows:
s = mani->getMaxLogFileNum(max_log_file_num);
if (s) {
lf_info = mani->getLogFileInfoP(max_log_file_num);
LogFileInfoGuard g_li(lf_info);
// Fast path: if log file is valid and writable, write directly
if (lf_info && !lf_info->isRemoved() && lf_info->file->isValidToWrite()) {|
|
||
| unit_test(NAME mt_test SOURCES ${TEST_DIR}/jungle/mt_test.cc) | ||
|
|
||
| unit_test(NAME mt_write_stress_test SOURCES ${TEST_DIR}/jungle/mt_write_stress_test.cc) |
There was a problem hiding this comment.
Please add this to runtest.sh and also code coverage list
Lines 252 to 281 in 9f2ad0f
src/log_mgr.cc
Outdated
| if ( !valid_number(rec.seqNum) && | ||
| !getDbConfig()->allowOverwriteSeqNum ) { |
There was a problem hiding this comment.
To be safe against any potential issue, please add an option bool allowConcurrentWrites to DBConfig and set the default value to false, and add a condition here.
- Add FAST_PATH_OPTIMIZATION CMake option (default OFF) - Guard fast path code with #ifdef FAST_PATH_OPTIMIZATION - Fix potential refCount leak by taking LogFileInfoGuard before checking validity - Update PR description to document the build flag
- Add DBConfig.allowConcurrentWrites flag (default false) - Fix reference count leak in fast path - Remove build-time FAST_PATH_OPTIMIZATION option - Update documentation with runtime usage
Fast Path Optimization for Multi-threaded Writes
(note: worked with Copilot based on performance benchmarking)
Summary
This PR adds a fast path optimization to
LogMgr::setSN()that significantly improves multi-threaded write throughput by skipping the heavywriteMutexfor common write operations.Performance Improvement
Benchmarks show 50-135% improvement in write throughput:
Tested on Intel Core i5-1035G1 @ 1.00GHz, 8GB RAM, Linux
How It Works
The fast path bypasses the mutex when all these conditions are met:
allowOverwriteSeqNum = false)Thread Safety Guarantees
The optimization is safe because:
MemTable::assignSeqNum()skiplist_insert)LogFileInfoGuardrefcounting prevents removal during writeCode Flow
Changes
src/log_mgr.cc: Added fast path optimization insetSN()(~35 lines)tests/jungle/mt_write_stress_test.cc: New multi-threaded write stress testtests/CMakeLists.txt: Added new test targetTesting
New Tests Added
mt write stress (2/4/8/16 threads)- Concurrent writes with verificationmt write read interleaved- Mixed read/write workloadmt write with flush- Writes with background flushermt write reopen- Durability test (write, close, reopen, verify)Existing Tests
All existing tests pass:
mt_test✅sync_and_flush_test✅basic_op_test(43/44 pass - 1 pre-existing failure)flush_stress_test✅log_reclaim_stress_test✅Backward Compatibility
Notes
This optimization is most beneficial for write-heavy workloads where:
allowOverwriteSeqNumis disabled (default)Workloads that specify sequence numbers or use overwrite mode will not benefit but will not be negatively affected.