Skip to content

Commit

Permalink
Merge #48144
Browse files Browse the repository at this point in the history
48144: storage: fix Pebble.DeleteDirAndFiles r=petermattis a=petermattis

Fix `Pebble.DeleteDirAndFiles` to actually delete the specified
directory. Previously it was only recursively deleting the directory's
contents. Not deleting the directory was causing the `TestDiskQueue` and
`TestSpillingQueue` tests to fail which assert that the queue directory
does not exist at the end of the test.

This fix also addresses a TODO to use a `RemoveAll` function which has
been implemented in `pebble/vfs.FS` for a while.

Release note: None

Co-authored-by: Peter Mattis <petermattis@gmail.com>
  • Loading branch information
craig[bot] and petermattis committed Apr 29, 2020
2 parents 05e45df + 0e7b2e1 commit 26b9716
Showing 1 changed file with 5 additions and 22 deletions.
27 changes: 5 additions & 22 deletions pkg/storage/pebble.go
Original file line number Diff line number Diff line change
Expand Up @@ -970,31 +970,14 @@ func (p *Pebble) DeleteFile(filename string) error {

// DeleteDirAndFiles implements the Engine interface.
func (p *Pebble) DeleteDirAndFiles(dir string) error {
// TODO(itsbilal): Implement FS.RemoveAll then call that here instead.
files, err := p.fs.List(dir)
// NB RemoveAll does not return an error if dir does not exist, but we want
// DeleteDirAndFiles to return an error in that case to match the RocksDB
// behavior.
_, err := p.fs.Stat(dir)
if err != nil {
return err
}

// Recurse through all files, calling DeleteFile or DeleteDirAndFiles as
// appropriate.
for _, filename := range files {
path := p.fs.PathJoin(dir, filename)
stat, err := p.fs.Stat(path)
if err != nil {
return err
}

if stat.IsDir() {
err = p.DeleteDirAndFiles(path)
} else {
err = p.DeleteFile(path)
}
if err != nil {
return err
}
}
return nil
return p.fs.RemoveAll(dir)
}

// LinkFile implements the FS interface.
Expand Down

0 comments on commit 26b9716

Please sign in to comment.