Skip to content

fix(upload): stop spamming when flight-review is offline - #30

Closed
dakejahl wants to merge 1 commit into
feat/mavlink-ftp-onlyfrom
feat/upload-backoff
Closed

fix(upload): stop spamming when flight-review is offline#30
dakejahl wants to merge 1 commit into
feat/mavlink-ftp-onlyfrom
feat/upload-backoff

Conversation

@dakejahl

Copy link
Copy Markdown
Collaborator

Summary

Second of three stacked PRs replacing #28. Based on #29review that one first, and this diff will shrink to a single commit once #29 merges.

Carries over 7a23c8e from #28 unchanged, rebased onto the FTP work. Authorship preserved.

Problem

When the local flight-review server is down, the upload loop probes it and logs a failure for every pending log. With a queue of logs and :5006 not running, one pass produces a wall of identical Connection to ... failed lines and a TEMPORARILY FAILED per log.

Worse, the loop is while (num_logs_to_upload()) and a failed upload does not reduce that count, so a dead server means an unbounded hot loop hammering it as fast as the connection attempts return.

Solution

Probe at most once per 60s cooldown, log only on the down/up transitions, and give the reachability failure an explicit 503 so the caller can bail the batch instead of walking the queue. Connect/read timeouts on the probe keep a black-holed server from stalling the thread.

Measured on hardware in #28: 1 unreachable line instead of a per-log wall, 0 TEMPORARILY FAILED.

When the local flight-review server is down, the upload loop previously
probed and logged a failure for every pending log. Probe once, log a
single unreachable message with a 60s cooldown, and bail the current
upload batch (HTTP 503) instead of walking the queue.

(cherry picked from commit 7a23c8e)
@dakejahl

Copy link
Copy Markdown
Collaborator Author

Superseded by #32, which contains this branch's commits unchanged and with authorship intact — merging #32 merges this.

Reviewing this stack turned up structural problems in the code underneath it, and fixing those meant rewriting the parts these PRs sit on. Keeping them open would have meant reviewing #29's ServerInterface changes and then reviewing their replacement in the same sitting, so it is one branch now.

Nothing here is dropped: the FTP transport, the reachability cooldown and the API-key headers are all in #32. The one deliberate change is that 401/403 no longer blacklist a log — see the note in #32.

@dakejahl dakejahl closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant