Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 26 additions & 5 deletions encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ import (
"fmt"
"io"
"os"

"github.com/google/renameio"
"path/filepath"
)

// errorEncoder is a helper type to encode multiple values
Expand Down Expand Up @@ -300,12 +299,22 @@ func LoadSavedGraph[K cmp.Ordered](path string) (*SavedGraph[K], error) {
}

// Save writes the graph to the file.
//
// The graph is written to a temporary file in the same directory, fsynced, and
// renamed over the target, so an interrupted save leaves the previous graph in
// place rather than a truncated one. os.Rename replaces an existing file on
// every platform Go supports, Windows included.
func (g *SavedGraph[K]) Save() error {
tmp, err := renameio.TempFile("", g.Path)
tmp, err := os.CreateTemp(filepath.Dir(g.Path), filepath.Base(g.Path)+".tmp-*")
if err != nil {
return err
}
defer tmp.Cleanup()
tmpName := tmp.Name()
defer func() {
// Both are no-ops once the rename below has succeeded.
_ = tmp.Close()
_ = os.Remove(tmpName)
}()

wr := bufio.NewWriter(tmp)
err = g.Export(wr)
Expand All @@ -318,7 +327,19 @@ func (g *SavedGraph[K]) Save() error {
return fmt.Errorf("flushing: %w", err)
}

err = tmp.CloseAtomicallyReplace()
// Before the rename, not after: a rename that reaches the disk ahead of the
// data it points at is what loses both graphs instead of one.
err = tmp.Sync()
if err != nil {
return fmt.Errorf("syncing: %w", err)
}

err = tmp.Close()
if err != nil {
return fmt.Errorf("closing: %w", err)
}

err = os.Rename(tmpName, g.Path)
if err != nil {
return fmt.Errorf("closing atomically: %w", err)
}
Expand Down
40 changes: 40 additions & 0 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package hnsw
import (
"bytes"
"cmp"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -231,3 +233,41 @@ func BenchmarkGraph_Export(b *testing.B) {
buf.Reset()
}
}

// TestSavedGraph_ReplacesAndLeavesNoTempFiles covers what the atomic write is
// for: a second Save must replace the first graph rather than fail on an
// existing file, and it must not leave its temporary file behind.
//
// Both are worth asserting because os.Rename over an existing file is the one
// part of this that is platform-dependent — it is a plain rename on Unix and
// MoveFileEx with MOVEFILE_REPLACE_EXISTING on Windows.
func TestSavedGraph_ReplacesAndLeavesNoTempFiles(t *testing.T) {
dir := t.TempDir()
path := filepath.Join(dir, "graph")

g1, err := LoadSavedGraph[int](path)
require.NoError(t, err)
for i := 0; i < 8; i++ {
g1.Add(Node[int]{i, randFloats(1)})
}
require.NoError(t, g1.Save())

// The second save writes over a file that already exists.
for i := 8; i < 24; i++ {
g1.Add(Node[int]{i, randFloats(1)})
}
require.NoError(t, g1.Save())

g2, err := LoadSavedGraph[int](path)
require.NoError(t, err)
require.Equal(t, 24, g2.Len(), "the second save should have replaced the first")
requireGraphApproxEquals(t, g1.Graph, g2.Graph)

entries, err := os.ReadDir(dir)
require.NoError(t, err)
names := make([]string, 0, len(entries))
for _, e := range entries {
names = append(names, e.Name())
}
require.Equal(t, []string{"graph"}, names, "no temporary file should survive a save")
}
5 changes: 1 addition & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@ module github.com/coder/hnsw

go 1.21.4

require (
github.com/google/renameio v1.0.1
github.com/stretchr/testify v1.9.0
)
require github.com/stretchr/testify v1.9.0

require (
github.com/chewxy/math32 v1.10.1 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ github.com/chewxy/math32 v1.10.1/go.mod h1:dOB2rcuFrCn6UHrze36WSLVPKtzPMRAQvBvUw
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/google/renameio v1.0.1 h1:Lh/jXZmvZxb0BBeSY5VKEfidcbcbenKjZFzM/q0fSeU=
github.com/google/renameio v1.0.1/go.mod h1:t/HQoYBZSsWSNK35C6CO/TpPLDVWvxOHboWUAweKUpk=
github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
Expand Down
Loading