fix: follow-ups from the 2026-09-23..30 merge review - #186
Merged
Merged
Conversation
srv.Shutdown never cancelled the request context, so a SIGTERM mid-export left the plaintext dump and pg_service.conf in TMPDIR. The handler now derives its context from the app context and the router removes staging dirs older than 20 minutes at startup.
MarkSent ran after the side effects, so a broker copy on another worker could win it and be streamed as a repeat of the first hearing. It now runs before any further DB work, unconditionally so the set is warm before anyone opts in, and never for TRACE, whose path bytes are per-hop SNR.
Sessions are clean and client IDs random, so deferring acks never bought redelivery; it only capped delivery at the broker's inflight window and pushed backlog into the broker queue, where drops are silent.
Periodic ticks now start five minutes off the task tickers so the hourly reconfirm trigger stops colliding with the cooldown, any capture clears a deferred periodic, subdirectories no longer stop the recorder, and the metadata sidecar is published before the profile.
The four-way GROUP BY over node_short_ids ran once per 1000-route batch, up to 750 times a run, and could eat the whole 5s batch budget. The task now fetches the set once and passes it to each batch.
A transient failure pushed the next attempt out by the full refresh interval. Failures now retry in five minutes, with a server Retry-After still honoured as a ceiling.
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
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 from a review of the PRs merged into dev this week (#149–#185). One commit per fix.
Bugs
database.sqlandpg_service.conf(DB password) in$TMPDIR/beacon-download-*becausesrv.Shutdownnever cancelled the request context. Exports now abort on shutdown (503) and stale staging dirs are swept at startup.MarkSentran after the side effects, so a second broker's copy of the same hearing could be streamed toincludeRepeatsclients asisRepeatover the path they just received. It now runs right after the insert, unconditionally (so the set is warm before anyone opts in), and never for TRACE, whose path bytes are per-hop SNR..jsonsidecar is published before the.pprof.invalid backup archive, notsize limit exceeded.node_short_ids) was recomputed per 1000-route batch, up to 750× a run. It is now fetched once per run and passed to each batch.Retry-Afteris still honoured as a ceiling.scopes:iatas:*andscope:name:*too, so/scopesstops lagging the filter dropdown by up to an hour.untilvalidation (feat(routes): expose bounded retained report evidence #172/fix(observers): provide truthful packet metrics and freshness #169): route evidence and observer activity acceptuntilup to five minutes ahead of server time instead of returning 400 on client clock skew.API shape
RouteEvidenceno longer carries the deadnextCursor;items,hasMoreandnextPageCursorare unchanged on the wire.Observer.statusMetadatais emitted as a JSON object instead of a base64 string (beacon-web keeps a base64 fallback for older servers).CI
TestPacketSummariesPostgresand theTestReconfirm*Postgrestests are now in the Postgres run; the analytics retention test readsGetSignalStats/GetPathStatson the 039 views.Follow-up (not in this PR)
Same-observer broker copies that block on each other's
ON CONFLICTinsert can still raceMarkSent; closing that needs the insert to return the stored path.