bug(api): singleflight on /v1/query has no clean context-cancellation choice #120

Problem

internal/api/query.go QueryHandler.Handle wraps the per-request ClickHouse execution in singleflight.Group.Do(cacheKey, ...) so that N concurrent identical queries collapse to one round-trip. The function inside Do builds queryCtx from r.Context() plus a 30s timeout. There's no clean choice for which request's context drives that work:

This was raised in PR #116 review (Gemini flagged the WithoutCancel change as a resource-exhaustion risk; reverting to r.Context() introduces the multi-waiter regression). Either choice is wrong for some shape of traffic.

Proposed Solution

Track active waiter count for each singleflight key. The query context should cancel only when the count drops to zero — i.e. when every caller has actually given up.

Sketch:

type waiterTracker struct {
    mu    sync.Mutex
    refs  map[string]int
    cancels map[string]context.CancelFunc
}

// On entry:
//   - inc refs[key]; if first, create cancellable detached ctx + store cancel
//   - register a goroutine watching r.Context().Done() that decs refs and
//     calls cancel only when refs hits 0
// On exit (singleflight returns):
//   - cleanup map entry

This matches the singleflight invariant: one in-flight execution per key, alive as long as any waiter is interested. Implementation can be local to QueryHandler — no library swap needed.

Alternatives Considered

• Bound the work-after-disconnect window (e.g. detached ctx with 5s grace instead of 30s). Reduces but doesn't eliminate either failure mode; just trades them off.
• Drop singleflight entirely for /v1/query. Removes the multi-waiter problem but reintroduces thundering-herd against ClickHouse for popular queries — the original reason singleflight was added.
• Switch to a request-coalescing cache layer (e.g. groupcache-style). Heavier and arguably overkill for a single ClickHouse-fronting endpoint.
• Status quo (r.Context()). Accept the multi-waiter regression as the lesser evil, document it. Reasonable if measurement shows multi-waiter collapse is rare in practice; needs a metric to confirm.

Additional Context

Surfaced during PR #116 review (Claude + Gemini both touched the area). Current code (post-PR-#116) uses r.Context() directly — the multi-waiter regression is live but not yet observed in production. Adding a wavehouse_query_singleflight_collapsed_total counter would tell us how often the collapse actually fires before deciding how much to invest here.