Fix truncated stdin backups by closing pipe before signaling done#1169
Closed
Fix truncated stdin backups by closing pipe before signaling done#1169
Conversation
53a5d14 to
a963f9a
Compare
Contributor
|
@Kidswiss I better leave this up to you for a review |
b68672a to
f3dd528
Compare
Application-aware backups (stdin backups from pod exec) were being truncated because the pipe writer was closed after signaling done to the backup trigger. This created a race condition: 1. StreamWithContext finishes writing data to the pipe 2. done <- true signals the backup trigger to call cmd.Wait() 3. defer stdoutWriter.Close() hasn't executed yet 4. restic finishes before all data flows through the pipe Fix: close the pipe writer before signaling done, ensuring restic's stdin receives all data and EOF before the backup command exits. Also improved error handling: use CloseWithError on stream failure instead of os.Exit(1), propagating the error through the pipe to the reader. Fixes #1109 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Aarno Aukia <aarno.aukia@vshn.ch>
f3dd528 to
4d25673
Compare
Member
|
Does not fix the issue as the wait on done was unneeded. The code waits for the restic command completion which should only happen when the pipe is closed and read to completion. Superseeded by #1183 which hopefully fixes the issue. |
3 tasks
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.
Summary
Fix a race condition causing application-aware backups (stdin backups from pod exec) to be truncated. Multiple users reported database dumps losing the final few MB of data.
Root cause
In
restic/kubernetes/pod_exec.go, the pipe writer was closed after signaling done:This created a race:
triggerBackup()receives the done signal and callscmd.Wait()on restic before the pipe writer is closed. Restic's stdin may not have received all data and EOF yet.Fix
Close the pipe writer before signaling done:
On stream errors,
CloseWithError()propagates the error through the pipe, thenos.Exit(1)ensures the backup pod fails hard (preserving existing behavior for error cases).Who is affected
Anyone using application-aware backups (
k8up.io/backupcommandannotation) with large outputs (100MB+). Confirmed by multiple users with PostgreSQL and MariaDB dumps.Fixes #1109
Checklist
area:operatorcharts/directoryTest plan
🤖 Generated with Claude Code