Skip to content

fork: COMBINED layer-split head mode (hydra_vortex #383)#28

Merged
hydra-z[bot] merged 3 commits into
hydra-forkfrom
fork/hydra-383-combined-layer-split
Jul 5, 2026
Merged

fork: COMBINED layer-split head mode (hydra_vortex #383)#28
hydra-z[bot] merged 3 commits into
hydra-forkfrom
fork/hydra-383-combined-layer-split

Conversation

@ddvnguyen

Copy link
Copy Markdown
Owner

Implements the C++ side of ddvnguyen/hydra_vortex#383 T1.

What & why

New --combined-split-mode layer + --combined-tensor-split flags
on llama-engine that register the peer RPC device before model load,
so llama.cpp's stock tensor_split allocator places whole layers
(including recurrent-layer state cache) on one device at load time,
avoiding the SSM-truncation crash class from spike ggml-org#381.

The existing COMBINED mechanism (--combined-ot-pattern expert-split)
is the wrong split strategy for dense + hybrid SSM/attention models:
whole-block -ot corrupts recurrent-layer state across the RPC boundary.
Layer-split avoids this entirely because the stock allocator places
whole layers (including their SSM state cache) on one device at load time.

Test plan

  • cmake --build build_sm86 --target llama-engine (CUDA 13.2, sm_86, compiles clean)
  • cmake --build build_sm120 --target llama-engine (RTX)
  • Hardware E2E: head+peer with a small dense GGUF + --combined-tensor-split

Implements the C++ side of ddvnguyen/hydra_vortex#383 T1.

New --combined-split-mode layer plus --combined-tensor-split flags
on llama-engine that register the peer RPC device BEFORE model load,
so llama.cpp's stock tensor_split allocator places whole layers
(including recurrent-layer state cache) on one device at load time,
avoiding the SSM-truncation crash class from spike ggml-org#381.

Key behaviors:
- Peer TCP probe (5s) at startup; unreachable => fail-fast abort
- Pre-load peer registration via ggml_backend_rpc_add_server
- tensor_split parsing from --combined-tensor-split
- --combined-ot-pattern rejected in layer mode
- INFO 0x41: mode:"combined", split_mode:"layer",
  capabilities includes "combined"
- SET_EXPERT_MODE("combined") => no-op;
  SET_EXPERT_MODE("solo") => error "combined_static"

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ddvnguyen

Copy link
Copy Markdown
Owner Author

Test Results (sm86+sm120 fat binary, real hardware)

All tests pass on the 2-GPU same-host pair (5060 Ti + 3060).

Test 1 — Peer unreachable → fail-fast ✅

layer-split COMBINED: peer localhost:9506 unreachable — aborting startup
(cannot load model without peer device)

Test 2 — Peer reachable + layer-split startup ✅

  • layer-split COMBINED: peer localhost:9506 reachable
  • tensor_split='10/22' (2 device(s))
  • RPC0: localhost:9506 (11911 MiB, 11334 MiB free)
  • COMBINED layer-split ready — peer localhost:9506, split 10/22

Test 3 — INFO (0x41) ✅

mode: combined
split_mode: layer
capabilities: [..., "combined"]
layer_split: 10/22
peer_reachable: True
combined_head_attached: True

Test 4 — SET_EXPERT_MODE ✅

  • "solo" → status=ERROR: combined_static: cannot switch to solo at runtime
  • "combined" → status=OK, no-op

Test 5 — HTTP inference ✅

  • /v1/chat/completions returns 5 tokens
  • /health{"status":"ok"}

Additional fix in this PR

  • TCP probe fix: try_tcp_connect now checks SO_ERROR after select() to correctly distinguish successful non-blocking connects from failed ones (ECONNREFUSED). Without this fix, the probe incorrectly reports 'reachable' when connecting to a non-listening port.

@ddvnguyen ddvnguyen marked this pull request as ready for review July 2, 2026 15:26
…gml-org#383 T2)

T2 implements --peer-only: starts llama-engine without loading any
model, just exposing the local GPU backend(s) as a ggml-RPC server and
serving HTTP health checks. Replaces the nomic-embed-text workaround
for the 3060 peer.

Also fixes the start_shared_backend_rpc_server fallback path: when
the scheduler has no non-CPU backends (e.g. tiny placeholder model),
enumerate globally registered accelerator devices instead — same
approach as rpc-server.cpp.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@ddvnguyen ddvnguyen left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — fork: COMBINED layer-split head mode (ggml-org#383 T1/T2)

The architecture is right and the load-bearing part is correct: registering the peer RPC device before model load so the stock allocator places whole layers (incl. recurrent SSM state) on one device — that's exactly what avoids the ggml-org#381 crash class. The SO_ERROR addition to try_tcp_connect also fixes a genuine base bug (a refused non-blocking connect was reported as success).

One blocking item and a few smaller ones (all detail inline on tools/llama-engine/llama-engine.cpp):

  • Blocking (P1) — peer-only mode installs a no-op SIGINT/SIGTERM handler + for(;;) loop, so the process only dies to SIGKILL and the cleanup after the loop is unreachable. Breaks hydra-head's restart/backoff loop for the peer.
  • P1 (deployment-dependent) — peer-only exposes ALL non-CPU devices; safe only under CDI/CUDA_VISIBLE_DEVICES scoping. Add a device filter.
  • P2 — uncaught std::stoi on malformed --rpc-engine → SIGABRT.
  • P2 — no guard against layer-split + --ggml-rpc-port (reaches the ggml-org#376 crash config).
  • Cleanup — ~35-line RPC-server duplication; hand-rolled tensor-split parse that common/arg.cpp:2388-2414 already does (reuse also removes the hardcoded 128); duplicated host:port parsing across two branches.

Verified clean (no action)

  • tensor_split[128] exactly matches common_params' array size — no overflow.
  • --rpc-port 0 is a documented no-op (skips the Hydra KV-RPC bind), not a failure.
  • INFO capabilities array + push_back("combined") is unambiguous nlohmann array; the set_hydra_capabilities defaulted 5th param has a single, updated call site — no ABI/ODR/regression risk.

Hydra-side review posted on ddvnguyen/hydra_vortex#385.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking (P1) — start_peer_only_engine cannot be stopped by SIGINT/SIGTERM. It installs a no-op handler (sa.sa_handler = +[](int){ /* no-op, start_loop handles it */ } — but there is no start_loop in this path) and then enters for (;;) { sleep_for(1s); } with no flag to break it. The termination path the model engine uses (shutdown_handler → ctx_server.terminate(), which unblocks start_loop) is bypassed, and no other code here (server_http_context, common_init) installs a terminating handler. So the peer process ignores kill/Ctrl-C and only dies to SIGKILL, and the cleanup after the loop (ctx_http.stop(), llama_backend_free(), return 0) is unreachable dead code. Since hydra-head owns restart/backoff for this process, every restart cycle will hang until SIGKILL.

Fix: a real handler setting a std::atomic<bool> the loop checks, mirroring the model path's shutdown_handler; then the cleanup block runs. Also please confirm the sigaction is fully initialized (sigemptyset(&sa.sa_mask); sa.sa_flags = 0;) — the model path does this at the existing signal-setup site.


P1 (deployment-dependent) — start_backend_rpc_peer_server exposes ALL non-CPU devices with no filter. Unlike stock rpc-server (which restricts via -d/--deviceget_devices()), this enumerates every registered non-CPU device. It's safe today only because the container's CDI injection scopes GPU visibility to device 1; run peer-only on a bare host (or with visibility misconfigured) and it exposes the head's own GPU over RPC, so the head could tensor-split onto itself across the network. Add a device-selection equivalent, or at minimum assert/document the hard dependency on CUDA_VISIBLE_DEVICES/CDI scoping.


P2 — uncaught std::stoi on a malformed --rpc-engine. The layer-split peer parse (port = std::stoi(peer.substr(colon+1))) is unguarded at process scope, so host:, host:abc, or an out-of-range port throws std::invalid_argument/out_of_rangestd::terminate/SIGABRT instead of a clean error. (Pre-existing pattern in the expert branch and llama_hydra_peer_reachable; this PR adds a third instance.) Wrap the parse and emit a LOG_ERR + clean exit.


P2 — no guard against layer-split + --ggml-rpc-port on one process. The plan's avoidance of ggml-org#376 relies on the head not setting --ggml-rpc-port, but nothing rejects the combination. An operator copying the old head config would reach the co-resident add_server + start_server_with_backends configuration that ggml-org#376 asserts on. Cheap to reject explicitly in the layer-split validation block alongside the existing --combined-ot-pattern check.


Cleanup.

  • start_backend_rpc_peer_server duplicates ~35 lines of the new fallback branch inside start_shared_backend_rpc_server (same ggml_backend_load_all → resolve RPC reg → non-CPU device enumeration → devices.empty() guard → endpoint/threads → detached thread). Extract one start_rpc_server_over_global_devices(int port) and call it from both.
  • The hand-rolled tensor-split parser (find_first_of(",/") loop → vector<float> → copy into tensor_split[128]) re-implements common/arg.cpp:2388-2414, which already parses the identical comma/slash format. Factoring a shared helper out of that lambda also lets you drop the hardcoded 128 in favor of llama_max_devices().
  • host:port parsing is copy-pasted across the layer-split and expert branches with two different default-port literals (9506 / 8080) — extract parse_host_port(peer, default_port, host, port).

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant