mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-29 22:54:38 +02:00
* fix(daemon): isolate runtime poll & heartbeat schedules per runtime A daemon serving multiple workspaces ran a single round-robin poll loop and a single HTTP heartbeat loop across every registered runtime. A 30s HTTP timeout for any one runtime serialized that delay across all the others — observed in production as one workspace's runtimes wedging every other workspace's runtimes on the same daemon. This change: - Replaces the shared runtime-set channel with a multi-subscriber watcher so taskWakeupLoop, heartbeatLoop, and pollLoop can each react to runtime-set changes independently. - Splits heartbeatLoop and pollLoop into supervisor + per-runtime worker goroutines. Each runtime owns its claim cadence and its heartbeat ticker, so a slow request on one runtime no longer blocks any other. - Stagers the per-runtime heartbeat first tick by a jittered delay up to one full interval to avoid a thundering herd at startup. - Sizes the WS writer channel to scale with the runtime count (max(16, 2*N)) so a full per-runtime heartbeat batch always fits; the previous fixed 8-slot buffer dropped heartbeats whenever a daemon watched more than ~8 runtimes. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): acquire execution slot only after ClaimTask, drain pollers before taskWG Two issues from review on the previous commit: 1. Acquiring the shared task slot before ClaimTask reintroduced the very head-of-line blocking the refactor was meant to remove. With MaxConcurrentTasks=1, a slow claim on one runtime parked the only slot for the duration of the HTTP timeout (up to 30s), starving every other runtime's claim attempts. Slots are now acquired after the claim returns a task; other runtimes' pollers stay free to claim. The already-dispatched task waits for a slot under MaxConcurrentTasks bounds, which is the same backpressure shape we had before. 2. pollLoop's shutdown path called taskWG.Wait immediately after cancelling pollers, but a poller could still be between ClaimTask returning a task and taskWG.Add(1). When taskWG's counter is zero that races with Wait — undefined sync.WaitGroup misuse, sometimes panic. Added a pollerWG so the supervisor blocks until every poller goroutine has actually returned before reaching taskWG.Wait. Tests: - TestRunRuntimePollerIsolatesSlowRuntime now uses MaxConcurrentTasks=1 (was 4) so it would have failed under the old slot-before-claim path. - New TestPollLoopShutdownWaitsForPollersBeforeTaskWG drives the exact race window — claim returns a task at the same moment shutdown fires — under -race. Co-authored-by: multica-agent <github@multica.ai> * fix(daemon): acquire slot before ClaimTask so capacity-waiters never enter dispatched The previous commit moved slot acquisition AFTER ClaimTask to address a review concern about head-of-line blocking with MaxConcurrentTasks=1. That introduced a strictly worse failure mode: server-side ClaimTask flips the task to `dispatched` immediately (agent.sql:174-176), and the runtime sweeper fails any task in `dispatched` for >300s with `failed/timeout` (runtime_sweeper.go:25-28). When local execution capacity is full and the next claimed task can't acquire a slot within 5 minutes, the user sees the exact failure this issue is fixing — `dispatched_at` set, `started_at` NULL, `failure_reason=timeout`. Reverted to slot-before-claim. The trade-off is the original review concern: with MaxConcurrentTasks=1 and a slow ClaimTask, other runtimes' claims are delayed by up to client.Timeout=30s. That's a 30s polling delay, not a failure — server-side those tasks remain `queued` (no timeout in that state) until a slot frees. 30s ≪ 300s, so other runtimes' tasks cannot get sweeper-failed because of this. The pollerWG fix from the previous commit (avoiding sync.WaitGroup misuse on shutdown) is preserved. Tests: - TestRunRuntimePollerIsolatesSlowRuntime: MaxConcurrentTasks back to 4 (the pre-issue baseline) — the headroom case where slot-before- claim still gives full per-runtime isolation. - New TestRunRuntimePollerSkipsClaimWhenAtCapacity: holds the only slot and verifies the poller never calls ClaimTask while sem is empty. The previous "claim first" path would have failed this. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: multica-agent <github@multica.ai>
320 lines
8.9 KiB
Go
320 lines
8.9 KiB
Go
package daemon
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"errors"
|
|
"fmt"
|
|
"math/rand"
|
|
"net/http"
|
|
"net/url"
|
|
"sort"
|
|
"strings"
|
|
"time"
|
|
|
|
"github.com/gorilla/websocket"
|
|
"github.com/multica-ai/multica/server/pkg/protocol"
|
|
)
|
|
|
|
var errRuntimeSetChanged = errors.New("runtime set changed")
|
|
|
|
func (d *Daemon) taskWakeupLoop(ctx context.Context, taskWakeups chan<- struct{}) {
|
|
backoff := time.Second
|
|
runtimeSetCh, unsub := d.runtimeSet.Subscribe()
|
|
defer unsub()
|
|
|
|
for {
|
|
runtimeIDs := d.allRuntimeIDs()
|
|
if len(runtimeIDs) == 0 {
|
|
if err := sleepWithContextOrRuntimeChange(ctx, 5*time.Second, runtimeSetCh); err != nil {
|
|
return
|
|
}
|
|
continue
|
|
}
|
|
|
|
err := d.runTaskWakeupConnection(ctx, runtimeIDs, taskWakeups, runtimeSetCh)
|
|
if ctx.Err() != nil {
|
|
return
|
|
}
|
|
if errors.Is(err, errRuntimeSetChanged) {
|
|
backoff = time.Second
|
|
continue
|
|
}
|
|
if err != nil {
|
|
d.logger.Debug("task wakeup websocket unavailable; polling fallback remains active", "error", err, "retry_in", backoff)
|
|
}
|
|
|
|
if err := sleepWithContextOrRuntimeChange(ctx, jitterDuration(backoff), runtimeSetCh); err != nil {
|
|
return
|
|
}
|
|
if backoff < 30*time.Second {
|
|
backoff *= 2
|
|
if backoff > 30*time.Second {
|
|
backoff = 30 * time.Second
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
func jitterDuration(d time.Duration) time.Duration {
|
|
if d <= 0 {
|
|
return d
|
|
}
|
|
spread := d / 5
|
|
if spread <= 0 {
|
|
return d
|
|
}
|
|
delta := time.Duration(rand.Int63n(int64(spread)*2+1)) - spread
|
|
return d + delta
|
|
}
|
|
|
|
func (d *Daemon) runTaskWakeupConnection(ctx context.Context, runtimeIDs []string, taskWakeups chan<- struct{}, runtimeSetCh <-chan struct{}) error {
|
|
wsURL, err := taskWakeupURL(d.cfg.ServerBaseURL, runtimeIDs)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
headers := http.Header{}
|
|
if token := d.client.Token(); token != "" {
|
|
headers.Set("Authorization", "Bearer "+token)
|
|
}
|
|
if d.client.platform != "" {
|
|
headers.Set("X-Client-Platform", d.client.platform)
|
|
}
|
|
if d.client.version != "" {
|
|
headers.Set("X-Client-Version", d.client.version)
|
|
}
|
|
if d.client.os != "" {
|
|
headers.Set("X-Client-OS", d.client.os)
|
|
}
|
|
|
|
dialer := websocket.Dialer{HandshakeTimeout: 10 * time.Second}
|
|
conn, _, err := dialer.DialContext(ctx, wsURL, headers)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
defer conn.Close()
|
|
// HTTP heartbeats resume the moment WS detaches so the freshness window
|
|
// from a previous connection cannot keep them silenced past disconnect.
|
|
defer d.clearWSHeartbeatAcks()
|
|
|
|
d.logger.Info("task wakeup websocket connected", "runtimes", len(runtimeIDs))
|
|
signalTaskWakeup(taskWakeups)
|
|
|
|
// Serialize all writes through a single channel: the gorilla/websocket
|
|
// Conn does not allow concurrent WriteMessage calls, and the heartbeat
|
|
// sender now coexists with future server-initiated writes. The buffer
|
|
// is sized to fit a full per-runtime heartbeat batch plus headroom; a
|
|
// fixed 8-slot queue would silently drop heartbeats once a daemon
|
|
// watched more than ~8 runtimes (typical when one machine connects to
|
|
// several workspaces), even when the network was healthy.
|
|
writeBufSize := 16
|
|
if 2*len(runtimeIDs) > writeBufSize {
|
|
writeBufSize = 2 * len(runtimeIDs)
|
|
}
|
|
writes := make(chan []byte, writeBufSize)
|
|
writerDone := make(chan struct{})
|
|
go d.runWSWriter(conn, writes, writerDone)
|
|
|
|
heartbeatCtx, cancelHeartbeat := context.WithCancel(ctx)
|
|
hbDone := make(chan struct{})
|
|
go func() {
|
|
defer close(hbDone)
|
|
d.runWSHeartbeatSender(heartbeatCtx, runtimeIDs, writes)
|
|
}()
|
|
|
|
errCh := make(chan error, 1)
|
|
go func() {
|
|
errCh <- d.readTaskWakeupMessages(conn, taskWakeups)
|
|
}()
|
|
|
|
// Defer cleanup must shut goroutines down in this order:
|
|
// 1. cancel the heartbeat sender's ctx
|
|
// 2. wait for the sender to actually return — only then is it safe
|
|
// to close the writes channel without a "send on closed channel"
|
|
// panic from sendWSHeartbeats
|
|
// 3. close writes; the writer drains and exits
|
|
// 4. wait for the writer to finish so it doesn't outlive the conn
|
|
//
|
|
// LIFO defer order would close writes before the sender stops, so the
|
|
// teardown is folded into a single deferred function instead.
|
|
defer func() {
|
|
cancelHeartbeat()
|
|
<-hbDone
|
|
close(writes)
|
|
<-writerDone
|
|
}()
|
|
|
|
select {
|
|
case <-ctx.Done():
|
|
return ctx.Err()
|
|
case <-runtimeSetCh:
|
|
return errRuntimeSetChanged
|
|
case err := <-errCh:
|
|
return err
|
|
}
|
|
}
|
|
|
|
// runWSWriter funnels writes from the heartbeat sender (and any future
|
|
// daemon-initiated message) into a single goroutine. gorilla/websocket
|
|
// requires that all WriteMessage calls happen from the same goroutine.
|
|
func (d *Daemon) runWSWriter(conn *websocket.Conn, writes <-chan []byte, done chan<- struct{}) {
|
|
defer close(done)
|
|
for frame := range writes {
|
|
conn.SetWriteDeadline(time.Now().Add(10 * time.Second))
|
|
if err := conn.WriteMessage(websocket.TextMessage, frame); err != nil {
|
|
d.logger.Debug("task wakeup websocket write failed", "error", err)
|
|
conn.Close()
|
|
// Drain remaining frames so the producers don't block forever
|
|
// while waiting for runTaskWakeupConnection to close the channel.
|
|
for range writes {
|
|
}
|
|
return
|
|
}
|
|
}
|
|
}
|
|
|
|
// runWSHeartbeatSender emits a daemon:heartbeat per runtime every
|
|
// HeartbeatInterval. The first batch fires immediately so the server learns
|
|
// the connection identity without waiting a full interval. Frames are queued
|
|
// to the writer; if the queue is full the heartbeat is dropped (the
|
|
// freshness window is short enough that one missed beat just means HTTP will
|
|
// pick it up next tick).
|
|
func (d *Daemon) runWSHeartbeatSender(ctx context.Context, runtimeIDs []string, writes chan<- []byte) {
|
|
d.sendWSHeartbeats(ctx, runtimeIDs, writes)
|
|
interval := d.cfg.HeartbeatInterval
|
|
if interval <= 0 {
|
|
interval = 15 * time.Second
|
|
}
|
|
ticker := time.NewTicker(interval)
|
|
defer ticker.Stop()
|
|
for {
|
|
select {
|
|
case <-ctx.Done():
|
|
return
|
|
case <-ticker.C:
|
|
d.sendWSHeartbeats(ctx, runtimeIDs, writes)
|
|
}
|
|
}
|
|
}
|
|
|
|
func (d *Daemon) sendWSHeartbeats(ctx context.Context, runtimeIDs []string, writes chan<- []byte) {
|
|
for _, rid := range runtimeIDs {
|
|
if ctx.Err() != nil {
|
|
return
|
|
}
|
|
frame, err := json.Marshal(protocol.Message{
|
|
Type: protocol.EventDaemonHeartbeat,
|
|
Payload: marshalRaw(protocol.DaemonHeartbeatRequestPayload{RuntimeID: rid}),
|
|
})
|
|
if err != nil {
|
|
d.logger.Debug("ws heartbeat marshal failed", "error", err, "runtime_id", rid)
|
|
continue
|
|
}
|
|
select {
|
|
case writes <- frame:
|
|
case <-ctx.Done():
|
|
return
|
|
default:
|
|
// Writer is backed up; drop this beat. HTTP heartbeat will resume
|
|
// on its next tick once the freshness window expires.
|
|
d.logger.Debug("ws heartbeat dropped: writer backlog", "runtime_id", rid)
|
|
}
|
|
}
|
|
}
|
|
|
|
func marshalRaw(v any) json.RawMessage {
|
|
data, err := json.Marshal(v)
|
|
if err != nil {
|
|
return nil
|
|
}
|
|
return data
|
|
}
|
|
|
|
func (d *Daemon) readTaskWakeupMessages(conn *websocket.Conn, taskWakeups chan<- struct{}) error {
|
|
conn.SetReadLimit(64 * 1024)
|
|
for {
|
|
_, raw, err := conn.ReadMessage()
|
|
if err != nil {
|
|
return err
|
|
}
|
|
var msg protocol.Message
|
|
if err := json.Unmarshal(raw, &msg); err != nil {
|
|
d.logger.Debug("task wakeup websocket invalid message", "error", err)
|
|
continue
|
|
}
|
|
switch msg.Type {
|
|
case protocol.EventDaemonTaskAvailable:
|
|
var payload protocol.TaskAvailablePayload
|
|
if len(msg.Payload) > 0 {
|
|
if err := json.Unmarshal(msg.Payload, &payload); err != nil {
|
|
d.logger.Debug("task wakeup websocket invalid payload", "error", err)
|
|
continue
|
|
}
|
|
}
|
|
if payload.RuntimeID != "" {
|
|
d.logger.Debug("task wakeup received", "runtime_id", payload.RuntimeID, "task_id", payload.TaskID)
|
|
}
|
|
signalTaskWakeup(taskWakeups)
|
|
case protocol.EventDaemonHeartbeatAck:
|
|
var ack HeartbeatResponse
|
|
if err := json.Unmarshal(msg.Payload, &ack); err != nil {
|
|
d.logger.Debug("ws heartbeat ack invalid payload", "error", err)
|
|
continue
|
|
}
|
|
if ack.RuntimeID == "" {
|
|
continue
|
|
}
|
|
d.recordWSHeartbeatAck(ack.RuntimeID)
|
|
d.handleHeartbeatActions(context.Background(), ack.RuntimeID, &ack)
|
|
}
|
|
}
|
|
}
|
|
|
|
func signalTaskWakeup(taskWakeups chan<- struct{}) {
|
|
select {
|
|
case taskWakeups <- struct{}{}:
|
|
default:
|
|
}
|
|
}
|
|
|
|
func taskWakeupURL(baseURL string, runtimeIDs []string) (string, error) {
|
|
u, err := url.Parse(strings.TrimSpace(baseURL))
|
|
if err != nil {
|
|
return "", fmt.Errorf("invalid daemon server URL: %w", err)
|
|
}
|
|
switch u.Scheme {
|
|
case "http":
|
|
u.Scheme = "ws"
|
|
case "https":
|
|
u.Scheme = "wss"
|
|
case "ws", "wss":
|
|
default:
|
|
return "", fmt.Errorf("daemon server URL must use http, https, ws, or wss")
|
|
}
|
|
|
|
u.Path = strings.TrimRight(u.Path, "/") + "/api/daemon/ws"
|
|
u.RawPath = ""
|
|
q := u.Query()
|
|
ids := append([]string(nil), runtimeIDs...)
|
|
sort.Strings(ids)
|
|
q.Set("runtime_ids", strings.Join(ids, ","))
|
|
u.RawQuery = q.Encode()
|
|
u.Fragment = ""
|
|
return u.String(), nil
|
|
}
|
|
|
|
func sleepWithContextOrRuntimeChange(ctx context.Context, d time.Duration, runtimeSetCh <-chan struct{}) error {
|
|
timer := time.NewTimer(d)
|
|
defer timer.Stop()
|
|
|
|
select {
|
|
case <-ctx.Done():
|
|
return ctx.Err()
|
|
case <-runtimeSetCh:
|
|
return nil
|
|
case <-timer.C:
|
|
return nil
|
|
}
|
|
}
|