Parallel dir entry sync options #12639
Conversation
|
Behavior with the setting set to "1": Behavior when set to -1: |
zwoop
force-pushed
the
BetterDirSync
branch
4 times, most recently
from
d9ed36b to
19221e0
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces parallel directory synchronization for cache stripes to improve performance on multi-drive cache configurations. The implementation replaces the single global CacheSync instance with multiple task-based sync instances, distributing stripe sync operations across ET_TASK threads.
Key changes:
- Added
proxy.config.cache.dir.sync_parallel_tasksconfiguration parameter to control sync parallelism - Refactored from global
cacheDirSyncto per-stripecache_syncpointers with task-based scheduling - Enhanced
CacheSyncdestructor to properly clean up resources
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/records/RecordsConfig.cc | Adds the new sync_parallel_tasks configuration record |
| src/proxy/http/HttpTransact.cc | Adds blank line at file start (formatting change) |
| src/iocore/cache/StripeSM.h | Adds cache_sync pointer to StripeSM and aligns member declarations |
| src/iocore/cache/StripeSM.cc | Updates references from global cacheDirSync to per-stripe cache_sync |
| src/iocore/cache/P_CacheInternal.h | Removes global cacheDirSync declaration and adds cache_config_dir_sync_parallel_tasks extern |
| src/iocore/cache/P_CacheDir.h | Adds destructor, stripe tracking members, and includes to CacheSync struct |
| src/iocore/cache/CacheDir.cc | Implements parallel sync initialization, task distribution logic, and shutdown cleanup |
| src/iocore/cache/Cache.cc | Removes global cacheDirSync and adds configuration loading for new parameter |
| doc/developer-guide/cache-architecture/architecture.en.rst | Documents directory synchronization architecture and configuration |
| doc/admin-guide/files/records.yaml.en.rst | Adds user-facing documentation for the new configuration parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
The idea here is to allow a configurable amount of directory synchronization to disk, where -1 means "as much parallelism as there are drives". This does have the downside of consuming additional memory during the sync phase.
The default is no parallelization still (same behavior as before). This does however move the sync continuation to the ET_TASK threads, even though most of their work is done on the AIO threads (but feels more correct to move these away from ET_NET).