Skip to content

Retry failed snapshot submission - #822

Open
seanlinsley wants to merge 3 commits into
mainfrom
retry-queue
Open

Retry failed snapshot submission#822
seanlinsley wants to merge 3 commits into
mainfrom
retry-queue

Conversation

@seanlinsley

@seanlinsley seanlinsley commented Jun 9, 2026

Copy link
Copy Markdown
Member

Snapshots are now submitted to a Queue that retains recent snapshots if submission fails. This significantly reduces the risk of data loss when there are temporary network connectivity or API availability issues.

return "", fmt.Errorf("Error - can't upload without valid S3 grant")
}

if grant.S3URL == "" && grant.LocalDir != "" {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is LocalDir still in use?

if testRun {
requestURL = server.Config.APIBaseURL + "/v2/snapshots/test"
} else if compact {
requestURL = server.Config.APIBaseURL + "/v2/snapshots/compact"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dropped this function primarily so that the snapshot UUID and collected timestamp no longer need to be retained. /v2/snapshots/compact is no longer used, but what about /v2/snapshots/test?

@lfittl

lfittl commented Jun 23, 2026

Copy link
Copy Markdown
Member

As discussed, an open question here is whether we should store this in temp files or not - if we can reliably assume that full snapshots are only a handful of MBs in size (which I think is unlikely) it might be fine to stay in memory, otherwise we should utilize temp files (at least when the snapshot is big) to avoid high peak memory use when the server side is temporarily unavailable.

We could also consider having a hard limit of how much memory is used across all servers on the collector for this purpose to avoid OOMs, if there is a good reason to not use temp files.

@seanlinsley
seanlinsley marked this pull request as draft July 13, 2026 13:17
@seanlinsley
seanlinsley force-pushed the retry-queue branch 2 times, most recently from 06b2f9f to 95f6a81 Compare July 13, 2026 17:53
@seanlinsley
seanlinsley marked this pull request as ready for review July 13, 2026 17:53
@seanlinsley

Copy link
Copy Markdown
Member Author

This PR was essentially rewritten. Notable changes:

  • All servers now respect a global 200 MB max queue size.
  • A single queue per server is used to avoid significant complexity needed to coordinate two separate queues (previously there were separate queues for compact snapshots and full snapshots). As a consequence the max per-server queue size was increased from 10 to 200 to ensure we retain at least one full snapshot for a reasonable amount of time.
  • Snapshot submission over the websocket now returns an error, so we're able to correctly retain snapshots in the queue.
  • There are now tests to validate the queue's behavior.

@seanlinsley
seanlinsley requested a review from a team July 13, 2026 18:13
@seanlinsley seanlinsley changed the title Retry failed snapshot submission using internal queue Retry failed snapshot submissions Jul 14, 2026
@seanlinsley seanlinsley changed the title Retry failed snapshot submissions Retry failed snapshot submission Jul 15, 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.

2 participants