Metadata manager pr5 by aschwartz12 · Pull Request #1930 · ai-dynamo/nixl

aschwartz12 · 2026-07-13T08:37:11Z

Metadata Manager PR5 Summary

This is the tip of the stack, so the change is described against main.
Replaces the agent's inline metadata-exchange path with an
internal, agent-owned nixlMDManager that owns a worker thread and routes each
call to a pluggable, self-contained backend (P2P, ETCD, or TCPStore). The public
nixlAgent metadata API is unchanged: callers still use the same sendLocalMD,
sendLocalPartialMD, fetchRemoteMD, and invalidateLocalMD calls with
nixl_opt_args_t.

On main, P2P and ETCD metadata exchange is implemented inline in nixlAgent on top
of a shared comm-worker thread, with the socket and etcd helpers living in
nixl_listener.cpp, and there is no TCPStore transport. This change factors all of
that behavior behind a nixlMetadataBackend contract, moves the worker thread into
the manager, deletes nixl_listener.cpp, and adds a TCPStore backend. Observable
behavior of the existing P2P and ETCD paths is preserved.

What?

Adds the internal manager/backend layer for metadata exchange and three
self-contained backends (P2P, ETCD, TCPStore), and removes the inline metadata
path and the shared comm worker. The manager is owned by nixlAgentData and is not
part of the installed public API. TCPStore is a new centralized-store option
alongside ETCD.

Why?

On main the metadata transport is entangled with the agent and a shared comm
thread, so each transport's details live inside nixlAgent and nixl_listener.cpp.
This creates a clean boundary: the manager owns scheduling and threading, each
backend owns its transport and I/O, and adding a new backend is one class behind
the contract with no agent-side or public-API changes.

How?

Manager - nixlMDManager is owned by nixlAgentData and is the single path for all
metadata exchange (the env opt-in and the inline fallback are gone). It owns one
worker thread and routes each call by precedence: a peer address selects P2P,
otherwise the configured name-addressed store backend.

Backends - nixlMetadataBackend defines the transport boundary. Three
implementations: P2P owns its sockets and listener, ETCD owns its nixlEtcdClient,
and TCPStore owns an in-house c10d client (no libtorch). Each backend is
self-contained and runs its blocking I/O on the worker; serviceEvents() drives
the watch / accept / read loops for backends that need them.

Contract - the prepare* methods run on the caller thread: they validate and
serialize, return a synchronous status, and hand back the transport work as a
nixlWorkerTask. The task and serviceEvents() run on the manager's worker thread.
needsWorker() gates whether the worker runs for a given backend.

Cutover - nixl_listener.cpp (the comm-worker loop plus the socket and etcd
helpers) is deleted. Its loop moves into the manager's worker, the socket helpers
move into the P2P backend, and the nixlEtcdClient moves into the ETCD backend.
nixlAgentData drops its comm-thread members and instead implements
nixlMetadataContext (getLocalMD / getLocalPartialMD / loadRemoteMD / getName /
getConfig / invalidateRemoteMD / submitTask) for the backends.

Scope - No public API change. Existing P2P and ETCD behavior is preserved, and
all transport I/O now runs on the manager's worker instead of the agent comm
thread. TCPStore is new and behaves like the other centralized store: fetchRemoteMD
returns immediately and the caller polls checkRemoteMD, so fetch semantics are
uniform across P2P, ETCD, and TCPStore.

Adds nixlMDManager as a name-keyed, P2P-only wrapper over the existing nixlAgent metadata APIs. Off by default; opt in by setting NIXL_MD_MANAGER in the environment. nixlAgent, the listener, ETCD, and UUID identity behavior are unchanged. ETCD support on the manager and the eventual cut-over of nixlAgent are deferred to PR 2 and PR 3. - New nixlMDManager (header in src/api/cpp, impl in src/core) keeps a name -> {ip, port} registry and delegates each call to the matching nixlAgent method via nixl_opt_args_t{ipAddr, port}. No new threads, sockets, or backend code. - nixlAgent::getMDManager() lazily constructs the manager on demand; returns NIXL_ERR_NOT_SUPPORTED when NIXL_MD_MANAGER is unset. - Tests in test/gtest/md_manager.cpp cover the env-var gate, registry validation, and P2P send/fetch/invalidate/check between two loopback agents.

Add the second pluggable backend behind the PR1 port, kept additive and off by default: - Add nixlEtcdMetadataBackend (HAVE_ETCD): depends only on the nixlMetadataContext port, reusing getLocalMD / getLocalPartialMD and enqueuing ETCD work on the agent's existing comm thread. It does not call the agent's public send/fetch, so env-gated routing cannot recurse. It duplicates the inline etcd path, which stays until a later PR. - nixlMDManager selects a single backend per run via the port: the KV (etcd) backend when NIXL_ETCD_ENDPOINTS is set, otherwise P2P. - Extend the NIXL_USE_MD_MANAGER routing in the agent's metadata methods to also cover the etcd (no-address) case. - Add NIXL_ETCD_ENDPOINTS-gated gtests for the etcd path. - No public API change; no friend (backend uses the port). Signed-off-by: Asaf Schwartz <aschwartz@nvidia.com>

Add the third pluggable backend behind the PR1 port, kept additive and off by default: - Add nixlTcpStoreMetadataBackend: depends only on the nixlMetadataContext port, reusing getLocalMD / getLocalPartialMD for serialization and a new loadRemoteMD port method for cache load. Unlike P2P/etcd it does its store I/O synchronously (no comm thread) and builds its own keys. It does not call the agent's public send/fetch, so env-gated routing cannot recurse. - Add nixlTcpStoreClient: a minimal in-house client of the c10d TCPStore wire protocol, so libnixl links no libtorch yet interoperates with a torch.distributed.TCPStore server. - nixlMDManager selects a single backend per run via the port: TCPStore when NIXL_TCPSTORE_ENDPOINTS is set, etcd when NIXL_ETCD_ENDPOINTS is set (the two stores are mutually exclusive), otherwise P2P. - Extend the NIXL_USE_MD_MANAGER routing in the agent's metadata methods to also cover the TCPStore (no-address) case; build the manager for TCPStore even without the comm thread. - Add NIXL_TCPSTORE_ENDPOINTS-gated gtests for the TCPStore path. - No public API change; no friend (backend uses the port). Single-peer fetch only (multi_get retained for the deferred group path); no shared etcd helpers since the shipped etcd backend is a comm-thread adapter that builds its keys on the listener thread. Signed-off-by: Asaf Schwartz <aschwartz@nvidia.com>

Remove the interim inline path that PR1-3 duplicated, so nixlMDManager and its backends are the only route for metadata exchange. Public API unchanged. - Build the manager unconditionally; drop the NIXL_USE_MD_MANAGER opt-in gate (detectMdManager / useMdManager_). - sendLocalMD / sendLocalPartialMD / fetchRemoteMD / invalidateLocalMD delegate straight to the manager; the inline socket/etcd branches are gone. The manager routes P2P when a peer address is given, otherwise the configured backend. - De-duplicate (de)serialization: nixlAgent::getLocalMD / getLocalPartialMD / loadRemoteMD delegate to the single nixlAgentData implementation. - Adjust SocketSendLocalPartialWithErrors: the redundant public-wrapper log is gone with the inline path; the serialization-failure log still applies. Signed-off-by: Asaf Schwartz <aschwartz@nvidia.com>

Make the metadata backends self-contained and move the single worker thread into nixlMDManager, removing the transport-specific comm-worker switch. Public API and async semantics are unchanged. - Contract encodes the thread boundary: each op is a prepare* method that runs on the caller thread (validate + serialize, returns a synchronous status) and hands back a nixlPreparedOp{status, task}; the task and serviceEvents() run on the manager's worker thread. Backends never touch a queue or a thread. - Context (nixlMetadataContext): backends reach the agent only through it, with getConfig(), invalidateRemoteMD(), plus the existing serialization/load hooks. - Manager: owns the worker thread + task queue; routes a call to a backend's prepare*, returns its status synchronously, and schedules the task. - P2P: owns its sockets, the listener, the socket helpers, and inbound LOAD/SEND/INVL handling in serviceEvents(). - ETCD: owns the etcd client and watchers; serviceEvents() drains watch invalidations via the context. - TCPStore: its I/O now runs on the worker (async, like the others). - Delete nixl_listener.cpp; relocate the shared cache helpers (loadConnInfo, loadRemoteSections, invalidateRemoteData) and default_metadata_label. Signed-off-by: Asaf Schwartz <aschwartz@nvidia.com>

github-actions · 2026-07-13T08:37:19Z

👋 Hi aschwartz12! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

copy-pr-bot · 2026-07-13T08:37:20Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

ColinNV · 2026-07-13T16:51:50Z

-      needsCommThread_(useEtcd_ || config.useListenThread),
+      // The manager runs a worker thread when P2P listens or a centralized store
+      // (etcd/tcpstore) is configured; that thread shares agent state, so the
+      // sync mode is upgraded to STRICT in those cases.


Sync mode none is upgraded to strict when the MD manager uses a thread, but sync mode rw is unchanged, which is inconsistent: if rw is sufficient then the upgrade should target it, if not it, too, needs to be "upgraded" to strict.

ColinNV · 2026-07-13T16:53:53Z

+ * @file nixl_etcd_metadata_backend.h
+ * @brief ETCD (centralized key/value) metadata backend.
+ */
+#ifndef NIXL_SRC_CORE_NIXL_ETCD_METADATA_BACKEND_H


Not sure this is the correct directory for these backends; src/plugins/metadata might be a better location?

ColinNV · 2026-07-13T16:54:34Z

+
+    /** Agent configuration a backend needs (listen port/thread, watch timeout). */
+    [[nodiscard]] virtual const nixlAgentConfig &
+    getConfig() const = 0;


This is leaking the whole agent config to the metadata backends...

ColinNV · 2026-07-13T16:55:10Z

+
+    /** This agent's name; used by centralized backends to build their KV keys. */
+    [[nodiscard]] virtual const std::string &
+    getName() const = 0;


Perhaps rename to agentName() or similar since the name it's returning is not that of the metadata context.

ColinNV · 2026-07-13T17:04:43Z

+
+void
+nixlMDManager::start() {
+    const bool need = p2pBackend_->needsWorker() || (backend_ && backend_->needsWorker());


It feels like this decision right here, made available to MD manager users in some small bool-returning function, should drive the "upgrade" of the locking in agent data instead of tracking leaked details there; since the MD worker is only started later this should be relatively easy.

ColinNV · 2026-07-13T17:57:36Z

+    /**
+     * @brief Whether the ETCD backend is selected (NIXL_ETCD_ENDPOINTS set and
+     *        this build has ETCD support). Single source of truth: the manager
+     *        uses it to pick the name backend, the agent to gate the comm thread.


The agent no longer has a comm thread.

ColinNV · 2026-07-13T18:00:27Z

+
+template<typename Prepare>
+nixl_status_t
+nixlMDManager::route(const nixl_opt_args_t *extra_params, Prepare prepare) {


Would this back-and-forth not be much simpler if each backend had its own thread instead of the MD manager having its worker?

ColinNV · 2026-07-13T18:05:17Z

+                    }
+                    const nixl_blob_t blob = client_->get(key);
+                    if (blob.empty()) {
+                        // The key was deleted between check() and get() (raced invalidate);


Or the check fails but by the time the get would have been performed the key is available; it feels slightly arbitrary to call check before get given that a small delay could accomplish the same without risking these false negatives?

ColinNV · 2026-07-13T18:09:37Z

+nixlTcpStoreMetadataBackend::nixlTcpStoreMetadataBackend(nixlMetadataContext &ctx) : ctx_(ctx) {
+    const std::string endpoints = nixl::config::getNonEmptyString("NIXL_TCPSTORE_ENDPOINTS");
+
+    // NIXL_TCPSTORE_ENDPOINTS is a single host:port (rfind keeps IPv4 hosts and


Will it support multiple host:port pairs in the future? If not the environment variable should not end with a plural S.

ColinNV · 2026-07-13T18:10:34Z

+constexpr char namespace_prefix[] = "/nixl/agents";
+
+// Bound the connect and each blocking store op. Overridable for slow bring-up.
+constexpr long default_timeout_ms = 30000;


The single MD worker thread can hang in a single operation for this amount of time?

aschwartz12 added 5 commits July 8, 2026 16:06

github-actions Bot added the external-contribution label Jul 13, 2026

pull-request-size Bot added the size/XXL label Jul 13, 2026

ColinNV requested changes Jul 13, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Metadata manager pr5#1930

Metadata manager pr5#1930
aschwartz12 wants to merge 5 commits into
ai-dynamo:mainfrom
aschwartz12:metadata_manager_pr5

aschwartz12 commented Jul 13, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

copy-pr-bot Bot commented Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

ColinNV Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

aschwartz12 commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Metadata Manager PR5 Summary

What?

Why?

How?

Uh oh!

github-actions Bot commented Jul 13, 2026

Uh oh!

copy-pr-bot Bot commented Jul 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aschwartz12 commented Jul 13, 2026 •

edited

Loading