From bd9e091e65cfbcf2d11ca9e93f596a73a13f7589 Mon Sep 17 00:00:00 2001 From: rouggy Date: Sun, 19 Jul 2026 03:05:02 +0200 Subject: [PATCH] fix: serialise auto-uploads so a burst doesn't 403 and leave QSOs at R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A QSO logged via UDP / ADIF-import fired `go m.upload` per QSO, so a pileup or an ADIF-import burst hit a service with dozens of concurrent requests at once. Club Log's nginx answers that burst with 403; the upload counts as failed and the QSO stays at "R" while a lucky few go through — the real cause behind the "it's uploaded but shows R" report (not the earlier display race). Route all immediate/delayed auto-uploads through a single serialised worker (one at a time, 250ms apart) so no burst ever trips a per-IP rate limiter. upload() now reports whether a failure is transient (HTTP/rate-limit) vs permanent (not eligible, wrong station callsign, no record); transient ones are retried with exponential back-off (up to 4 attempts, 1s/2s/4s), so a QSO that hit a momentary 403 ends up marked instead of stuck at R. A full queue falls back to a goroutine rather than dropping an upload. --- internal/extsvc/manager.go | 105 ++++++++++++++++++++++++++++++------- 1 file changed, 85 insertions(+), 20 deletions(-) diff --git a/internal/extsvc/manager.go b/internal/extsvc/manager.go index f0d7f5e..70e18f9 100644 --- a/internal/extsvc/manager.go +++ b/internal/extsvc/manager.go @@ -87,17 +87,76 @@ type Manager struct { mu sync.Mutex cfg ExternalServices rnd *rand.Rand + + // uploadCh serialises immediate auto-uploads through a single worker. Firing a + // goroutine per QSO meant a pileup / ADIF-import burst hit a service with dozens + // of concurrent requests at once — Club Log's nginx answers that with 403, the + // upload is counted as failed and the QSO stays at "R" despite the others going + // through. One-at-a-time with a small gap keeps every upload under the limit. + uploadCh chan uploadJob } +// uploadJob is one queued auto-upload. +type uploadJob struct { + svc Service + id int64 + cfg ServiceConfig + attempt int // 0 on first try; incremented on each retry +} + +// uploadGap spaces serialized uploads so a burst never trips a service's per-IP +// rate limiter. maxUploadAttempts bounds retries of a transient failure. +const ( + uploadGap = 250 * time.Millisecond + maxUploadAttempts = 4 +) + func NewManager(deps Deps) *Manager { if deps.Client == nil { deps.Client = &http.Client{Timeout: 20 * time.Second} } - return &Manager{ + m := &Manager{ deps: deps, // Seeded from the clock; the delay only needs to be unpredictable // enough to spread bursts, not cryptographically random. - rnd: rand.New(rand.NewSource(time.Now().UnixNano())), + rnd: rand.New(rand.NewSource(time.Now().UnixNano())), + uploadCh: make(chan uploadJob, 4096), + } + go m.uploadWorker() + return m +} + +// uploadWorker drains the queue one upload at a time, spacing them so a burst of +// freshly-logged QSOs can't hammer (and get 403'd by) a service. A transient +// failure is re-queued with an exponential back-off, so a QSO that hit a +// momentary rate-limit still ends up marked instead of stuck at "R". +func (m *Manager) uploadWorker() { + for job := range m.uploadCh { + ok, retryable := m.upload(job.svc, job.id, job.cfg) + if !ok && retryable && job.attempt+1 < maxUploadAttempts { + next := job + next.attempt++ + backoff := time.Duration(1<