Conversation
…r/unregister Mooncake's Ascend transport is bound to a thread-local ACL context. put()/get() run register_buffer/unregister_buffer on a per-call ThreadPoolExecutor whose worker threads never called npu.set_device, so unregister_buffer failed with INVALID_PARAMS (aclrtGetCurrentContext failed) and the registered host buffers were leaked. Run _ensure_accelerator_context as the executor initializer and once before store setup, and add a hardware-free unit test. Signed-off-by: ys2025-AI <yuanshuaien@163.com>
CLA Signature Passys2025-AI, thanks for your pull request. All authors of the commits have signed the CLA. 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Bind the Ascend ACL device context on every thread that calls Mooncake
register_buffer/unregister_buffer, i.e. the worker threads of the per-callThreadPoolExecutorinput()/get().Why
MooncakeStoreClient.put()/get()create a freshThreadPoolExecutorper call and register/unregister transient buffers inside those worker threads. ACL contexts are thread-local, so the workers had no context:register_buffersilently fell back to host memory;unregister_bufferfailed withINVALID_PARAMS, leaking the registration and flooding logs withaclrtGetCurrentContext failed.Changes
transfer_queue/storage/clients/mooncake_client.py_ensure_accelerator_context(): no-op whentorch.npuis unavailable (CUDA/CPU/generic unaffected), otherwisenpu.set_device(...)for the calling thread with a one-time context warmup;store.setup(...)(the client thread needs a context too);initializer=_ensure_accelerator_contextto bothThreadPoolExecutors (put()andget())._register_all_buffers/_unregister_all_buffersrun only inside those pools, so no other call sites are needed.Tests
tests/test_mooncake_client_thread_context.py(hardware-free):_ensure_accelerator_contextsets the device and warms up only once;put()andget()build their executor with a non-Noneinitializerthat binds the context (executor faked, store faked).Verification on hardware (Ascend 910B + mooncake-transfer-engine 0.3.11.post1)
Before:
Unregister buffer failed/aclrtGetCurrentContext failed1500+ times per short run.After: 0 occurrences; register/unregister succeed; no leak.