[PR #1053] [MERGED] [Store|TransferEngine]: use condition-variable based completion instead of busy-polling #1982

New issue

Closed

opened 2026-10-29 21:31:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-29 21:31:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/kvcache-ai/Mooncake/pull/1053
Author: @wwq2333
Created: 11/13/2025
Status: ✅ Merged
Merged: 11/17/2025
Merged by: @stmatengss

Base: main ← Head: feature/event-driven-completion

📝 Commits (10+)

33ca8c2 (Event-driven completion) Provide an option to use condition-variable based completion instead of busy-polling
298fc6b replace C-style BatchID→BatchDesc casts with Transport::toBatchDesc helper (reinterpret_cast)
83ffbb1 Cleanup(TransferEngineOperationState): remove spurious notify and confusing comment in set_result_internal
6815962 Use a single atomic completed counter to avoid reading inconsistent results
6c2425d clang-format
4941ef1 deduplicate completion signaling via check_completion
3e92383 fast‑path completed check before waiting on BatchDesc::completion_cv
f62a6fb Refine comments explaining atomic operations and memory ordering
f2f7420 fix: protect batch_desc.is_finished update with completion_mutex to prevent lost wakeups
4a1520c clang-format

📊 Changes

3 files changed (+157 additions, -7 deletions)

View changed files

📝 mooncake-common/common.cmake (+7 -0)
📝 mooncake-store/src/transfer_task.cpp (+51 -4)
📝 mooncake-transfer-engine/include/transport/transport.h (+99 -3)

📄 Description

Summary

Add USE_EVENT_DRIVEN_COMPLETION compile-time option in common.cmake (default OFF).
- Store: TransferEngineOperationState waits on BatchDesc::completion_cv with timeout; falls back to original polling when the flag is OFF.
- TransferEngine: add per-batch finished_task_count, has_failure, is_finished and completion_cv on BatchDesc; notify on last task completion; unify _atomic* usage.
No behavior change when USE_EVENT_DRIVEN_COMPLETION is OFF (default).

Motivation

Replace CPU-intensive busy-polling in the store with an event-driven approach.
Reduce CPU usage while improving latency on completion notification.

Fix #1033

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/kvcache-ai/Mooncake/pull/1053 **Author:** [@wwq2333](https://github.com/wwq2333) **Created:** 11/13/2025 **Status:** ✅ Merged **Merged:** 11/17/2025 **Merged by:** [@stmatengss](https://github.com/stmatengss) **Base:** `main` ← **Head:** `feature/event-driven-completion` --- ### 📝 Commits (10+) - [`33ca8c2`](https://github.com/kvcache-ai/Mooncake/commit/33ca8c22023f926446a17203ca509d148dfae37f) (Event-driven completion) Provide an option to use condition-variable based completion instead of busy-polling - [`298fc6b`](https://github.com/kvcache-ai/Mooncake/commit/298fc6b19c289dd41510bc69c10351366103cbf8) replace C-style BatchID→BatchDesc casts with Transport::toBatchDesc helper (reinterpret_cast) - [`83ffbb1`](https://github.com/kvcache-ai/Mooncake/commit/83ffbb13c496a768279f43ec09d51dc815cc6f42) Cleanup(TransferEngineOperationState): remove spurious notify and confusing comment in set_result_internal - [`6815962`](https://github.com/kvcache-ai/Mooncake/commit/6815962f389464664d8bcb53930494510e30381b) Use a single atomic completed counter to avoid reading inconsistent results - [`6c2425d`](https://github.com/kvcache-ai/Mooncake/commit/6c2425dcb41bfe9be3fdaa7a4b4a5c2357dbcfc8) clang-format - [`4941ef1`](https://github.com/kvcache-ai/Mooncake/commit/4941ef1f263d6aeac2b68b4cea2a4197657c3ad6) deduplicate completion signaling via check_completion - [`3e92383`](https://github.com/kvcache-ai/Mooncake/commit/3e923834f7fcb9a571d7268d1f9fe7d534bcf772) fast‑path completed check before waiting on BatchDesc::completion_cv - [`f62a6fb`](https://github.com/kvcache-ai/Mooncake/commit/f62a6fb2172cb47a145cef748146d04869a38cda) Refine comments explaining atomic operations and memory ordering - [`f2f7420`](https://github.com/kvcache-ai/Mooncake/commit/f2f74203b7a04d9b7f8424e0557f713bea03b2cf) fix: protect batch_desc.is_finished update with completion_mutex to prevent lost wakeups - [`4a1520c`](https://github.com/kvcache-ai/Mooncake/commit/4a1520c1ce3a8e8249dc49456db71bd5efce5acb) clang-format ### 📊 Changes **3 files changed** (+157 additions, -7 deletions) <details> <summary>View changed files</summary> 📝 `mooncake-common/common.cmake` (+7 -0) 📝 `mooncake-store/src/transfer_task.cpp` (+51 -4) 📝 `mooncake-transfer-engine/include/transport/transport.h` (+99 -3) </details> ### 📄 Description **Summary** - Add USE_EVENT_DRIVEN_COMPLETION compile-time option in common.cmake (default OFF). - Store: TransferEngineOperationState waits on BatchDesc::completion_cv with timeout; falls back to original polling when the flag is OFF. - TransferEngine: add per-batch finished_task_count, has_failure, is_finished and completion_cv on BatchDesc; notify on last task completion; unify __atomic_* usage. - No behavior change when USE_EVENT_DRIVEN_COMPLETION is OFF (default). **Motivation** - Replace CPU-intensive busy-polling in the store with an event-driven approach. - Reduce CPU usage while improving latency on completion notification. **Fix #1033** --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-29 21:31:52 +00:00