Skip to content

Commit

Permalink
db: fix Open issue with single-item relative paths
Browse files Browse the repository at this point in the history
Previously Open with a relative path containing a single element would fail on
real filesystems, because Open would attempt to open the empty path.

Fix #3842.
  • Loading branch information
jbowens committed Aug 13, 2024
1 parent 57ff76d commit 4512006
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 46 deletions.
11 changes: 4 additions & 7 deletions checkpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,23 +91,20 @@ func mkdirAllAndSyncParents(fs vfs.FS, destDir string) (vfs.File, error) {
// Collect paths for all directories between destDir (excluded) and its
// closest existing ancestor (included).
var parentPaths []string
foundExistingAncestor := false
for parentPath := fs.PathDir(destDir); parentPath != "."; parentPath = fs.PathDir(parentPath) {
for parentPath := fs.PathDir(destDir); ; parentPath = fs.PathDir(parentPath) {
parentPaths = append(parentPaths, parentPath)
if fs.PathDir(parentPath) == parentPath {
break
}
_, err := fs.Stat(parentPath)
if err == nil {
// Exit loop at the closest existing ancestor.
foundExistingAncestor = true
break
}
if !oserror.IsNotExist(err) {
return nil, err
}
}
// Handle empty filesystem edge case.
if !foundExistingAncestor {
parentPaths = append(parentPaths, "")
}
// Create destDir and any of its missing parents.
if err := fs.MkdirAll(destDir, 0755); err != nil {
return nil, err
Expand Down
46 changes: 46 additions & 0 deletions open_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1478,3 +1478,49 @@ func TestOpenRatchetsNextFileNum(t *testing.T) {
require.NoError(t, d.Compact([]byte("a"), []byte("z"), false))

}

func TestMkdirAllAndSyncParents(t *testing.T) {
if filepath.Separator != '/' {
t.Skip("skipping due to path separator")
}
pwd, err := os.Getwd()
require.NoError(t, err)
defer func() { os.Chdir(pwd) }()

filesystems := map[string]vfs.FS{}
rootPaths := map[string]string{}
var buf bytes.Buffer
datadriven.RunTest(t, "testdata/mkdir_all_and_sync_parents", func(t *testing.T, td *datadriven.TestData) string {
buf.Reset()
switch td.Cmd {
case "mkfs":
var fsName string
td.ScanArgs(t, "fs", &fsName)
if td.HasArg("memfs") {
filesystems[fsName] = vfs.NewMem()
return "new memfs"
}
filesystems[fsName] = vfs.Default
rootPaths[fsName] = t.TempDir()
return "new default fs"
case "mkdir-all-and-sync-parents":
var fsName, path string
td.ScanArgs(t, "fs", &fsName)
td.ScanArgs(t, "path", &path)
if p, ok := rootPaths[fsName]; ok {
require.NoError(t, os.Chdir(p))
}
fs := vfs.WithLogging(filesystems[fsName], func(format string, args ...interface{}) {
fmt.Fprintf(&buf, format+"\n", args...)
})
f, err := mkdirAllAndSyncParents(fs, path)
if err != nil {
return err.Error()
}
require.NoError(t, f.Close())
return buf.String()
default:
return fmt.Sprintf("unrecognized command %q", td.Cmd)
}
})
}
12 changes: 6 additions & 6 deletions testdata/checkpoint
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
open db
----
mkdir-all: db 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db
close: db
open-dir: db
Expand Down Expand Up @@ -92,9 +92,9 @@ mkdir-all: checkpoints/checkpoint1 0755
open-dir: checkpoints
sync: checkpoints
close: checkpoints
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: checkpoints/checkpoint1
link: db/OPTIONS-000003 -> checkpoints/checkpoint1/OPTIONS-000003
open-dir: checkpoints/checkpoint1
Expand Down
12 changes: 6 additions & 6 deletions testdata/checkpoint_shared
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
open db
----
mkdir-all: db 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db
close: db
open-dir: db
Expand Down Expand Up @@ -80,9 +80,9 @@ mkdir-all: checkpoints/checkpoint1 0755
open-dir: checkpoints
sync: checkpoints
close: checkpoints
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: checkpoints/checkpoint1
link: db/OPTIONS-000003 -> checkpoints/checkpoint1/OPTIONS-000003
open-dir: checkpoints/checkpoint1
Expand Down
36 changes: 18 additions & 18 deletions testdata/cleaner
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
open db archive
----
mkdir-all: db 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db
close: db
mkdir-all: db_wal 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db_wal
close: db_wal
open-dir: db
Expand Down Expand Up @@ -132,15 +132,15 @@ list db_wal/archive
open db1
----
mkdir-all: db1 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db1
close: db1
mkdir-all: db1_wal 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db1_wal
close: db1_wal
open-dir: db1
Expand Down Expand Up @@ -211,15 +211,15 @@ close: db1/000456.sst
open db1
----
mkdir-all: db1 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db1
close: db1
mkdir-all: db1_wal 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db1_wal
close: db1_wal
open-dir: db1
Expand Down
18 changes: 9 additions & 9 deletions testdata/event_listener
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
open
----
mkdir-all: db 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: db
close: db
mkdir-all: wal 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: wal
close: wal
open-dir: db
Expand Down Expand Up @@ -346,9 +346,9 @@ sstables
checkpoint
----
mkdir-all: checkpoint 0755
open-dir:
sync:
close:
open-dir: .
sync: .
close: .
open-dir: checkpoint
link: db/OPTIONS-000003 -> checkpoint/OPTIONS-000003
open-dir: checkpoint
Expand Down
67 changes: 67 additions & 0 deletions testdata/mkdir_all_and_sync_parents
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
mkfs memfs fs=mem1
----
new memfs

mkdir-all-and-sync-parents fs=mem1 path=foo/bar/baz/bax
----
mkdir-all: foo/bar/baz/bax 0755
open-dir: foo/bar/baz
sync: foo/bar/baz
close: foo/bar/baz
open-dir: foo/bar
sync: foo/bar
close: foo/bar
open-dir: foo
sync: foo
close: foo
open-dir: .
sync: .
close: .
open-dir: foo/bar/baz/bax
close: foo/bar/baz/bax

# Repeating the same command should only sync the parent, and then the new data
# directory itself.

mkdir-all-and-sync-parents fs=mem1 path=foo/bar/baz/bax
----
mkdir-all: foo/bar/baz/bax 0755
open-dir: foo/bar/baz
sync: foo/bar/baz
close: foo/bar/baz
open-dir: foo/bar/baz/bax
close: foo/bar/baz/bax

mkfs fs=default1
----
new default fs

mkdir-all-and-sync-parents fs=default1 path=foo/bar/baz/bax
----
mkdir-all: foo/bar/baz/bax 0755
open-dir: foo/bar/baz
sync: foo/bar/baz
close: foo/bar/baz
open-dir: foo/bar
sync: foo/bar
close: foo/bar
open-dir: foo
sync: foo
close: foo
open-dir: .
sync: .
close: .
open-dir: foo/bar/baz/bax
close: foo/bar/baz/bax

# Repeating the same command should only sync the parent, and then the new data
# directory itself.

mkdir-all-and-sync-parents fs=default1 path=foo/bar/baz/bax
----
mkdir-all: foo/bar/baz/bax 0755
open-dir: foo/bar/baz
sync: foo/bar/baz
close: foo/bar/baz
open-dir: foo/bar/baz/bax
close: foo/bar/baz/bax
3 changes: 3 additions & 0 deletions vfs/mem_fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ func (y *MemFS) walk(fullname string, f func(dir *memNode, frag string, final bo
for len(fullname) > 0 && fullname[0] == sep[0] {
fullname = fullname[1:]
}
if fullname == "." {
fullname = ""
}
dir := y.root

for {
Expand Down

0 comments on commit 4512006

Please sign in to comment.