Skip to content

fix: propagate TP KV mapping failures#373

Open
shipiyouniao wants to merge 3 commits into
ovg-project:mainfrom
shipiyouniao:fix/tp-ipc-error-propagation
Open

fix: propagate TP KV mapping failures#373
shipiyouniao wants to merge 3 commits into
ovg-project:mainfrom
shipiyouniao:fix/tp-ipc-error-propagation

Conversation

@shipiyouniao

@shipiyouniao shipiyouniao commented Jul 1, 2026

Copy link
Copy Markdown

Summary

Fixes #372.

This PR tightens TP IPC error handling around KV tensor mapping:

  • propagate worker-side map_to_kv_tensors / unmap_from_kv_tensors failures instead of replying success
  • preserve pp_rank and group_id from the KVCacheManager when checking KV tensor creation and when invoking map/unmap callbacks
  • include the last observed KV tensor creation exception in timeout errors
  • synchronize CUDA after TP map and before TP unmap so mapped pages are visible before schedulers touch them, and unmap follows the single-process allocator safety ordering

Why

Without this, the coordinator can lose the real map/unmap failure and later report only a generic KV tensor wait timeout. In TP/PP deployments the default callback context can also make the wait path observe the wrong group. The additional synchronization mirrors the allocator ordering used outside the TP IPC path.

Validation

python -m pytest tests/test_kvcache_manager_post_init.py tests/test_tp_ipc_unmap_sync.py -q
python -m py_compile kvcached/kv_cache_manager.py kvcached/tp_ipc_util.py tests/test_kvcache_manager_post_init.py tests/test_tp_ipc_unmap_sync.py

Copilot AI review requested due to automatic review settings July 1, 2026 02:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses issue #372 by tightening TP IPC error propagation for KV tensor map/unmap operations and improving KV tensor creation diagnostics so the coordinator doesn’t lose the root-cause error behind a generic timeout.

Changes:

  • Propagate worker-side map_to_kv_tensors / unmap_from_kv_tensors failures through the IPC response instead of always returning success.
  • Ensure broadcast map callbacks use the KVCacheManager’s recorded pp_rank / group_id context.
  • Augment KV tensor creation wait timeouts with the last observed exception to improve debuggability.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
kvcached/tp_ipc_util.py Makes worker IPC replies reflect map/unmap failures so callers receive the real error.
kvcached/kv_cache_manager.py Fixes callback context usage and improves timeout error reporting when waiting for KV tensors.

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

Comment thread kvcached/kv_cache_manager.py
Comment thread kvcached/kv_cache_manager.py
@shipiyouniao shipiyouniao force-pushed the fix/tp-ipc-error-propagation branch from b6369c9 to 460b737 Compare July 6, 2026 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TP KV tensor map failures can be hidden behind generic wait timeouts

2 participants