NZBGet: warn once per failed history entry, and only for monitored entries in polls - #950
Open
m4bard wants to merge 1 commit into
Open
NZBGet: warn once per failed history entry, and only for monitored entries in polls#950m4bard wants to merge 1 commit into
m4bard wants to merge 1 commit into
Conversation
…tries in polls NZBGet keeps failed entries in history until somebody deletes them, and the history enrichment workflow warned about every failed entry it saw on every poll. A few old failures nobody had cleaned up therefore produced a warning each, per poll, forever. Monitor polls made it worse. They pass the set of download IDs the monitor cares about, but the history scan ignored that set. It walked the whole configured category, logged and merged everything in it, and NzbgetQueueFilter.FilterByIds discarded the surplus afterwards, so most of the warnings were about downloads Listenarr had never submitted. Two changes. The history scan now attributes an entry to a requested ID before it logs or merges it, whenever a poll supplies IDs. That rule sits next to FilterByIds as IsRequestedByIds. It admits a deliberate superset of what FilterByIds keeps, because an entry with no canonical NZBID can be attributed through the active row its title matches and is then dropped later for having an empty ID; erring wide leaves queue output unchanged. Polls that supply no IDs (the queue UI, the removal workflow) still read the whole category, and GetItemsAsync is unchanged. NzbgetFailedHistoryWarningTracker remembers which failed entries have been warned about and reports only first sightings. State is kept per client and per reading surface, and a read updates it according to how much of the history it actually saw. An unscoped read saw the whole category, so it replaces the stored set, and that is what evicts entries which have left history; memory stays bounded by the size of the history itself with no expiry timer, and the queue poller reads unscoped on a timer so eviction keeps happening. A scoped read saw only the downloads it asked about, so absence from it is not evidence of anything and it may only add. Without that split, a monitor poll would drop every entry outside its scope and the next full poll would report them all as new again. The tracker is registered as a singleton because the workflow that reads it is scoped. The warning template is unchanged for a first sighting and repeats log nothing. The state is in memory, so a restart warns once more for whatever is still in history, and an entry that leaves history and comes back warns again. What a user sees: the queue UI poll still reads the whole configured category and now warns once per failed entry instead of once per entry per poll, and monitor polls no longer warn about entries they do not own. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GDr4ZwWYwhX8fAy26Vq5KD
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #951.
NzbgetHistoryEnrichmentWorkflow.AppendHistorycallsLogFailedHistoryEntryfor every history entry that passesIsHistoryCandidate, and that filter only excludes entries outside the configured category, duplicates within one response, and entries still matched by a non-terminal active queue item. An old failed entry with no active match is a candidate on every call, and theprocessedHistoryIdsset is local to the call, so every failed entry in the category warns again on every poll. NZBGet never purges failed history, so on an install whose NZBGet predates Listenarr that is one warning per old failure every five seconds, with no end to it. During monitor polls the scan is also unscoped:monitoredIdsonly gates whether history is read at all, andNzbgetQueueFilter.FilterByIdsdrops the unmonitored entries afterwards, after they have been logged.Two changes:
idsnon-empty), a history entry is a candidate only if it can be attributed to a monitored download: its canonical NZBID is monitored, or the active identity it matches carries a monitored ID. The predicate isNzbgetQueueFilter.IsRequestedByIds, next toFilterByIds, and agrees with it, so nothing that the filter would have kept is dropped early. Non-monitor polls (the queue UI and the removal workflow) keep the full category scan, so the queue page still shows failed history.NzbgetFailedHistoryWarningTracker, a singleton, keeps per client and per surface the set of failed entry keys seen in the most recent full history read and returns the ones that are new. An unscoped read (the queue UI poll, the items surface) replaces the set, which bounds memory by NZBGet's own history size and evicts entries that leave history without needing a timer. A scoped read (a monitor poll with ids) only adds, so the narrow poll cannot evict what the wide poll has already warned about and the two timers do not re-warn each other's entries. The first sighting keeps the existing warning message verbatim; repeats log nothing. After a process restart each entry warns once more per surface. Readarr's NZBGet client logs nothing per history item at all; this keeps the one line that came in with [Architecture] Clarify download client and DDL handoffs #599 and stops the repeats.Tests: six new cases in
NzbgetAdapterTests(warn once across two polls with the item still in both results, re-warn after an entry leaves and returns, monitor poll warns only for the monitored ID and drops the other, non-monitor poll still returns it, wide and narrow queue polls do not re-warn each other's entries, queue and items surfaces do not either) and unit tests for the tracker. Full suite green;dotnet format --verify-no-changesclean.Validated on a running install whose NZBGet carries a few dozen old failed entries from before Listenarr. On a build with this patch, over a thirty-minute window from container start, the log carried zero "NZBGet history reported failure for" lines, against tens of thousands the previous build would have written over the same window; both download clients were still being polled at the same cadence and still reported the same queue, so the scan is still running and has stopped warning. Two things that window did not cover: no genuinely new failure arrived, so the once-per-start behaviour was observed only against pre-existing entries, and only one container start was measured.
One thing to know before merging:
NzbgetHistoryEnrichmentWorkflow.csends up two lines under the 500-line architecture cap, so the next change in that file will have to split something out first.Disclosure: drafted with Claude Code at my direction; I read the cited code at the stated commit and reviewed this before posting.