From e01fc39abcada601f9718fc1614b53f69878bbcc Mon Sep 17 00:00:00 2001 From: rouggy Date: Sun, 23 Aug 2026 15:37:49 +0200 Subject: [PATCH] fix(delete): stop asking Club Log to delete QSOs it never had Deleting a selection with 'delete from the remote services' on took minutes. Club Log was called for EVERY deleted QSO, uploaded or not, and for the ones it never had it answers 403 -- a full network round trip per contact, in series, on the goroutine the UI was waiting on. QRZ already skipped records with no stored logid; Club Log had no equivalent guard. Three changes, each fixing one part of the delay: - only contacts whose clublog upload status is Y or M are withdrawn; - the withdrawals run in the background, after a snapshot -- the local log is the operator's own copy and must not wait on two websites; - the hooks read the rows in one query instead of one per id, which on a remote MySQL logbook was 2 round trips per QSO before the DELETE. Consecutive refusals now abort the run: Club Log blocks an account that keeps sending requests it refuses, so working through the rest of the selection only earns a longer block. --- app.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++---- changelog.json | 6 ++-- syncfolder.go | 10 +++--- 3 files changed, 85 insertions(+), 13 deletions(-) diff --git a/app.go b/app.go index 46679ed..2796187 100644 --- a/app.go +++ b/app.go @@ -6451,6 +6451,26 @@ func (a *App) DeleteQSO(id int64) error { // Failures never block the local delete. The operator asked for this QSO to go; // a refusing website is not a reason to keep it, and the log says what happened // so it can be sorted out on the site afterwards. +// deleteSnapshot reads the rows behind a set of ids in ONE query. +// +// The delete hooks used to fetch them one by one. That is a round trip per +// contact, and on a remote MySQL logbook 200 deletions became 400 sequential +// queries before the DELETE was even issued — the reason deleting a selection +// felt broken rather than slow. +func (a *App) deleteSnapshot(ids []int64) []qso.QSO { + if a.qso == nil || len(ids) == 0 { + return nil + } + out := make([]qso.QSO, 0, len(ids)) + if err := a.qso.IterateByIDs(a.ctx, ids, func(q qso.QSO) error { + out = append(out, q) + return nil + }); err != nil { + applog.Printf("delete: reading %d QSO(s) before deletion failed: %v", len(ids), err) + } + return out +} + func (a *App) deleteRemoteCopies(ids []int64) { if a.qso == nil || len(ids) == 0 { return @@ -6469,9 +6489,37 @@ func (a *App) deleteRemoteCopies(ids []int64) { if !doQRZ && !doClublog { return } - for _, id := range ids { - q, err := a.qso.GetByID(a.ctx, id) - if err != nil || q.Callsign == "" { + // Snapshot first — the rows are about to disappear — then withdraw them in + // the background. Each service is an HTTP round trip PER CONTACT, so doing + // this on the caller's goroutine held the whole UI for as long as the + // websites took: minutes for a large selection, with nothing on screen + // saying why. The local log is the operator's own copy and must not wait on + // two remote sites to answer. + rows := a.deleteSnapshot(ids) + go a.withdrawRemoteCopies(rows, cfg, doQRZ, doClublog) +} + +// maxClublogRefusals is how many consecutive refusals are tolerated before the +// withdrawals stop. Three is enough to tell a one-off from a blocked account. +const maxClublogRefusals = 3 + +// clublogWasUploaded reports whether this QSO ever reached Club Log. "M" +// (modified since upload) counts: the copy is there, it is merely out of date. +func clublogWasUploaded(status string) bool { + switch strings.ToUpper(strings.TrimSpace(status)) { + case "Y", "M": + return true + } + return false +} + +func (a *App) withdrawRemoteCopies(rows []qso.QSO, cfg extsvc.ExternalServices, doQRZ, doClublog bool) { + started := time.Now() + clublogRefused := 0 + for i := range rows { + q := rows[i] + id := q.ID + if q.Callsign == "" { continue } if doQRZ { @@ -6490,14 +6538,31 @@ func (a *App) deleteRemoteCopies(ids []int64) { applog.Printf("extsvc: QRZ delete of QSO %d (%s): %s", id, q.Callsign, msg) } } - if doClublog { + // Only ask Club Log about contacts it was actually given. A QSO that was + // never uploaded has no copy to withdraw, and asking anyway is not merely + // pointless: Club Log answers 403 and starts blocking the account, so a + // selection of never-uploaded QSOs turned into hundreds of refused + // requests, one after another, each waiting on the network. + if doClublog && !clublogWasUploaded(q.ClublogUploadStatus) { + applog.Printf("extsvc: QSO %d (%s) was never uploaded to Club Log — nothing to withdraw", id, q.Callsign) + } else if doClublog { if msg, err := extsvc.DeleteClublog(a.ctx, nil, cfg.Clublog, q.Callsign, q.QSODate, q.Band); err != nil { + clublogRefused++ applog.Printf("extsvc: Club Log delete of QSO %d (%s) failed: %v", id, q.Callsign, err) + // Club Log blocks an account that keeps sending requests it + // refuses. Stop after a few in a row rather than work through + // the whole selection earning a longer block. + if clublogRefused >= maxClublogRefusals { + applog.Printf("extsvc: Club Log refused %d deletions in a row — giving up on the rest", clublogRefused) + doClublog = false + } } else { + clublogRefused = 0 applog.Printf("extsvc: Club Log delete of QSO %d (%s): %s", id, q.Callsign, msg) } } } + applog.Printf("extsvc: withdrew %d deleted QSO(s) from the remote services in %s", len(rows), time.Since(started).Round(time.Millisecond)) } // DeleteQSOs removes several QSOs at once (multi-row selection). Returns the @@ -6506,16 +6571,21 @@ func (a *App) DeleteQSOs(ids []int64) (int64, error) { if a.qso == nil { return 0, fmt.Errorf("db not initialized") } + started := time.Now() a.deleteRemoteCopies(ids) a.syncPublishDeletes(ids) + beforeDelete := time.Since(started) n, err := a.qso.DeleteMany(a.ctx, ids) // Logged because a delete that does nothing is otherwise indistinguishable // from a delete that worked: the rows leave the grid either way once it // reloads, and the only place the truth survives is here. if err != nil { - applog.Printf("delete: %d QSO(s) requested, FAILED: %v", len(ids), err) + applog.Printf("delete: %d QSO(s) requested, FAILED after %s: %v", len(ids), time.Since(started).Round(time.Millisecond), err) } else { - applog.Printf("delete: %d QSO(s) requested, %d row(s) removed", len(ids), n) + // Both timings, because they blame different things: a long prelude is + // the sync/withdraw hooks, a long DELETE is the database itself. + applog.Printf("delete: %d QSO(s) requested, %d row(s) removed — %s before the delete, %s total", + len(ids), n, beforeDelete.Round(time.Millisecond), time.Since(started).Round(time.Millisecond)) } return n, err } diff --git a/changelog.json b/changelog.json index 1e0a4e3..2d78d20 100644 --- a/changelog.json +++ b/changelog.json @@ -6,13 +6,15 @@ "Edit QSO: HAMLOG.online joins the QSL Info tab — its own channel in the picker, with sent/received and both dates, and a row in the status table. Its four columns also move from the QSL group to Uploads, next to Club Log and QRZ.com.", "The filter can now match on when a QSO was added to the log ('Added to the log on'), which is the only way to isolate what an import brought in — an import of old contacts carries old QSO dates and otherwise hides inside the log.", "QSL Manager: HAMLOG.online gains 'Import confirmations (ADIF)…'. Their site exports a log, this reads it back and stamps the confirmations on QSOs already present — it never inserts. Importing that same file through the ordinary ADIF import does insert, because it matches on the UTC minute and their export rarely agrees to the minute.", - "Deleting QSOs now says how many rows were actually removed, and writes it to the log. A delete that removes nothing used to look exactly like one that worked." + "Deleting QSOs now says how many rows were actually removed, and writes it to the log. A delete that removes nothing used to look exactly like one that worked.", + "Deleting QSOs is fast again when 'delete from the remote services' is on. Club Log is only asked about contacts that were actually uploaded to it — asking about the others earned a 403 each time, one network round trip per QSO — the withdrawals now run in the background instead of holding the window, the rows behind a selection are read in one query rather than one per QSO, and repeated refusals stop the run rather than earning a longer block." ], "fr": [ "Édition de QSO : HAMLOG.online rejoint l'onglet QSL Info — son propre canal dans la liste, avec envoyé/reçu et les deux dates, et une ligne dans le tableau de statut. Ses quatre colonnes passent aussi du groupe QSL au groupe Uploads, à côté de Club Log et QRZ.com.", "Le filtre peut maintenant porter sur la date d'ajout au journal (« Ajouté au journal le »), seul moyen d'isoler ce qu'un import a apporté : un import de vieux contacts porte de vieilles dates de QSO et se fond sinon dans le journal.", "Gestionnaire QSL : HAMLOG.online reçoit « Importer les confirmations (ADIF)… ». Leur site exporte un log, cette fonction le relit et appose les confirmations sur les QSO déjà présents — elle n'ajoute jamais rien. Le même fichier passé par l'import ADIF ordinaire, lui, ajoute : il compare à la minute UTC près et leur export s'accorde rarement à la minute.", - "La suppression de QSO indique maintenant combien de lignes ont réellement été supprimées, et l'écrit dans le journal. Une suppression sans effet ressemblait exactement à une suppression réussie." + "La suppression de QSO indique maintenant combien de lignes ont réellement été supprimées, et l'écrit dans le journal. Une suppression sans effet ressemblait exactement à une suppression réussie.", + "La suppression de QSO redevient rapide quand « supprimer aussi des services externes » est activé. Club Log n'est plus interrogé que pour les contacts qui y ont réellement été envoyés — pour les autres il répondait 403, un aller-retour réseau par QSO — les retraits se font désormais en arrière-plan au lieu de bloquer la fenêtre, les lignes d'une sélection sont lues en une seule requête au lieu d'une par QSO, et une série de refus interrompt le traitement au lieu d'aggraver le blocage." ] }, { diff --git a/syncfolder.go b/syncfolder.go index f10747c..aa09e32 100644 --- a/syncfolder.go +++ b/syncfolder.go @@ -315,11 +315,11 @@ func (a *App) syncPublishDeletes(ids []int64) { if store == nil { return } - for _, id := range ids { - q, err := a.qso.GetByID(a.ctx, id) - if err != nil { - continue - } + // One read for the whole set: see deleteSnapshot. Tombstones are written + // before the rows go, because a QSO that never had a sync identity has to be + // given one here — after the DELETE there would be nothing left to stamp. + for _, q := range a.deleteSnapshot(ids) { + id := q.ID // A contact never touched since the sync was switched on has no identity, // and giving it one now is what makes the deletion addressable at all. a.syncMu.Lock()