Skip to content

Stream restores instead of buffering them in RAM - #79

Merged
kevinmcconnell merged 1 commit into
mainfrom
streaming-restores
Jul 31, 2026
Merged

Stream restores instead of buffering them in RAM#79
kevinmcconnell merged 1 commit into
mainfrom
streaming-restores

Conversation

@kevinmcconnell

@kevinmcconnell kevinmcconnell commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

When restoring a backup, we had been reading the entire archive into memory before extracting the metadata and populating the volume. For a large backup this would be problematic.

Instead, let's stream the contents in, file at a time.

Copilot AI review requested due to automatic review settings July 30, 2026 16:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the backup-restore flow to avoid buffering the entire backup archive in memory by reading required metadata entries first and then streaming the remaining tar entries directly into the restored Docker volume.

Changes:

  • Replace restore parsing that buffered volume contents in RAM with openBackup(...) + a streaming tar.Reader pipeline.
  • Update restore volume population to stream tar entries via io.Pipe() into Docker’s CopyToContainer.
  • Adjust tests to validate the new “metadata-first then streamed entries” restore behavior.

Tip

If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/docker/namespace.go Switch restore path to use the new streaming backup opener instead of buffering parsed volume data.
internal/docker/application_backup.go Introduce openBackup and streaming tar copy helpers; update restore to stream into the volume.
internal/docker/application_backup_test.go Update round-trip test to validate streamed entries after metadata parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/docker/application_backup.go Outdated
Copilot AI review requested due to automatic review settings July 31, 2026 12:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

internal/docker/application_backup.go:375

  • The metadata-first archive contract is not enforced here: this loop accepts and discards arbitrary volume entries until both settings files appear. A backup with metadata at the end therefore decompresses the full data set during this pass and again after the rewind, contrary to the PR's stated design. Reject non-metadata entries until both settings files have been read, and update the test fixture to place metadata first plus cover rejection of misplaced metadata.
	for appData == nil || volData == nil {

@kevinmcconnell
kevinmcconnell merged commit 09421aa into main Jul 31, 2026
7 of 8 checks passed
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