Skip to content

Commit d226ccf

Browse files
sarg3ntclaude
andcommitted
fix(haproxy): address Copilot review — parallelize fetches, correct gear list (#112)
Two Copilot review findings on the original capability-aware filter: 1. Doc inaccuracy: the example agent-gear list included "services", but the agent has no `services` gear (the /api/v1/services endpoint is served by the `metrics` gear). Replace with the correct list and call out that some dashboard gears (services, alerts, bx) don't have agent counterparts and shouldn't be filtered via this helper. 2. O(N * fetchTimeout) cold-cache latency: synchronous fetches could stall a page render to N × fetchTimeout when multiple agents are unreachable. Parallelize across boxes with one goroutine per box and a sync.WaitGroup join, so cold-cache renders pay one round of timeout at most. Warm-cache renders hit the cache's read-lock and stay sub-millisecond. Box order is preserved by writing into a positional `keep []bool` and compacting after the wait. `go test -race` clean on services/handler/agent/haproxy packages. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 26b9b06 commit d226ccf

1 file changed

Lines changed: 57 additions & 24 deletions

File tree

gearbox/internal/framework/services/server_adapter.go

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package services
33

44
import (
55
"log/slog"
6+
"sync"
67

78
"github.com/sarg3nt/gearbox/internal/framework/agent"
89
"github.com/sarg3nt/gearbox/internal/framework/services/crypto"
@@ -113,15 +114,22 @@ func (a *ServerAdapter) SetCapabilitiesCache(c *agent.CapabilitiesCache) {
113114

114115
// GetEnabledServersWithGearAvailable returns the subset of enabled servers
115116
// whose probe table reports the named **agent** gear as Available. Used by
116-
// the HAProxy / Logs / Services / OS-Updates dashboard pages to skip
117-
// rendering tiles and tabs for boxes the gear physically can't run on
118-
// (e.g. an agent in a distroless container has no haproxy binary, so
119-
// rendering its HAProxy stats tile produces a 503 storm — issue #112).
117+
// the HAProxy / Logs / OS-Updates dashboard pages to skip rendering tiles
118+
// and tabs for boxes the gear physically can't run on (e.g. an agent in a
119+
// distroless container has no haproxy binary, so rendering its HAProxy
120+
// stats tile produces a 503 storm — issue #112).
120121
//
121-
// The gearName argument is the **agent-side** gear name, not the
122-
// dashboard gear name (see dashboardGearToAgentGear in handler/gears.go
123-
// for the mapping). Pass "haproxy", "logs", "services", "metrics",
124-
// "updates", "certificates", or "traffic" as appropriate.
122+
// The gearName argument is the **agent-side** gear name. The agent's
123+
// registered gears (see [internal/gears] in the gearbox-agent repo) are:
124+
//
125+
// access-log, apache, caddy, certificates, docker, haproxy, host,
126+
// logs, metrics, nginx, security, traefik, traffic, updates
127+
//
128+
// Note that some dashboard gears don't map 1:1 to agent gears — the
129+
// dashboard's "services" / "alerts" / "bx" gears have no agent-side
130+
// counterpart and shouldn't be filtered via this method. See
131+
// dashboardGearToAgentGear in handler/gears.go for the dashboard-side
132+
// mapping that is filtered.
125133
//
126134
// Fail-open contract: a box whose capabilities aren't reachable (agent
127135
// down, no API key, cache miss + fetch failure) is **included** in the
@@ -130,28 +138,53 @@ func (a *ServerAdapter) SetCapabilitiesCache(c *agent.CapabilitiesCache) {
130138
// capability cache hasn't been wired in (SetCapabilitiesCache wasn't
131139
// called), every enabled server is returned — same fail-open posture.
132140
//
141+
// Performance: cold-cache fetches are fired in parallel (one goroutine
142+
// per box) so a render path that calls this on N agents pays one round
143+
// of fetchTimeout latency at most, instead of N × fetchTimeout when
144+
// agents are unreachable. The underlying CapabilitiesCache memoizes
145+
// for capabilityCacheTTL, so steady-state renders are lock-only.
146+
//
133147
// The result is in the same order as GetEnabledServersAsModels.
134148
func (a *ServerAdapter) GetEnabledServersWithGearAvailable(gearName string) []models.BoxConfig {
135149
all := a.GetEnabledServersAsModels()
136-
if a.capabilities == nil || gearName == "" {
150+
if a.capabilities == nil || gearName == "" || len(all) == 0 {
137151
return all
138152
}
153+
154+
// Fetch every box's capabilities in parallel — cold-cache requests
155+
// can each block up to the cache's fetchTimeout, so sequential
156+
// resolution would push page render latency to O(N * timeout) when
157+
// agents are unreachable. Bounded by len(all) goroutines per call;
158+
// the cache itself memoizes, so warm-cache renders take only the
159+
// per-box read-lock latency.
160+
keep := make([]bool, len(all))
161+
var wg sync.WaitGroup
162+
wg.Add(len(all))
163+
for i := range all {
164+
go func(i int) {
165+
defer wg.Done()
166+
srv := all[i]
167+
caps, err := a.capabilities.Get(srv.ID, srv.AgentURL, srv.APIKey)
168+
if err != nil || caps == nil {
169+
// Fail-open: include the box if we can't see its probe table.
170+
keep[i] = true
171+
return
172+
}
173+
entry, present := caps.Entry(gearName)
174+
if !present {
175+
// Agent didn't report this gear at all (older agent that
176+
// pre-dates it). Fail-open — keep the box.
177+
keep[i] = true
178+
return
179+
}
180+
keep[i] = entry.IsAvailable()
181+
}(i)
182+
}
183+
wg.Wait()
184+
139185
out := make([]models.BoxConfig, 0, len(all))
140-
for _, srv := range all {
141-
caps, err := a.capabilities.Get(srv.ID, srv.AgentURL, srv.APIKey)
142-
if err != nil || caps == nil {
143-
// Fail-open: include the box if we can't see its probe table.
144-
out = append(out, srv)
145-
continue
146-
}
147-
entry, present := caps.Entry(gearName)
148-
if !present {
149-
// Agent didn't report this gear at all (older agent that
150-
// pre-dates it). Fail-open — keep the box.
151-
out = append(out, srv)
152-
continue
153-
}
154-
if entry.IsAvailable() {
186+
for i, srv := range all {
187+
if keep[i] {
155188
out = append(out, srv)
156189
}
157190
}

0 commit comments

Comments
 (0)