From 816c6ffcf1d64c1ed27f8e97e1b6e2be1b23d799 Mon Sep 17 00:00:00 2001
From: rouggy <legreg002@hotmail.com>
Date: Sun, 19 Jul 2026 17:41:48 +0200
Subject: [PATCH] =?UTF-8?q?fix:=20revert=20upload=20serialisation=20?=
 =?UTF-8?q?=E2=80=94=20it=20stranded=20QSOs=20at=20R=20(regression)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The single-worker upload queue introduced head-of-line blocking: one slow
upload (LoTW signing via TQSL, or a service on a 30 s timeout) stalled the
whole queue, so every following QSO's Club Log upload waited and the rows sat
at "R". Operators on the older, concurrent build had no such problem — the
tell that this was a fresh regression.

Back to one goroutine per upload (never serialised), so a slow upload never
holds up the rest, while keeping the transient-failure retry with back-off
(concurrent, so it can't block anything).
---
 internal/extsvc/manager.go | 86 +++++++++++---------------------------
 1 file changed, 24 insertions(+), 62 deletions(-)

diff --git a/internal/extsvc/manager.go b/internal/extsvc/manager.go
index 70e18f9..5b6c481 100644
--- a/internal/extsvc/manager.go
+++ b/internal/extsvc/manager.go
@@ -87,77 +87,39 @@ 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
-)
+// maxUploadAttempts bounds retries of a transient upload failure.
+const maxUploadAttempts = 4
 
 func NewManager(deps Deps) *Manager {
 	if deps.Client == nil {
 		deps.Client = &http.Client{Timeout: 20 * time.Second}
 	}
-	m := &Manager{
+	return &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())),
-		uploadCh: make(chan uploadJob, 4096),
+		rnd: rand.New(rand.NewSource(time.Now().UnixNano())),
 	}
-	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<<uint(job.attempt)) * time.Second // 1s, 2s, 4s…
-			m.logf("extsvc: %s upload of QSO %d will retry (attempt %d) in %s", job.svc, job.id, next.attempt+1, backoff)
-			time.AfterFunc(backoff, func() { m.enqueueJob(next) })
+// attemptUpload uploads a QSO in its OWN goroutine and, on a TRANSIENT failure
+// (rate-limit / network), re-arms itself with exponential back-off. Each upload is
+// independent — never serialised through a shared worker, because a single slow
+// upload (LoTW signs via TQSL; a service on a 30 s timeout) would otherwise block
+// every following QSO's upload and strand them all at "R" (the regression that hit
+// the operator on the newest build while everyone on the old concurrent path was
+// fine).
+func (m *Manager) attemptUpload(svc Service, id int64, cfg ServiceConfig, attempt int) {
+	go func() {
+		ok, retryable := m.upload(svc, id, cfg)
+		if !ok && retryable && attempt+1 < maxUploadAttempts {
+			backoff := time.Duration(1<<uint(attempt)) * time.Second // 1s, 2s, 4s…
+			m.logf("extsvc: %s upload of QSO %d will retry (attempt %d) in %s", svc, id, attempt+2, backoff)
+			time.AfterFunc(backoff, func() { m.attemptUpload(svc, id, cfg, attempt+1) })
 		}
-		time.Sleep(uploadGap)
-	}
-}
-
-// enqueueUpload queues a first-attempt upload without blocking the logging path.
-func (m *Manager) enqueueUpload(svc Service, id int64, cfg ServiceConfig) {
-	m.enqueueJob(uploadJob{svc: svc, id: id, cfg: cfg})
-}
-
-// enqueueJob queues a job (possibly a retry). If the queue is somehow full (an
-// enormous burst), it falls back to a goroutine rather than dropping the upload —
-// a dropped upload would leave the QSO stuck at "R".
-func (m *Manager) enqueueJob(job uploadJob) {
-	select {
-	case m.uploadCh <- job:
-	default:
-		go m.upload(job.svc, job.id, job.cfg)
-	}
+	}()
 }
 
 func (m *Manager) logf(format string, args ...any) {
@@ -234,17 +196,17 @@ func (m *Manager) route(svc Service, id int64, cfg ServiceConfig) {
 	m.scheduleUpload(svc, id, cfg)
 }
 
-// scheduleUpload either queues the upload now (immediate) or arms a timer that
-// queues it later (delayed). Both go through the serialised worker so uploads are
-// never fired concurrently in a burst.
+// scheduleUpload uploads now (immediate) or after a random fuse (delayed). Each
+// upload runs in its own goroutine (attemptUpload) — never serialised — so a slow
+// one never holds up the rest.
 func (m *Manager) scheduleUpload(svc Service, id int64, cfg ServiceConfig) {
 	if cfg.UploadMode == ModeDelayed {
 		d := m.delaySeconds()
 		m.logf("extsvc: %s upload of QSO %d scheduled in %s", svc, id, d)
-		time.AfterFunc(d, func() { m.enqueueUpload(svc, id, cfg) })
+		time.AfterFunc(d, func() { m.attemptUpload(svc, id, cfg, 0) })
 		return
 	}
-	m.enqueueUpload(svc, id, cfg)
+	m.attemptUpload(svc, id, cfg, 0)
 }
 
 // onCloseServices returns the services configured for on-close auto-upload,