[BugFix] fix cache transfer tasks failure after cache cleared - #6202
Jiang-Jia-Jun merged 19 commits into
Conversation
|
Thanks for your contribution! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6202 +/- ##
==========================================
Coverage ? 68.15%
==========================================
Files ? 391
Lines ? 52389
Branches ? 8177
==========================================
Hits ? 35705
Misses ? 14071
Partials ? 2613
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
这个PR为CacheTransferManager引入了pause/resume机制,旨在修复缓存清除后缓存传输任务失败的问题。当kv_cache_status_signal触发pause时,系统会等待所有正在执行的任务完成后再清除缓存,避免了缓存清除和传输任务之间的竞争条件。
Changes:
- 在CacheTransferManager中添加了pause/resume方法和inflight任务跟踪机制
- 修改了PrefixCacheManager的reset方法,移除了清除缓存前阻塞等待任务结果的限制
- 在EngineCacheQueue中添加了clear_transfer_task和result_queue_empty方法
- 重构了engine_client.py中的update/clear模型权重逻辑,改进了日志和流程控制
- 增强了调试日志输出,便于问题追踪
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| fastdeploy/cache_manager/cache_transfer_manager.py | 实现核心的pause/resume机制,包括inflight任务计数、跨rank同步以及暂停/恢复传输任务的逻辑 |
| fastdeploy/cache_manager/prefix_cache_manager.py | 修改sync_swap_task使用超时机制,在非正常状态下提前退出;增强reset方法以等待inflight任务和结果队列清空;增加错误处理以忽略非正常状态下的异常 |
| fastdeploy/inter_communicator/engine_cache_queue.py | 新增barrier0、pause_barrier和resume_barrier用于跨rank同步;添加clear_transfer_task清空任务队列;添加result_queue_empty检查结果队列状态 |
| fastdeploy/entrypoints/engine_client.py | 重构update_model_weight和clear_load_weight方法,改进控制流程和日志标记,调整prefix tree操作的执行顺序 |
| fastdeploy/engine/common_engine.py | 在clear_data方法中调用cache_task_queue.clear_transfer_task()清空传输任务 |
| fastdeploy/engine/sched/resource_manager_v1.py | 在关键位置添加verbose日志输出,改进错误信息 |
| tests/cache_manager/test_prefix_cache_manager.py | 更新测试以支持新的signal和方法,包括result_queue_empty、wait timeout参数等 |
| @@ -862,22 +894,46 @@ def do_data_transfer(self): | |||
|
|
|||
| while True: | |||
| try: | |||
| if self.rank == 0: | |||
| self.cache_task_is_paused_signal.value[0] = 1 if self.is_paused else 0 | |||
| if self.n_ranks > 1: | |||
| self.cache_task_queue.barrier0.wait() | |||
| if self.rank == 0: | |||
| self.cache_task_queue.barrier0.reset() | |||
|
|
|||
| # Ensure all ranks synchronically do one of the following things: | |||
| # (1) If rank#0 is paused, wait for a short time and check out rank#0 status again; | |||
| # (2) otherwise, all ranks are allowed to pull tasks from cache task queue | |||
| if self.cache_task_is_paused_signal.value[0] == 1: | |||
| # wait for inflight tasks to finish first | |||
| while self.inflight != 0: | |||
| time.sleep(0.1) | |||
| # mark the current rank as not having inflight tasks | |||
| self.cache_task_inflight_signal.value[self.rank] = 0 | |||
| time.sleep(1) | |||
| continue | |||
| else: | |||
| self.cache_task_inflight_signal.value[self.rank] = 1 | |||
|
|
|||
| if self.rank == 0: | |||
| if not self.cache_task_queue.empty(): | |||
| self.cache_task_broadcast_signal.value[0] = 1 | |||
| if self.n_ranks > 1: | |||
| self.cache_task_queue.barrier1.wait() | |||
| if self.rank == 0: | |||
| self.cache_task_queue.barrier1.reset() | |||
|
|
|||
| if self.cache_task_broadcast_signal.value[0] == 1: | |||
| self.inflight += 1 | |||
There was a problem hiding this comment.
The inflight counter (lines 277, 881, 927) is not protected by a lock. Multiple threads could potentially read and modify it concurrently, leading to race conditions. For example, the decrement in line 881 (self.inflight -= 1) is not atomic. In high-concurrency scenarios, this could result in incorrect counts, causing the pause() method to either wait indefinitely or proceed prematurely. Consider using threading.Lock or an atomic counter (e.g., threading.Event or multiprocessing.Value with lock=True) to protect access to this counter.
…Paddle#6202) * [fix] fix cache transfer tasks failure after cache cleared * [fix] fix submit_task * [fix] fix cache manager hang when clearing prefix cache * [fix] fix list_proxy has no clear method * [fix] fix barrier * [fix] add barrier0 * [fix] add cache_task_is_paused_signal * [fix] fix condition * [fix] fix cache transfer sync and delay prefix cache tree clearing * [fix] fix typo * [chore] polish code * [fix] revert only rank0 write kv_cache_status_signal * [fix] fix thread pool and prefix cache manager hang * [fix] add timeout for task_swapping_event * [fix] tolerate prefix cache manager error while prefix tree is cleared * [chore] add more log * [fix] fix test_prefix_cache_manager * [fix] fix prefix_cache_status_signal usage
…Paddle#6202) * [fix] fix cache transfer tasks failure after cache cleared * [fix] fix submit_task * [fix] fix cache manager hang when clearing prefix cache * [fix] fix list_proxy has no clear method * [fix] fix barrier * [fix] add barrier0 * [fix] add cache_task_is_paused_signal * [fix] fix condition * [fix] fix cache transfer sync and delay prefix cache tree clearing * [fix] fix typo * [chore] polish code * [fix] revert only rank0 write kv_cache_status_signal * [fix] fix thread pool and prefix cache manager hang * [fix] add timeout for task_swapping_event * [fix] tolerate prefix cache manager error while prefix tree is cleared * [chore] add more log * [fix] fix test_prefix_cache_manager * [fix] fix prefix_cache_status_signal usage
…Paddle#6202) * [fix] fix cache transfer tasks failure after cache cleared * [fix] fix submit_task * [fix] fix cache manager hang when clearing prefix cache * [fix] fix list_proxy has no clear method * [fix] fix barrier * [fix] add barrier0 * [fix] add cache_task_is_paused_signal * [fix] fix condition * [fix] fix cache transfer sync and delay prefix cache tree clearing * [fix] fix typo * [chore] polish code * [fix] revert only rank0 write kv_cache_status_signal * [fix] fix thread pool and prefix cache manager hang * [fix] add timeout for task_swapping_event * [fix] tolerate prefix cache manager error while prefix tree is cleared * [chore] add more log * [fix] fix test_prefix_cache_manager * [fix] fix prefix_cache_status_signal usage
Motivation
为 CacheTransferManager 引入 pause/resume 机制,由 kv_cache_status_signal 触发 pause:
Modifications
Usage or Command
无
Accuracy Tests
无
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.