Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
beeb5bc31b |
@@ -958,6 +958,36 @@ schema change (reuses `transcript_status` from migration 007).
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## ADR-026 — Foreground caption fetches take priority; the credentials probe is dead
|
||||||
|
|
||||||
|
**Status:** Accepted (2026-06-10). **Pillar A of the manual-mode UX work** (Pillar B was
|
||||||
|
ADR-025). Builds on ADR-014 (the shared per-IP gate).
|
||||||
|
|
||||||
|
**Context.** Every caption fetch — the background sweep and the web click-path — shared one
|
||||||
|
process-wide rate gate equally. So a user waiting on a "Summarize" click competed with the
|
||||||
|
firehose for both pacing and the scarce pre-429 window; on a busy IP the click was slow or
|
||||||
|
429'd while the background churned.
|
||||||
|
|
||||||
|
**Decision.** A context-marked priority lane. The web path
|
||||||
|
(`engineProcessor.ProcessVideo`) wraps its context with `ForegroundContext`; the gate gives
|
||||||
|
foreground fetches a token immediately, while **background fetches yield** — they wait until no
|
||||||
|
foreground fetch is pending before taking a token. Threaded via a context value (not new
|
||||||
|
signatures) and a process-wide `foregroundPending` counter. Clicks are rare and bursty, so the
|
||||||
|
background barely loses throughput; the waiting human gets the next (and cleanest) slot.
|
||||||
|
|
||||||
|
**Credentials probe — rejected, not built.** The idea was to fetch captions with the user's
|
||||||
|
auth in manual mode to dodge 429s. It is a dead end, already settled by ADR-010 and the code:
|
||||||
|
the caption path is *deliberately anonymous* because the InnerTube/timedtext endpoints **reject
|
||||||
|
or break on authenticated requests** (`captions.go`: "no OAuth token — it can break the
|
||||||
|
timedtext endpoint"). The user's OAuth (a Data API credential) does not authenticate InnerTube
|
||||||
|
at all, and the official `captions.download` is owner-only (403 on third-party). So auth cannot
|
||||||
|
help here and can actively hurt. No probe needed — building one would only re-confirm the ADR.
|
||||||
|
|
||||||
|
**Reversibility.** Context-marker + a yield loop in the gate; removing the marker collapses to
|
||||||
|
the prior equal-share behaviour. No schema or API change.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
## Rejected alternatives
|
## Rejected alternatives
|
||||||
|
|
||||||
Approaches considered during the 2026-06-02 planning + grill session and **deliberately not
|
Approaches considered during the 2026-06-02 planning + grill session and **deliberately not
|
||||||
|
|||||||
@@ -113,6 +113,11 @@ type engineProcessor struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (p *engineProcessor) ProcessVideo(ctx context.Context, userID, videoID string) error {
|
func (p *engineProcessor) ProcessVideo(ctx context.Context, userID, videoID string) error {
|
||||||
|
// This is the user-initiated (foreground) path — a click on "Summarize",
|
||||||
|
// "Try now", or a pasted URL. Mark the context so the caption gate gives it
|
||||||
|
// priority over the background sweep (ADR-026, Pillar A).
|
||||||
|
ctx = youtube.ForegroundContext(ctx)
|
||||||
|
|
||||||
row, err := p.store.GetVideoRow(ctx, userID, videoID)
|
row, err := p.store.GetVideoRow(ctx, userID, videoID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("load video %q: %w", videoID, err)
|
return fmt.Errorf("load video %q: %w", videoID, err)
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package youtube
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"golang.org/x/time/rate"
|
"golang.org/x/time/rate"
|
||||||
@@ -29,9 +30,55 @@ func SetFetchRate(interval time.Duration) {
|
|||||||
globalFetchGate = rate.NewLimiter(rate.Every(interval), 1)
|
globalFetchGate = rate.NewLimiter(rate.Every(interval), 1)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// foregroundPending counts in-flight foreground (user-initiated) caption fetches.
|
||||||
|
// The background sweep yields the gate while this is non-zero so a human waiting
|
||||||
|
// on a click gets the next slot — and, on a near-throttled IP, the pre-429 window
|
||||||
|
// — instead of competing equally with the firehose (ADR-026, Pillar A). Clicks are
|
||||||
|
// rare and bursty, so background barely notices; the win to the click is large.
|
||||||
|
var foregroundPending atomic.Int64
|
||||||
|
|
||||||
|
// fgCtxKey marks a context as foreground (user-initiated). Unexported; set via
|
||||||
|
// ForegroundContext and read via isForeground so only this package owns the key.
|
||||||
|
type fgCtxKey struct{}
|
||||||
|
|
||||||
|
// ForegroundContext marks ctx as a user-initiated (foreground) fetch so the gate
|
||||||
|
// gives it priority. The web "Summarize"/paste/retry path wraps its context with
|
||||||
|
// this; the background scheduler leaves it unset.
|
||||||
|
func ForegroundContext(ctx context.Context) context.Context {
|
||||||
|
return context.WithValue(ctx, fgCtxKey{}, true)
|
||||||
|
}
|
||||||
|
|
||||||
|
func isForeground(ctx context.Context) bool {
|
||||||
|
v, _ := ctx.Value(fgCtxKey{}).(bool)
|
||||||
|
return v
|
||||||
|
}
|
||||||
|
|
||||||
|
// fgYieldPoll is how often a background waiter re-checks whether a foreground
|
||||||
|
// fetch is still pending. Short enough to feel immediate, long enough not to spin.
|
||||||
|
const fgYieldPoll = 200 * time.Millisecond
|
||||||
|
|
||||||
// WaitFetchGate blocks until the process-wide gate allows one timedtext fetch,
|
// WaitFetchGate blocks until the process-wide gate allows one timedtext fetch,
|
||||||
// respecting ctx cancellation. Called from httpDo before every live outbound
|
// respecting ctx cancellation. Called from httpDo before every live outbound
|
||||||
// caption fetch so the scheduler and the click-path share the same egress budget.
|
// caption fetch so the scheduler and the click-path share the same egress budget.
|
||||||
|
//
|
||||||
|
// Foreground (user-initiated) fetches take priority: they register as pending and
|
||||||
|
// acquire a token immediately. Background fetches first yield — they wait until no
|
||||||
|
// foreground fetch is pending — so a live click is never stuck behind the
|
||||||
|
// background sweep and gets the cleaner slot against the per-IP limit (ADR-026).
|
||||||
func WaitFetchGate(ctx context.Context) error {
|
func WaitFetchGate(ctx context.Context) error {
|
||||||
|
if isForeground(ctx) {
|
||||||
|
foregroundPending.Add(1)
|
||||||
|
defer foregroundPending.Add(-1)
|
||||||
|
return globalFetchGate.Wait(ctx)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Background: defer to any pending foreground fetch before taking a token.
|
||||||
|
for foregroundPending.Load() > 0 {
|
||||||
|
select {
|
||||||
|
case <-ctx.Done():
|
||||||
|
return ctx.Err()
|
||||||
|
case <-time.After(fgYieldPoll):
|
||||||
|
}
|
||||||
|
}
|
||||||
return globalFetchGate.Wait(ctx)
|
return globalFetchGate.Wait(ctx)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -82,3 +82,37 @@ func TestSetFetchRateZeroIsUnlimited(t *testing.T) {
|
|||||||
require.NoError(t, WaitFetchGate(context.Background()))
|
require.NoError(t, WaitFetchGate(context.Background()))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestForegroundContextMarker(t *testing.T) {
|
||||||
|
require.False(t, isForeground(context.Background()), "plain context is background")
|
||||||
|
require.True(t, isForeground(ForegroundContext(context.Background())), "marked context is foreground")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestWaitFetchGateForegroundProceedsImmediately: a foreground fetch acquires a
|
||||||
|
// token without yielding, even when background callers exist.
|
||||||
|
func TestWaitFetchGateForegroundProceedsImmediately(t *testing.T) {
|
||||||
|
SetFetchRate(0) // unlimited limiter — isolate the yield logic from pacing
|
||||||
|
foregroundPending.Store(0)
|
||||||
|
t.Cleanup(func() { foregroundPending.Store(0) })
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||||
|
defer cancel()
|
||||||
|
require.NoError(t, WaitFetchGate(ForegroundContext(ctx)), "foreground proceeds immediately")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestWaitFetchGateBackgroundYieldsToForeground: while a foreground fetch is
|
||||||
|
// pending, a background fetch yields (does not take a token) until the foreground
|
||||||
|
// clears — proven by a background wait timing out against its own deadline, then
|
||||||
|
// succeeding once the foreground is done.
|
||||||
|
func TestWaitFetchGateBackgroundYieldsToForeground(t *testing.T) {
|
||||||
|
SetFetchRate(0)
|
||||||
|
foregroundPending.Store(1) // simulate a foreground fetch in flight
|
||||||
|
t.Cleanup(func() { foregroundPending.Store(0) })
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond)
|
||||||
|
defer cancel()
|
||||||
|
require.Error(t, WaitFetchGate(ctx), "background yields (blocks) while foreground is pending")
|
||||||
|
|
||||||
|
foregroundPending.Store(0) // foreground done
|
||||||
|
require.NoError(t, WaitFetchGate(context.Background()), "background proceeds once foreground clears")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user