-
Notifications
You must be signed in to change notification settings - Fork 71
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add directory tracking to sync (#425)
## Changes This change replaces usage of the `repofiles` package with the `filer` package to consolidate WSFS code paths. The `repofiles` package implemented the following behavior. If a file at `foo/bar.txt` was created and removed, the directory `foo` was kept around because we do not perform directory tracking. If subsequently, a file at `foo` was created, it resulted in an `fs.ErrExist` because it is impossible to overwrite a directory. It would then perform a recursive delete of the path if this happened and retry the file write. To make this use case work without resorting to a recursive delete on conflict, we need to implement directory tracking as part of sync. The approach in this commit is as follows: 1. Maintain set of directories needed for current set of files. Compare to previous set of files. This results in mkdir of added directories and rmdir of removed directories. 2. Creation of new directories should happen prior to writing files. Otherwise, many file writes may race to create the same parent directories, resulting in additional API calls. Removal of existing directories should happen after removing files. 3. Making new directories can be deduped across common prefixes where only the longest prefix is created recursively. 4. Removing existing directories must happen sequentially, starting with the longest prefix. 5. Removal of directories is a best effort. It fails only if the directory is not empty, and if this happens we know something placed a file or directory manually, outside of sync. ## Tests * Existing integration tests pass (modified where it used to assert directories weren't cleaned up) * New integration test to confirm the inability to remove a directory doesn't fail the sync run
- Loading branch information
Showing
11 changed files
with
444 additions
and
297 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,100 @@ | ||
| package sync | ||
|
|
||
| import ( | ||
| "path" | ||
| ) | ||
|
|
||
| type diff struct { | ||
| put []string | ||
| delete []string | ||
| rmdir []string | ||
| mkdir []string | ||
| put []string | ||
| } | ||
|
|
||
| func (d diff) IsEmpty() bool { | ||
| return len(d.put) == 0 && len(d.delete) == 0 | ||
| } | ||
|
|
||
| // groupedMkdir returns a slice of slices of paths to create. | ||
| // Because the underlying mkdir calls create intermediate directories, | ||
| // we can group them together to reduce the total number of calls. | ||
| // This returns a slice of a slice for parity with [groupedRmdir]. | ||
| func (d diff) groupedMkdir() [][]string { | ||
| // Compute the set of prefixes of all paths to create. | ||
| prefixes := make(map[string]bool) | ||
| for _, name := range d.mkdir { | ||
| dir := path.Dir(name) | ||
| for dir != "." && dir != "/" { | ||
| prefixes[dir] = true | ||
| dir = path.Dir(dir) | ||
| } | ||
| } | ||
|
|
||
| var out []string | ||
|
|
||
| // Collect all paths that are not a prefix of another path. | ||
| for _, name := range d.mkdir { | ||
| if !prefixes[name] { | ||
| out = append(out, name) | ||
| } | ||
| } | ||
|
|
||
| return [][]string{out} | ||
| } | ||
|
|
||
| // groupedRmdir returns a slice of slices of paths to delete. | ||
| // The outer slice is ordered such that each inner slice can be | ||
| // deleted in parallel, as long as it is processed in order. | ||
| // The first entry will contain leaf directories, the second entry | ||
| // will contain intermediate directories, and so on. | ||
| func (d diff) groupedRmdir() [][]string { | ||
| // Compute the number of times each directory is a prefix of another directory. | ||
| prefixes := make(map[string]int) | ||
| for _, dir := range d.rmdir { | ||
| prefixes[dir] = 0 | ||
| } | ||
| for _, dir := range d.rmdir { | ||
| dir = path.Dir(dir) | ||
| for dir != "." && dir != "/" { | ||
| // Increment the prefix count for this directory, only if it | ||
| // it one of the directories we are deleting. | ||
| if _, ok := prefixes[dir]; ok { | ||
| prefixes[dir]++ | ||
| } | ||
| dir = path.Dir(dir) | ||
| } | ||
| } | ||
|
|
||
| var out [][]string | ||
|
|
||
| for len(prefixes) > 0 { | ||
| var toDelete []string | ||
|
|
||
| // Find directories which are not a prefix of another directory. | ||
| // These are the directories we can delete. | ||
| for dir, count := range prefixes { | ||
| if count == 0 { | ||
| toDelete = append(toDelete, dir) | ||
| delete(prefixes, dir) | ||
| } | ||
| } | ||
|
|
||
| // Remove these directories from the prefixes map. | ||
| for _, dir := range toDelete { | ||
| dir = path.Dir(dir) | ||
| for dir != "." && dir != "/" { | ||
| // Decrement the prefix count for this directory, only if it | ||
| // it one of the directories we are deleting. | ||
| if _, ok := prefixes[dir]; ok { | ||
| prefixes[dir]-- | ||
| } | ||
| dir = path.Dir(dir) | ||
| } | ||
| } | ||
|
|
||
| // Add these directories to the output. | ||
| out = append(out, toDelete) | ||
| } | ||
|
|
||
| return out | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| package sync | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestDiffGroupedMkdir(t *testing.T) { | ||
| d := diff{ | ||
| mkdir: []string{ | ||
| "foo", | ||
| "foo/bar", | ||
| "foo/bar/baz1", | ||
| "foo/bar/baz2", | ||
| "foo1", | ||
| "a/b", | ||
| "a/b/c/d/e/f", | ||
| }, | ||
| } | ||
|
|
||
| // Expect only leaf directories to be included. | ||
| out := d.groupedMkdir() | ||
| assert.Len(t, out, 1) | ||
| assert.ElementsMatch(t, []string{ | ||
| "foo/bar/baz1", | ||
| "foo/bar/baz2", | ||
| "foo1", | ||
| "a/b/c/d/e/f", | ||
| }, out[0]) | ||
| } | ||
|
|
||
| func TestDiffGroupedRmdir(t *testing.T) { | ||
| d := diff{ | ||
| rmdir: []string{ | ||
| "a/b/c/d/e/f", | ||
| "a/b/c/d/e", | ||
| "a/b/c/d", | ||
| "a/b/c", | ||
| "a/b/e/f/g/h", | ||
| "a/b/e/f/g", | ||
| "a/b/e/f", | ||
| "a/b/e", | ||
| "a/b", | ||
| }, | ||
| } | ||
|
|
||
| out := d.groupedRmdir() | ||
| assert.Len(t, out, 5) | ||
| assert.ElementsMatch(t, []string{"a/b/c/d/e/f", "a/b/e/f/g/h"}, out[0]) | ||
| assert.ElementsMatch(t, []string{"a/b/c/d/e", "a/b/e/f/g"}, out[1]) | ||
| assert.ElementsMatch(t, []string{"a/b/c/d", "a/b/e/f"}, out[2]) | ||
| assert.ElementsMatch(t, []string{"a/b/c", "a/b/e"}, out[3]) | ||
| assert.ElementsMatch(t, []string{"a/b"}, out[4]) | ||
| } | ||
|
|
||
| func TestDiffGroupedRmdirWithLeafsOnly(t *testing.T) { | ||
| d := diff{ | ||
| rmdir: []string{ | ||
| "foo/bar/baz1", | ||
| "foo/bar1", | ||
| "foo/bar/baz2", | ||
| "foo/bar2", | ||
| "foo1", | ||
| "foo2", | ||
| }, | ||
| } | ||
|
|
||
| // Expect all directories to be included. | ||
| out := d.groupedRmdir() | ||
| assert.Len(t, out, 1) | ||
| assert.ElementsMatch(t, d.rmdir, out[0]) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package sync | ||
|
|
||
| import ( | ||
| "path" | ||
| "path/filepath" | ||
| "sort" | ||
| ) | ||
|
|
||
| // DirSet is a set of directories. | ||
| type DirSet map[string]struct{} | ||
|
|
||
| // MakeDirSet turns a list of file paths into the complete set of directories | ||
| // that is needed to store them (including parent directories). | ||
| func MakeDirSet(files []string) DirSet { | ||
| out := map[string]struct{}{} | ||
|
|
||
| // Iterate over all files. | ||
| for _, f := range files { | ||
| // Get the directory of the file in /-separated form. | ||
| dir := filepath.ToSlash(filepath.Dir(f)) | ||
|
|
||
| // Add this directory and its parents until it is either "." or already in the set. | ||
| for dir != "." { | ||
| if _, ok := out[dir]; ok { | ||
| break | ||
| } | ||
| out[dir] = struct{}{} | ||
| dir = path.Dir(dir) | ||
| } | ||
| } | ||
|
|
||
| return out | ||
| } | ||
|
|
||
| // Slice returns a sorted copy of the dirset elements as a slice. | ||
| func (dirset DirSet) Slice() []string { | ||
| out := make([]string, 0, len(dirset)) | ||
| for dir := range dirset { | ||
| out = append(out, dir) | ||
| } | ||
| sort.Strings(out) | ||
| return out | ||
| } | ||
|
|
||
| // Remove returns the set difference of two DirSets. | ||
| func (dirset DirSet) Remove(other DirSet) DirSet { | ||
| out := map[string]struct{}{} | ||
| for dir := range dirset { | ||
| if _, ok := other[dir]; !ok { | ||
| out[dir] = struct{}{} | ||
| } | ||
| } | ||
| return out | ||
| } |
Oops, something went wrong.