[Feat] Mooncake|Posix#972
Conversation
9d4e4d6 to
b0bd13a
Compare
b0bd13a to
20ddb7d
Compare
7a70990 to
bff2484
Compare
7979410 to
e1490d5
Compare
e1490d5 to
d31e443
Compare
ygwpz
left a comment
There was a problem hiding this comment.
Follow-up Review Summary
Status: All previous concerns have been addressed
After reviewing the current state of the PR, I can confirm that all concerns raised in previous reviews have been addressed:
-
Thread-safety for
_pending_dump_tasks(line 910) - Addressed: UESTC-AHao explained thatwait_for_saveand_drain_pending_dump_tasksare both called sequentially within the same scheduler main loop, so synchronization is unnecessary. -
Check result of
unregister_buffer()(mooncake_store.cc:84) - Verified fixed: Warning log added for failed unregister operations (lines 84-87). -
Performance concern for per-request dump tasks - Addressed: UESTC-AHao confirmed no practical impact upon testing.
-
Extract function to StoreV1 (fake_store.cc:85) - Addressed: Function already extracted in StoreV1.
-
3fsstore implementation of RegisterMemory (ucmstore_v1.h:161) - Verified: ds3fs_store.cc implements
RegisterMemoryat lines 174-179. -
Dump task leaking in C++ task map (line 750) - Verified fixed: Now using
store.wait()to properly clean up tasks after completion. -
Async dump failure handling (line 909) - Addressed: Failed requests are properly returned as finished in
get_finished.
Unresolved Issue: Merge Conflicts
Important: This PR currently has merge conflicts (mergeable_state: dirty). The branch needs to be rebased against the develop branch to resolve conflicts before merging.
Please rebase the branch and resolve any conflicts with the upstream develop branch.
d31e443 to
e578441
Compare
Purpose
UCM + Mooncake & Mooncake|Posix (Ascend 0.18.0)
Modifications
unified-cache-management\examples\ucm_mooncake_config.yaml
Test