mirror of
https://github.com/mudler/LocalAI.git
synced 2026-10-06 21:14:23 -04:00
Review round 1 on the end-to-end proof. Zero blocking items, eleven non-blocking, and three of them turned out to be production defects rather than notes on the report. The one that matters is a misclassification the phase is built to prevent. A dial carries the caller's deadline down to the socket, so when the budget runs out the socket's timer fires and the error travels back up through the WebSocket handshake and the multiplexer. The context's cancellation is a separate timer whose func the scheduler has to run before ctx.Err() stops returning nil, and nothing orders the two. Under contention the socket's error is back in PeerPool.Open first, ctx.Err() reads nil, and a peer that is listening and healthy is reported as ErrPeerUnreachable to a caller that simply ran out of time. An unreachable peer is a fact a caller may act on and an expired deadline is not, and core/services/nodes routes around a replica it is told is unreachable. callerRanOut answers that question in one place: ctx.Err() when it is set, and otherwise the wall clock against the caller's own deadline. That is sound because it is the same instant the socket compared itself against, so if the socket's timer fired this comparison is past it too. The ambiguous instant resolves towards the caller, which is the direction that never blames a peer. The spec that caught it, peerlink_test.go's "blames the caller's deadline", was red in three of seven -race runs and had been since Task 5, which is often enough to read as noise and is why single-run verification never saw it. Rather than leave the proof to a coin flip, a second spec makes the window deterministic: Open is handed a context whose deadline has passed and whose cancellation has not been delivered, against an address nothing is listening on, so the dial fails for real. It reddens without the fix. The peer link's yamux windows were applied to one end only. A receive window is advertised by the side that RECEIVES, so configuring the dialler alone tunes exactly one direction, and the direction left on the 256 KiB default is the one that carries a relayed model artifact INTO the replica that owns the worker's tunnel. That is the largest thing the link ever moves and it is the direction the load measurement exercises: the review read it as flowing toward the dialler and it does not. PeerLinkConfig is now exported and used on both ends. Measured, same box, 128 MiB staged through the relay against the same transfer without one: the relayed path cost 1.6x to 2.0x the direct path's transfer window before, and 1.06x to 1.25x after. The SSRF reachability spec could be fooled into reporting an SSRF that did not happen. It bound the victim on 127.0.0.2 at an ephemeral port and required 127.0.0.1 at the same port to refuse, so any other spec in the run holding that number made the dial succeed; red one run in seven, green five of five in isolation. It now picks from below the kernel's ephemeral range, the same fix the harness got for the adjacent-port collision. The rest are the specs and the report saying what they mean. Scenario 1's advertisement assertion could not tell "the worker advertises nothing" from "the JSON key moved", which matters because removing the advertisement is the change it covers. It was green against a renamed key. The roster now keeps the raw key set beside the decoded fields and the spec requires both keys present before reading them as empty. Scenario 4's refusal-body check was a four-way disjunction admitting bare "tunnel", "not connected" and "unroutable". Those alternatives were inert and each would be satisfied by refusals that say nothing about routing, in the one assertion the whole negative control rests on. It is "no route" alone. The head-of-line gate bounded the worst probe by the whole transfer window, which admits about eightfold degradation and loosens as the box slows. It is now half the window, plus a scale-free ratio against the worst probe under the SAME cold load with nothing to transfer, which is the control that isolates the transfer from the load. Not tighter than that, and the reason is measured rather than cautious: under a concurrent -race suite the worst relayed probe reached a fifth of its window, so a quarter-window gate would have had 1.2x of margin, and a spec that fails one run in three is worse than no spec. The report entry printed p90 and p99 off samples of twenty, where both land on the same element and p99 often lands on the max, so one number appeared three times under three names. A quantile is now printed only when the sample can separate it. Two claims in the report were wrong and are withdrawn rather than softened. Scenario 2's race is closed by the trailing re-read of the owner, not by the pre-assertion the report credited: a move to the non-owner mid-request would serve directly and still return 200, and only the trailing read reddens on it. And "the median request is unchanged" holds on this box and not on the reviewer's, where the relayed median rises up to 82% and p99 up to 3.5x. What survives on both is structural: the worst probe is a small fraction of the window in which bytes are moving, so the session interleaves rather than serialising. Sharing a session with a bulk transfer costs latency; it does not cost service. The disk footprint note undercounted, and the reviewer lost a run to a full disk on this box, so it is worth having right: two bulk models seeded into two frontends and staged to the worker is about 768 MiB, not 512 MiB. Left alone deliberately: the worker's backend port allocator still hands out ports without checking they are free, and its default range still overlaps the kernel's ephemeral range. It is confirmed, it is out of scope here, and it is being tracked as a named follow-up rather than fixed under an e2e task. Assisted-by: Claude Opus 5 [claude-code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
119 lines
4.6 KiB
Go
119 lines
4.6 KiB
Go
// SPDX-License-Identifier: MIT
|
|
|
|
// Package cluster serves the replica-to-replica link that a LocalAI frontend
|
|
// uses to reach a worker tunnel it does not own. A peer dials
|
|
// GET /api/cluster/peer, the connection becomes one multiplexed yamux session,
|
|
// and the relay opens a stream on it per request.
|
|
package cluster
|
|
|
|
import (
|
|
"crypto/subtle"
|
|
"net/http"
|
|
"strings"
|
|
|
|
"github.com/gorilla/websocket"
|
|
"github.com/labstack/echo/v4"
|
|
"github.com/libp2p/go-yamux/v5"
|
|
clustersvc "github.com/mudler/LocalAI/core/services/cluster"
|
|
"github.com/mudler/xlog"
|
|
)
|
|
|
|
// PeerHandler upgrades an authenticated peer dial to a WebSocket, wraps it as
|
|
// a yamux server session and hands it to onSession.
|
|
//
|
|
// onSession runs on the request goroutine, so it must return promptly; the
|
|
// session outlives the handler because the upgrade hijacks the connection, and
|
|
// closing it is the caller's job.
|
|
func PeerHandler(token string, onSession func(peerID string, sess *yamux.Session)) echo.HandlerFunc {
|
|
// gorilla's default CheckOrigin already restricts a browser to same-origin
|
|
// and lets a header-less client (which every peer is) through, so the
|
|
// zero value is what this link wants.
|
|
upgrader := websocket.Upgrader{}
|
|
|
|
return func(c echo.Context) error {
|
|
// Reject before upgrading. Upgrading and then closing would give the
|
|
// dialer a WebSocket error in place of an HTTP status, and both the
|
|
// route-coverage test and a peer's own retry logic read the status.
|
|
if !authorizedPeer(c.Request(), token) {
|
|
return echo.NewHTTPError(http.StatusUnauthorized, "unauthorized")
|
|
}
|
|
|
|
peerID := c.QueryParam("id")
|
|
if peerID == "" {
|
|
return echo.NewHTTPError(http.StatusBadRequest, "missing peer id")
|
|
}
|
|
|
|
ws, err := upgrader.Upgrade(c.Response(), c.Request(), nil)
|
|
if err != nil {
|
|
// Upgrade has already written its own failure to the client.
|
|
xlog.Debug("cluster peer link upgrade failed", "peer", peerID, "error", err)
|
|
return nil
|
|
}
|
|
|
|
// Server side of the mux: the dialing peer is the client, so it owns
|
|
// the odd stream IDs and this side the even ones.
|
|
//
|
|
// The SAME configuration the dialler uses, and that is load bearing
|
|
// rather than symmetry for its own sake. A yamux receive window is
|
|
// advertised by the receiving side, so a nil here left this end on the
|
|
// 256 KiB default while the dialler ran at 4 MiB, and the direction
|
|
// governed by this end is the one that carries a relayed model artifact
|
|
// INTO the replica that owns the worker's tunnel. That direction was
|
|
// measured at roughly half the throughput of the same transfer without
|
|
// a relay in it.
|
|
//
|
|
// It also puts the same ceiling on unread data at this end that
|
|
// PeerLinkConfig already documents for the dialling end, so a replica
|
|
// is now sized against that figure per link in BOTH directions. That
|
|
// is the cost of the window being useful at all: a window is a bound
|
|
// on data received and not yet read, so a receiver that will not
|
|
// buffer cannot advertise one.
|
|
sess, err := yamux.Server(clustersvc.WebsocketConn(ws), clustersvc.PeerLinkConfig(), nil)
|
|
if err != nil {
|
|
xlog.Error("cluster peer link session setup failed", "peer", peerID, "error", err)
|
|
_ = ws.Close()
|
|
return nil
|
|
}
|
|
|
|
if onSession == nil {
|
|
// Nothing will ever read from this session, so do not leave the
|
|
// peer believing it has a live link.
|
|
_ = sess.Close()
|
|
return nil
|
|
}
|
|
|
|
xlog.Debug("cluster peer link established", "peer", peerID, "remote", ws.RemoteAddr().String())
|
|
// net/http recovers a panic from this goroutine but does not close a
|
|
// hijacked connection afterwards, so a panicking callback would leave
|
|
// the peer holding a link nobody accepts streams on: its opens would
|
|
// fill the 256-deep backlog and then hang without an error.
|
|
defer func() {
|
|
if r := recover(); r != nil {
|
|
_ = sess.Close()
|
|
panic(r)
|
|
}
|
|
}()
|
|
onSession(peerID, sess)
|
|
return nil
|
|
}
|
|
}
|
|
|
|
// authorizedPeer compares the request's bearer token with the cluster token in
|
|
// constant time, matching the check the worker file-transfer server makes.
|
|
//
|
|
// Unlike that one, an empty configured token authorizes nobody: this route is
|
|
// registered in every deployment, so failing open would publish an
|
|
// unauthenticated mux to any caller that can reach the port.
|
|
func authorizedPeer(r *http.Request, expected string) bool {
|
|
if expected == "" {
|
|
return false
|
|
}
|
|
// RFC 7235 makes the scheme case-insensitive; the token after it is not.
|
|
const prefix = "Bearer "
|
|
header := r.Header.Get("Authorization")
|
|
if len(header) < len(prefix) || !strings.EqualFold(header[:len(prefix)], prefix) {
|
|
return false
|
|
}
|
|
return subtle.ConstantTimeCompare([]byte(header[len(prefix):]), []byte(expected)) == 1
|
|
}
|