Skip to content

Commit d0608a9

Browse files
corylanouclaude
andcommitted
fix(vfs): improve temp file deletion and level-1 position initialization
- Refactor Delete() error handling for clearer control flow - Ignore ErrNotExist when removing temp files from disk - Seed maxTXID1 during Open() based on restore plan for correct dual-polling - Add tests for Open() position seeding and Delete() edge cases 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 21f6cb3 commit d0608a9

File tree

2 files changed

+125
-7
lines changed

2 files changed

+125
-7
lines changed

vfs.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,13 @@ func (vfs *VFS) openMainDB(name string, flags sqlite3vfs.OpenFlag) (sqlite3vfs.F
7777

7878
func (vfs *VFS) Delete(name string, dirSync bool) error {
7979
slog.Info("deleting file", "name", name, "dirSync", dirSync)
80-
if err := vfs.deleteTempFile(name); err == nil {
81-
return nil
82-
} else if !errors.Is(err, os.ErrNotExist) && !errors.Is(err, errTempFileNotFound) {
80+
if err := vfs.deleteTempFile(name); err != nil {
81+
if errors.Is(err, os.ErrNotExist) || errors.Is(err, errTempFileNotFound) {
82+
return nil
83+
}
8384
return err
8485
}
85-
return fmt.Errorf("cannot delete vfs file")
86+
return nil
8687
}
8788

8889
func (vfs *VFS) Access(name string, flag sqlite3vfs.AccessFlag) (bool, error) {
@@ -176,7 +177,7 @@ func (vfs *VFS) deleteTempFile(name string) error {
176177
if !ok {
177178
return errTempFileNotFound
178179
}
179-
if err := os.Remove(path); err != nil {
180+
if err := os.Remove(path); err != nil && !errors.Is(err, os.ErrNotExist) {
180181
return err
181182
}
182183
vfs.tempFiles.Delete(vfs.canonicalTempName(name))
@@ -307,12 +308,24 @@ func (f *VFSFile) Open() error {
307308
}
308309
f.pageSize = pageSize
309310

310-
// Determine the current position based off the latest LTX file.
311-
var pos ltx.Pos
311+
// Determine the current position & level-1 polling point based on restore plan.
312+
var (
313+
pos ltx.Pos
314+
maxTXID1 ltx.TXID
315+
)
316+
for _, info := range infos {
317+
if info.Level == 1 && info.MaxTXID > maxTXID1 {
318+
maxTXID1 = info.MaxTXID
319+
}
320+
}
312321
if len(infos) > 0 {
313322
pos = ltx.Pos{TXID: infos[len(infos)-1].MaxTXID}
323+
if maxTXID1 == 0 {
324+
maxTXID1 = pos.TXID
325+
}
314326
}
315327
f.pos = pos
328+
f.maxTXID1 = maxTXID1
316329

317330
// Build the page index so we can lookup individual pages.
318331
if err := f.buildIndex(f.ctx, infos); err != nil {

vfs_lock_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,56 @@ func TestVFSFile_CorruptedPageIndexRecovery(t *testing.T) {
332332
}
333333
}
334334

335+
func TestVFSFile_OpenSeedsLevel1Position(t *testing.T) {
336+
client := newMockReplicaClient()
337+
snapshot := buildLTXFixture(t, 1, 's')
338+
snapshot.info.Level = SnapshotLevel
339+
client.addFixture(t, snapshot)
340+
l1 := buildLTXFixture(t, 2, 'l')
341+
l1.info.Level = 1
342+
client.addFixture(t, l1)
343+
l0 := buildLTXFixture(t, 3, 'z')
344+
l0.info.Level = 0
345+
client.addFixture(t, l0)
346+
347+
f := NewVFSFile(client, "seed-level1.db", slog.Default())
348+
if err := f.Open(); err != nil {
349+
t.Fatalf("open vfs file: %v", err)
350+
}
351+
defer f.Close()
352+
353+
if got, want := f.maxTXID1, l1.info.MaxTXID; got != want {
354+
t.Fatalf("unexpected maxTXID1: got %s want %s", got, want)
355+
}
356+
if got, want := f.Pos().TXID, l0.info.MaxTXID; got != want {
357+
t.Fatalf("unexpected pos after open: got %s want %s", got, want)
358+
}
359+
}
360+
361+
func TestVFSFile_OpenSeedsLevel1PositionFromPos(t *testing.T) {
362+
client := newMockReplicaClient()
363+
snapshot := buildLTXFixture(t, 1, 's')
364+
snapshot.info.Level = SnapshotLevel
365+
client.addFixture(t, snapshot)
366+
l0 := buildLTXFixture(t, 2, '0')
367+
l0.info.Level = 0
368+
client.addFixture(t, l0)
369+
370+
f := NewVFSFile(client, "seed-default.db", slog.Default())
371+
if err := f.Open(); err != nil {
372+
t.Fatalf("open vfs file: %v", err)
373+
}
374+
defer f.Close()
375+
376+
pos := f.Pos().TXID
377+
if pos == 0 {
378+
t.Fatalf("expected non-zero position")
379+
}
380+
if got := f.maxTXID1; got != pos {
381+
t.Fatalf("expected maxTXID1 to equal pos when no L1 files, got %s want %s", got, pos)
382+
}
383+
}
384+
335385
func TestVFSFile_HeaderForcesDeleteJournal(t *testing.T) {
336386
client := newMockReplicaClient()
337387
client.addFixture(t, buildLTXFixture(t, 1, 'h'))
@@ -568,6 +618,61 @@ func TestVFS_TempFileDeleteOnClose(t *testing.T) {
568618
if _, ok := vfs.loadTempFilePath(name); ok {
569619
t.Fatalf("temp file tracking entry should be cleared")
570620
}
621+
if err := vfs.Delete(name, false); err != nil {
622+
t.Fatalf("delete should ignore missing temp files: %v", err)
623+
}
624+
if err := vfs.Delete(name, false); err != nil {
625+
t.Fatalf("delete should ignore repeated temp deletes: %v", err)
626+
}
627+
}
628+
629+
func TestVFS_DeleteIgnoresMissingTempFiles(t *testing.T) {
630+
vfs := NewVFS(nil, slog.Default())
631+
632+
t.Run("AlreadyRemovedEntry", func(t *testing.T) {
633+
name := "already-removed.db"
634+
flags := sqlite3vfs.OpenTempDB | sqlite3vfs.OpenReadWrite | sqlite3vfs.OpenCreate | sqlite3vfs.OpenDeleteOnClose
635+
636+
file, _, err := vfs.openTempFile(name, flags)
637+
if err != nil {
638+
t.Fatalf("open temp file: %v", err)
639+
}
640+
tf := file.(*localTempFile)
641+
if err := tf.Close(); err != nil {
642+
t.Fatalf("close temp file: %v", err)
643+
}
644+
if err := vfs.Delete(name, false); err != nil {
645+
t.Fatalf("delete should ignore missing tracked entry: %v", err)
646+
}
647+
})
648+
649+
t.Run("MissingOnDisk", func(t *testing.T) {
650+
name := "missing-on-disk.db"
651+
flags := sqlite3vfs.OpenTempDB | sqlite3vfs.OpenReadWrite | sqlite3vfs.OpenCreate
652+
653+
file, _, err := vfs.openTempFile(name, flags)
654+
if err != nil {
655+
t.Fatalf("open temp file: %v", err)
656+
}
657+
tf := file.(*localTempFile)
658+
659+
path, ok := vfs.loadTempFilePath(name)
660+
if !ok {
661+
t.Fatalf("temp file not tracked")
662+
}
663+
if err := os.Remove(path); err != nil {
664+
t.Fatalf("remove backing file: %v", err)
665+
}
666+
if err := vfs.Delete(name, false); err != nil {
667+
t.Fatalf("delete should ignore missing file: %v", err)
668+
}
669+
if _, ok := vfs.loadTempFilePath(name); ok {
670+
t.Fatalf("temp file tracking entry should be cleared")
671+
}
672+
if err := tf.Close(); err != nil {
673+
t.Fatalf("close temp file: %v", err)
674+
}
675+
})
571676
}
572677

573678
func TestVFS_TempDirExhaustion(t *testing.T) {

0 commit comments

Comments
 (0)