Skip to content

Commit

Permalink
Prevent leaking file descriptor during snapshotting and provide bette…
Browse files Browse the repository at this point in the history
…r logging of errors

Signed-off-by: Marek Siarkowicz <[email protected]>
  • Loading branch information
serathius committed Dec 20, 2024
1 parent 9fa35e5 commit b3a68b9
Showing 1 changed file with 29 additions and 8 deletions.
37 changes: 29 additions & 8 deletions client/v3/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package snapshot
import (
"context"
"crypto/sha256"
"errors"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -54,38 +55,58 @@ func SaveWithVersion(ctx context.Context, lg *zap.Logger, cfg clientv3.Config, d
if err != nil {
return "", err
}
defer cli.Close()
defer func() {
err := cli.Close()
if err != nil {
lg.Error("Failed to close client", zap.Error(err))

Check warning on line 61 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L61

Added line #L61 was not covered by tests
}
}()

partpath := dbPath + ".part"
defer os.RemoveAll(partpath)
defer func() {
err := os.RemoveAll(partpath)
if err != nil {
lg.Error("Failed to cleanup .part file", zap.Error(err))

Check warning on line 69 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L69

Added line #L69 was not covered by tests
}
}()

var f *os.File
f, err = os.OpenFile(partpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileutil.PrivateFileMode)
f, err := os.OpenFile(partpath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, fileutil.PrivateFileMode)
if err != nil {
return "", fmt.Errorf("could not open %s (%w)", partpath, err)
}
defer func() {
err := f.Close()
if err != nil && !errors.Is(err, os.ErrClosed) {
lg.Error("Could not close file descriptor", zap.Error(err))

Check warning on line 80 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L80

Added line #L80 was not covered by tests
}
}()
lg.Info("created temporary db file", zap.String("path", partpath))

start := time.Now()
resp, err := cli.SnapshotWithVersion(ctx)
if err != nil {
return "", err
}
defer resp.Snapshot.Close()
defer func() {
err := resp.Snapshot.Close()
if err != nil {
lg.Error("Could not close snapshot stream", zap.Error(err))

Check warning on line 93 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L93

Added line #L93 was not covered by tests
}
}()
lg.Info("fetching snapshot", zap.String("endpoint", cfg.Endpoints[0]))
var size int64
size, err = io.Copy(f, resp.Snapshot)
if err != nil {
return resp.Version, err
return resp.Version, fmt.Errorf("could not write snapshot: %w", err)

Check warning on line 100 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L100

Added line #L100 was not covered by tests
}
if !hasChecksum(size) {
return resp.Version, fmt.Errorf("sha256 checksum not found [bytes: %d]", size)
}
if err = fileutil.Fsync(f); err != nil {
return resp.Version, err
return resp.Version, fmt.Errorf("could not fsync snapshot: %w", err)

Check warning on line 106 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L106

Added line #L106 was not covered by tests
}
if err = f.Close(); err != nil {
return resp.Version, err
return resp.Version, fmt.Errorf("could not close file descriptor: %w", err)

Check warning on line 109 in client/v3/snapshot/v3_snapshot.go

View check run for this annotation

Codecov / codecov/patch

client/v3/snapshot/v3_snapshot.go#L109

Added line #L109 was not covered by tests
}
lg.Info("fetched snapshot",
zap.String("endpoint", cfg.Endpoints[0]),
Expand Down

0 comments on commit b3a68b9

Please sign in to comment.