add shared KV backend abstraction layer#1944
Draft
hxieustc wants to merge 1 commit into
Draft
Conversation
Introduce src/plugins/kv/ as the canonical home for all KV-family backend plugins. This layer factors out boilerplate duplicated across the REDIS (ai-dynamo#1791) and WQSKV (ai-dynamo#1903) PRs so future backends have a single, consistent foundation to build on.
|
👋 Hi hxieustc! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
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.
Summary
This PR adds
src/plugins/kv/as the canonical home for all KV-family backend plugins and introducesnixlKVBackendBase, a shared abstract base class that factors out the boilerplate that existing KV plugins use (PR #1791 (REDIS) and PR #1903 (WQSKV) independently duplicated).Neither plugin is moved or modified by this PR — it purely establishes the abstraction so both can be adapted on top of it.
Motivation
PR #1791 and PR #1903 each implement a NIXL backend that stores descriptor data in a key-value store. Despite targeting different vendor libraries (hiredis vs. WDS KVCache), they independently arrived at identical implementations for:
metaInfoif non-empty, elsestd::to_string(devId))supportsRemote/Local/Notif)connect,disconnect,unloadMD,loadLocalMD)nixlBackendPluginCreator<T>)postXferdispatches →checkXferpolls →releaseReqHblocks)Without a shared base, every future KV backend will re-copy this boilerplate and risk diverging silently over time.
What's in this PR
src/plugins/kv/kv_req_handle.h—nixlKVReqH+nixlKVXferCallbackShared async request handle using an atomic
pendingcounter, afirst_errorCAS field, and a condition variable.nixlKVXferCallbackis a C-style completion callback compatible with both C++ async APIs and vendor C callback conventions (hiredis,wds_kvcache_put, etc.).src/plugins/kv/kv_backend.h—nixlKVBackendBase : nixlBackendEngineAbstract base class providing shared implementations of:
registerMem/deregisterMem—metaInfo-or-devIdkey resolution,devIdToKey_map managementcheckXfer/releaseReqH— poll/block onnixlKVReqHpending counterconnect,disconnect,unloadMD,loadLocalMD— no-op stubsgetSupportedMems()— defaults to{DRAM_SEG}; subclasses override to addOBJ_SEGetc.resolveKey()— two-step lookup (metadata pointer cast →devIdToKey_fallback)nixlKVMetadata— shared inner metadata type (subclassable for backend-specific fields)Leaves
prepXfer,postXfer,queryMemas pure virtuals.src/plugins/kv/kv_backend.cpp— concrete implementations of all shared methods.src/plugins/kv/meson.build— buildsnixl_kv_baseas a static library and exposesnixl_kv_base_depfor plugin subdirectories to link against. Pluginsubdir()stubs forredis/andwqskv/are present but commented out, ready to be activated as each PR migrates.src/plugins/meson.build— addssubdir('kv')unconditionally (base infrastructure, not an optional plugin).How each existing PR adapts
PR #1903 (WQSKV): mechanical — WQSKV already uses the atomic counter + CV pattern. Change the base class, remove duplicated fields/methods, replace
nixlWqskvBackendReqHwithnixlKVReqHandwqskv_xfer_cbwithnixlKVXferCallback. All 23 helper unit tests are unaffected.PR #1791 (REDIS): requires one behavioral change —
postXfercurrently usesstd::promise/std::futureper descriptor, which doesn't map ontonixlKVReqH. The hiredis completion callback should callnixlKVXferCallbackinstead ofpromise.set_value(status).checkXferandreleaseReqHare then deleted (inherited). TheiRedisClientinterface is retained for testability with updated signatures.File layout (target state after both PRs adapt)
Future KV backends follow the same pattern: add a subdirectory under
src/plugins/kv/, subclassnixlKVBackendBase, implementprepXfer/postXfer/queryMem.