16535: Refactor Sync implementation.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

diff --git a/sdk/go/arvados/fs_base.go b/sdk/go/arvados/fs_base.go
index d06aba3695adc37f3d74057de2568778bcd9f9c9..5e57fed3beab3281b1d498936edb4eed813398ec 100644 (file)
--- a/sdk/go/arvados/fs_base.go
+++ b/sdk/go/arvados/fs_base.go
@@ -31,6 +31,10 @@ var (
        ErrPermission        = os.ErrPermission
 )
 
+type syncer interface {
+       Sync() error
+}
+
 // A File is an *os.File-like interface for reading and writing files
 // in a FileSystem.
 type File interface {
@@ -299,6 +303,22 @@ func (n *treenode) Readdir() (fi []os.FileInfo, err error) {
        return
 }
 
+func (n *treenode) Sync() error {
+       n.RLock()
+       defer n.RUnlock()
+       for _, inode := range n.inodes {
+               syncer, ok := inode.(syncer)
+               if !ok {
+                       return ErrInvalidOperation
+               }
+               err := syncer.Sync()
+               if err != nil {
+                       return err
+               }
+       }
+       return nil
+}
+
 type fileSystem struct {
        root inode
        fsBackend
@@ -576,8 +596,11 @@ func (fs *fileSystem) remove(name string, recursive bool) error {
 }
 
 func (fs *fileSystem) Sync() error {
-       log.Printf("TODO: sync fileSystem")
-       return ErrInvalidOperation
+       if syncer, ok := fs.root.(syncer); ok {
+               return syncer.Sync()
+       } else {
+               return ErrInvalidOperation
+       }
 }
 
 func (fs *fileSystem) Flush(string, bool) error {

diff --git a/sdk/go/arvados/fs_deferred.go b/sdk/go/arvados/fs_deferred.go
index 1f888a4e8dd44c37fb628d4d95ce05f555137105..254b90c812a337de96cb34da01b767dbe7adcc5a 100644 (file)
--- a/sdk/go/arvados/fs_deferred.go
+++ b/sdk/go/arvados/fs_deferred.go
@@ -87,15 +87,16 @@ func (dn *deferrednode) Child(name string, replace func(inode) (inode, error)) (
        return dn.realinode().Child(name, replace)
 }
 
-// Sync is currently unimplemented, except when it's a no-op because
-// the real inode hasn't been created.
+// Sync is a no-op if the real inode hasn't even been created yet.
 func (dn *deferrednode) Sync() error {
        dn.mtx.Lock()
        defer dn.mtx.Unlock()
-       if dn.created {
-               return ErrInvalidArgument
-       } else {
+       if !dn.created {
                return nil
+       } else if syncer, ok := dn.wrapped.(syncer); ok {
+               return syncer.Sync()
+       } else {
+               return ErrInvalidOperation
        }
 }
 

diff --git a/sdk/go/arvados/fs_lookup.go b/sdk/go/arvados/fs_lookup.go
index 42322a14a9adda155c75f225ef43e2cdfd96615c..ad44ef22df938c04bca22f54bf25f52715b2625d 100644 (file)
--- a/sdk/go/arvados/fs_lookup.go
+++ b/sdk/go/arvados/fs_lookup.go
@@ -15,7 +15,7 @@ import (
 //
 // See (*customFileSystem)MountUsers for example usage.
 type lookupnode struct {
-       inode
+       treenode
        loadOne func(parent inode, name string) (inode, error)
        loadAll func(parent inode) ([]inode, error)
        stale   func(time.Time) bool
@@ -36,7 +36,7 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
                        return nil, err
                }
                for _, child := range all {
-                       _, err = ln.inode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
+                       _, err = ln.treenode.Child(child.FileInfo().Name(), func(inode) (inode, error) {
                                return child, nil
                        })
                        if err != nil {
@@ -49,7 +49,7 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
                // newer than ln.staleAll. Reclaim memory.
                ln.staleOne = nil
        }
-       return ln.inode.Readdir()
+       return ln.treenode.Readdir()
 }
 
 func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (inode, error) {
@@ -57,7 +57,7 @@ func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (in
        defer ln.staleLock.Unlock()
        checkTime := time.Now()
        if ln.stale(ln.staleAll) && ln.stale(ln.staleOne[name]) {
-               _, err := ln.inode.Child(name, func(inode) (inode, error) {
+               _, err := ln.treenode.Child(name, func(inode) (inode, error) {
                        return ln.loadOne(ln, name)
                })
                if err != nil {
@@ -69,5 +69,5 @@ func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (in
                        ln.staleOne[name] = checkTime
                }
        }
-       return ln.inode.Child(name, replace)
+       return ln.treenode.Child(name, replace)
 }

diff --git a/sdk/go/arvados/fs_site.go b/sdk/go/arvados/fs_site.go
index 8b61ccfd6ecb13593f4df719f3eb256aeb3353b0..95b2f71c252563ff95ad14a6d9bbfd9990e1d19f 100644 (file)
--- a/sdk/go/arvados/fs_site.go
+++ b/sdk/go/arvados/fs_site.go
@@ -5,7 +5,6 @@
 package arvados
 
 import (
-       "fmt"
        "os"
        "strings"
        "sync"
@@ -41,7 +40,7 @@ func (c *Client) CustomFileSystem(kc keepClient) CustomFileSystem {
                        thr:       newThrottle(concurrentWriters),
                },
        }
-       root.inode = &treenode{
+       root.treenode = treenode{
                fs:     fs,
                parent: root,
                fileinfo: fileinfo{
@@ -55,9 +54,9 @@ func (c *Client) CustomFileSystem(kc keepClient) CustomFileSystem {
 }
 
 func (fs *customFileSystem) MountByID(mount string) {
-       fs.root.inode.Child(mount, func(inode) (inode, error) {
+       fs.root.treenode.Child(mount, func(inode) (inode, error) {
                return &vdirnode{
-                       inode: &treenode{
+                       treenode: treenode{
                                fs:     fs,
                                parent: fs.root,
                                inodes: make(map[string]inode),
@@ -73,18 +72,18 @@ func (fs *customFileSystem) MountByID(mount string) {
 }
 
 func (fs *customFileSystem) MountProject(mount, uuid string) {
-       fs.root.inode.Child(mount, func(inode) (inode, error) {
+       fs.root.treenode.Child(mount, func(inode) (inode, error) {
                return fs.newProjectNode(fs.root, mount, uuid), nil
        })
 }
 
 func (fs *customFileSystem) MountUsers(mount string) {
-       fs.root.inode.Child(mount, func(inode) (inode, error) {
+       fs.root.treenode.Child(mount, func(inode) (inode, error) {
                return &lookupnode{
                        stale:   fs.Stale,
                        loadOne: fs.usersLoadOne,
                        loadAll: fs.usersLoadAll,
-                       inode: &treenode{
+                       treenode: treenode{
                                fs:     fs,
                                parent: fs.root,
                                inodes: make(map[string]inode),
@@ -116,43 +115,7 @@ func (c *Client) SiteFileSystem(kc keepClient) CustomFileSystem {
 }
 
 func (fs *customFileSystem) Sync() error {
-       fs.staleLock.Lock()
-       fs.staleThreshold = time.Now()
-       fs.staleLock.Unlock()
-       return fs.syncTree("/", fs.root.inode)
-}
-
-// syncTree calls node.Sync() if it has its own Sync method, otherwise
-// it calls syncTree() on all of node's children.
-//
-// Returns ErrInvalidArgument if node does not implement Sync() and
-// isn't a directory (or if Readdir() fails for any other reason).
-func (fs *customFileSystem) syncTree(path string, node inode) error {
-       if vn, ok := node.(*vdirnode); ok {
-               node = vn.inode
-       }
-       if syncer, ok := node.(interface{ Sync() error }); ok {
-               err := syncer.Sync()
-               if err != nil {
-                       return fmt.Errorf("%s: %T: %w", path, syncer, err)
-               }
-               return nil
-       }
-       fis, err := node.Readdir()
-       if err != nil {
-               return fmt.Errorf("%s: %T: %w", path, node, ErrInvalidArgument)
-       }
-       for _, fi := range fis {
-               child, err := node.Child(fi.Name(), nil)
-               if err != nil {
-                       continue
-               }
-               err = fs.syncTree(path+"/"+fi.Name(), child)
-               if err != nil {
-                       return err
-               }
-       }
-       return nil
+       return fs.root.Sync()
 }
 
 // Stale returns true if information obtained at time t should be
@@ -197,7 +160,7 @@ func (fs *customFileSystem) newProjectNode(root inode, name, uuid string) inode
                stale:   fs.Stale,
                loadOne: func(parent inode, name string) (inode, error) { return fs.projectsLoadOne(parent, uuid, name) },
                loadAll: func(parent inode) ([]inode, error) { return fs.projectsLoadAll(parent, uuid) },
-               inode: &treenode{
+               treenode: treenode{
                        fs:     fs,
                        parent: root,
                        inodes: make(map[string]inode),
@@ -217,17 +180,17 @@ func (fs *customFileSystem) newProjectNode(root inode, name, uuid string) inode
 // create() can return either a new node, which will be added to the
 // treenode, or nil for ENOENT.
 type vdirnode struct {
-       inode
+       treenode
        create func(parent inode, name string) inode
 }
 
 func (vn *vdirnode) Child(name string, replace func(inode) (inode, error)) (inode, error) {
-       return vn.inode.Child(name, func(existing inode) (inode, error) {
+       return vn.treenode.Child(name, func(existing inode) (inode, error) {
                if existing == nil && vn.create != nil {
                        existing = vn.create(vn, name)
                        if existing != nil {
                                existing.SetParent(vn, name)
-                               vn.inode.(*treenode).fileinfo.modTime = time.Now()
+                               vn.treenode.fileinfo.modTime = time.Now()
                        }
                }
                if replace == nil {