From 31de31f9fd586d9647cd538ed0b12231370427e2 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 14:17:16 +1000 Subject: [PATCH 01/20] Updated jacobsa/fuse to 962e8e2. --- vendor.json | 32 ++++----- vendor/github.com/jacobsa/fuse/connection.go | 15 +++-- vendor/github.com/jacobsa/fuse/debug.go | 21 ++++++ vendor/github.com/jacobsa/fuse/fuseops/ops.go | 13 +--- .../jacobsa/fuse/fusetesting/stat.go | 26 ++++---- vendor/github.com/jacobsa/fuse/mount.go | 8 +-- .../github.com/jacobsa/fuse/mount_config.go | 65 +++++++++++++++++++ 7 files changed, 132 insertions(+), 48 deletions(-) diff --git a/vendor.json b/vendor.json index 6e6e8bfc5b..1b9182179c 100755 --- a/vendor.json +++ b/vendor.json @@ -13,57 +13,57 @@ "canonical": "github.com/jacobsa/fuse", "comment": "", "local": "vendor/github.com/jacobsa/fuse", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/fsutil", "comment": "", "local": "vendor/github.com/jacobsa/fuse/fsutil", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/fuseops", "comment": "", "local": "vendor/github.com/jacobsa/fuse/fuseops", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/fusetesting", "comment": "", "local": "vendor/github.com/jacobsa/fuse/fusetesting", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/fuseutil", "comment": "", "local": "vendor/github.com/jacobsa/fuse/fuseutil", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/internal/buffer", "comment": "", "local": "vendor/github.com/jacobsa/fuse/internal/buffer", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/internal/freelist", "comment": "", "local": "vendor/github.com/jacobsa/fuse/internal/freelist", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/fuse/internal/fusekernel", "comment": "", "local": "vendor/github.com/jacobsa/fuse/internal/fusekernel", - "revision": "5348027205ba7a3565af455c3e6e94be57025215", - "revisionTime": "2015-08-11T10:55:48+10:00" + "revision": "962e8e26d5cc338f60d21394e1ae45218009d651", + "revisionTime": "2015-08-12T12:42:00+10:00" }, { "canonical": "github.com/jacobsa/gcloud/gcs", diff --git a/vendor/github.com/jacobsa/fuse/connection.go b/vendor/github.com/jacobsa/fuse/connection.go index 7dfd29f7a9..0d7e13a987 100644 --- a/vendor/github.com/jacobsa/fuse/connection.go +++ b/vendor/github.com/jacobsa/fuse/connection.go @@ -57,6 +57,7 @@ const maxReadahead = 1 << 20 // A connection to the fuse kernel process. type Connection struct { + cfg MountConfig debugLogger *log.Logger errorLogger *log.Logger @@ -65,9 +66,6 @@ type Connection struct { dev *os.File protocol fusekernel.Protocol - // The context from which all op contexts inherit. - parentCtx context.Context - mu sync.Mutex // A map from fuse "unique" request ID (*not* the op ID for logging used @@ -94,15 +92,15 @@ type opState struct { // // The loggers may be nil. func newConnection( - parentCtx context.Context, + cfg MountConfig, debugLogger *log.Logger, errorLogger *log.Logger, dev *os.File) (c *Connection, err error) { c = &Connection{ + cfg: cfg, debugLogger: debugLogger, errorLogger: errorLogger, dev: dev, - parentCtx: parentCtx, cancelFuncs: make(map[uint64]func()), } @@ -165,6 +163,11 @@ func (c *Connection) Init() (err error) { // Tell the kernel not to use pitifully small 4 KiB writes. initOp.Flags |= fusekernel.InitBigWrites + // Enable writeback caching if the user hasn't asked us not to. + if !c.cfg.DisableWritebackCaching { + initOp.Flags |= fusekernel.InitWritebackCache + } + c.Reply(ctx, nil) return } @@ -227,7 +230,7 @@ func (c *Connection) beginOp( opCode uint32, fuseID uint64) (ctx context.Context) { // Start with the parent context. - ctx = c.parentCtx + ctx = c.cfg.OpContext // Set up a cancellation function. // diff --git a/vendor/github.com/jacobsa/fuse/debug.go b/vendor/github.com/jacobsa/fuse/debug.go index 14e1d36431..06b12c8a09 100644 --- a/vendor/github.com/jacobsa/fuse/debug.go +++ b/vendor/github.com/jacobsa/fuse/debug.go @@ -53,6 +53,27 @@ func describeRequest(op interface{}) (s string) { case *unknownOp: addComponent("opcode %d", typed.OpCode) + case *fuseops.LookUpInodeOp: + addComponent("parent %d", typed.Parent) + addComponent("name %q", typed.Name) + + case *fuseops.SetInodeAttributesOp: + if typed.Size != nil { + addComponent("size %d", *typed.Size) + } + + if typed.Mode != nil { + addComponent("mode %v", *typed.Mode) + } + + if typed.Atime != nil { + addComponent("atime %v", *typed.Atime) + } + + if typed.Mtime != nil { + addComponent("mtime %v", *typed.Mtime) + } + case *fuseops.ReadFileOp: addComponent("handle %d", typed.Handle) addComponent("offset %d", typed.Offset) diff --git a/vendor/github.com/jacobsa/fuse/fuseops/ops.go b/vendor/github.com/jacobsa/fuse/fuseops/ops.go index f7864746b0..5da1b5e423 100644 --- a/vendor/github.com/jacobsa/fuse/fuseops/ops.go +++ b/vendor/github.com/jacobsa/fuse/fuseops/ops.go @@ -509,17 +509,8 @@ type ReadFileOp struct { // // Note that the kernel *will* ensure that writes are received and acknowledged // by the file system before sending a FlushFileOp when closing the file -// descriptor to which they were written: -// -// * (http://goo.gl/PheZjf) fuse_flush calls write_inode_now, which appears -// to start a writeback in the background (it talks about a "flusher -// thread"). -// -// * (http://goo.gl/1IiepM) fuse_flush then calls fuse_sync_writes, which -// "[waits] for all pending writepages on the inode to finish". -// -// * (http://goo.gl/zzvxWv) Only then does fuse_flush finally send the -// flush request. +// descriptor to which they were written. Cf. the notes on +// fuse.MountConfig.DisableWritebackCaching. // // (See also http://goo.gl/ocdTdM, fuse-devel thread "Fuse guarantees on // concurrent requests".) diff --git a/vendor/github.com/jacobsa/fuse/fusetesting/stat.go b/vendor/github.com/jacobsa/fuse/fusetesting/stat.go index c084303625..c8bdbc1e42 100644 --- a/vendor/github.com/jacobsa/fuse/fusetesting/stat.go +++ b/vendor/github.com/jacobsa/fuse/fusetesting/stat.go @@ -28,28 +28,32 @@ import ( // also that it matches. func MtimeIs(expected time.Time) oglematchers.Matcher { return oglematchers.NewMatcher( - func(c interface{}) error { return mtimeIs(c, expected) }, + func(c interface{}) error { return mtimeIsWithin(c, expected, 0) }, fmt.Sprintf("mtime is %v", expected)) } -func mtimeIs(c interface{}, expected time.Time) error { +// Like MtimeIs, but allows for a tolerance. +func MtimeIsWithin(expected time.Time, d time.Duration) oglematchers.Matcher { + return oglematchers.NewMatcher( + func(c interface{}) error { return mtimeIsWithin(c, expected, d) }, + fmt.Sprintf("mtime is within %v of %v", d, expected)) +} + +func mtimeIsWithin(c interface{}, expected time.Time, d time.Duration) error { fi, ok := c.(os.FileInfo) if !ok { return fmt.Errorf("which is of type %v", reflect.TypeOf(c)) } // Check ModTime(). - if fi.ModTime() != expected { - d := fi.ModTime().Sub(expected) - return fmt.Errorf("which has mtime %v, off by %v", fi.ModTime(), d) + diff := fi.ModTime().Sub(expected) + absDiff := diff + if absDiff < 0 { + absDiff = -absDiff } - // Check Sys(). - if sysMtime, ok := extractMtime(fi.Sys()); ok { - if sysMtime != expected { - d := sysMtime.Sub(expected) - return fmt.Errorf("which has Sys() mtime %v, off by %v", sysMtime, d) - } + if !(absDiff < d) { + return fmt.Errorf("which has mtime %v, off by %v", fi.ModTime(), diff) } return nil diff --git a/vendor/github.com/jacobsa/fuse/mount.go b/vendor/github.com/jacobsa/fuse/mount.go index 7f9c55308e..6227617109 100644 --- a/vendor/github.com/jacobsa/fuse/mount.go +++ b/vendor/github.com/jacobsa/fuse/mount.go @@ -67,14 +67,14 @@ func Mount( } // Choose a parent context for ops. - opContext := config.OpContext - if opContext == nil { - opContext = context.Background() + cfgCopy := *config + if cfgCopy.OpContext == nil { + cfgCopy.OpContext = context.Background() } // Create a Connection object wrapping the device. connection, err := newConnection( - opContext, + cfgCopy, config.DebugLogger, config.ErrorLogger, dev) diff --git a/vendor/github.com/jacobsa/fuse/mount_config.go b/vendor/github.com/jacobsa/fuse/mount_config.go index dd76561e3e..61f805b9a3 100644 --- a/vendor/github.com/jacobsa/fuse/mount_config.go +++ b/vendor/github.com/jacobsa/fuse/mount_config.go @@ -48,6 +48,71 @@ type MountConfig struct { // performed. DebugLogger *log.Logger + // Linux only. OS X always behaves as if writeback caching is disabled. + // + // By default on Linux we allow the kernel to perform writeback caching + // (cf. http://goo.gl/LdZzo1): + // + // * When the user calls write(2), the kernel sticks the user's data into + // its page cache. Only later does it call through to the file system, + // potentially after coalescing multiple small user writes. + // + // * The file system may receive multiple write ops from the kernel + // concurrently if there is a lot of page cache data to flush. + // + // * Write performance may be significantly improved due to the user and + // the kernel not waiting for serial round trips to the file system. This + // is especially true if the user makes tiny writes. + // + // * close(2) (and anything else calling f_op->flush) causes all dirty + // pages to be written out before it proceeds to send a FlushFileOp + // (cf. https://goo.gl/TMrY6X). + // + // * Similarly, close(2) causes the kernel to send a setattr request + // filling in the mtime if any dirty pages were flushed, since the time + // at which the pages were written to the file system can't be trusted. + // + // * close(2) (and anything else calling f_op->flush) writes out all dirty + // pages, then sends a setattr request with an appropriate mtime for + // those writes if there were any, and only then proceeds to send a flush + // + // Code walk: + // + // * (https://goo.gl/zTIZQ9) fuse_flush calls write_inode_now before + // calling the file system. The latter eventually calls into + // __writeback_single_inode. + // + // * (https://goo.gl/L7Z2w5) __writeback_single_inode calls + // do_writepages, which writes out any dirty pages. + // + // * (https://goo.gl/DOPgla) __writeback_single_inode later calls + // write_inode, which calls into the superblock op struct's write_inode + // member. For fuse, this is fuse_write_inode + // (cf. https://goo.gl/eDSKOX). + // + // * (https://goo.gl/PbkGA1) fuse_write_inode calls fuse_flush_times. + // + // * (https://goo.gl/ig8x9V) fuse_flush_times sends a setttr request + // for setting the inode's mtime. + // + // However, this brings along some caveats: + // + // * The file system must handle SetInodeAttributesOp or close(2) will fail, + // due to the call chain into fuse_flush_times listed above. + // + // * The kernel caches mtime and ctime regardless of whether the file + // system tells it to do so, disregarding the result of further getattr + // requests (cf. https://goo.gl/3ZZMUw, https://goo.gl/7WtQUp). It + // appears this may be true of the file size, too. Writeback caching may + // therefore not be suitable for file systems where these attributes can + // spontaneously change for reasons the kernel doesn't observe. See + // http://goo.gl/V5WQCN for more discussion. + // + // Setting DisableWritebackCaching disables this behavior. Instead the file + // system is called one or more times for each write(2), and the user's + // syscall doesn't return until the file system returns. + DisableWritebackCaching bool + // OS X only. // // Normally on OS X we mount with the novncache option From ee437efbba8d1e37823638e1544b739a7a019cac Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:27:25 +0000 Subject: [PATCH 02/20] StressTest.CreateAndReadManyFilesInParallel --- internal/fs/stress_test.go | 49 +++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/internal/fs/stress_test.go b/internal/fs/stress_test.go index fe6d742e72..e58fd6ec0a 100644 --- a/internal/fs/stress_test.go +++ b/internal/fs/stress_test.go @@ -32,8 +32,9 @@ import ( // Helpers //////////////////////////////////////////////////////////////////////// -// Run the supplied function for each name, with parallelism. -func forEachName(names []string, f func(string)) { +// Run the supplied function for each name, with parallelism. Return an error +// if any invocation does. +func forEachName(names []string, f func(string) error) (err error) { const parallelism = 8 // Fill a channel. @@ -44,18 +45,29 @@ func forEachName(names []string, f func(string)) { close(c) // Run workers. + firstErr := make(chan error, 1) + var wg sync.WaitGroup for i := 0; i < parallelism; i++ { wg.Add(1) go func() { defer wg.Done() for n := range c { - f(n) + err := f(n) + if err != nil { + select { + case firstErr <- err: + default: + } + } } }() } wg.Wait() + + err, _ = <-firstErr + return } //////////////////////////////////////////////////////////////////////// @@ -69,6 +81,8 @@ type StressTest struct { func init() { RegisterTestSuite(&StressTest{}) } func (t *StressTest) CreateAndReadManyFilesInParallel() { + var err error + // Ensure that we get parallelism for this test. defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(runtime.NumCPU())) @@ -81,21 +95,34 @@ func (t *StressTest) CreateAndReadManyFilesInParallel() { } // Create a file for each name with concurrent workers. - forEachName( + err = forEachName( names, - func(n string) { - err := ioutil.WriteFile(path.Join(t.Dir, n), []byte(n), 0400) - AssertEq(nil, err) + func(n string) (err error) { + err = ioutil.WriteFile(path.Join(t.Dir, n), []byte(n), 0400) + return }) + AssertEq(nil, err) + // Read each back. - forEachName( + err = forEachName( names, - func(n string) { + func(n string) (err error) { contents, err := ioutil.ReadFile(path.Join(t.Dir, n)) - AssertEq(nil, err) - AssertEq(n, string(contents)) + if err != nil { + err = fmt.Errorf("ReadFile: %v", err) + return + } + + if string(contents) != n { + err = fmt.Errorf("Contents mismatch: %q vs. %q", contents, n) + return + } + + return }) + + AssertEq(nil, err) } func (t *StressTest) TruncateFileManyTimesInParallel() { From dc695c6335b2e57816935ce419d612f16da7cfe6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:29:57 +0000 Subject: [PATCH 03/20] StressTest.TruncateFileManyTimesInParallel --- internal/fs/stress_test.go | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/internal/fs/stress_test.go b/internal/fs/stress_test.go index e58fd6ec0a..790d4c2c79 100644 --- a/internal/fs/stress_test.go +++ b/internal/fs/stress_test.go @@ -24,8 +24,11 @@ import ( "sync" "time" + "golang.org/x/net/context" + "github.com/jacobsa/fuse/fusetesting" . "github.com/jacobsa/ogletest" + "github.com/jacobsa/syncutil" ) //////////////////////////////////////////////////////////////////////// @@ -136,7 +139,7 @@ func (t *StressTest) TruncateFileManyTimesInParallel() { // Set up a function that repeatedly truncates the file to random lengths, // writing the final size to a channel. - worker := func(finalSize chan<- int64) { + worker := func(finalSize chan<- int64) (err error) { const desiredDuration = 500 * time.Millisecond var size int64 @@ -144,28 +147,33 @@ func (t *StressTest) TruncateFileManyTimesInParallel() { for time.Since(startTime) < desiredDuration { for i := 0; i < 10; i++ { size = rand.Int63n(1 << 14) - err := f.Truncate(size) - AssertEq(nil, err) + err = f.Truncate(size) + if err != nil { + return + } } } finalSize <- size + return } // Run several workers. + b := syncutil.NewBundle(t.ctx) + const numWorkers = 16 finalSizes := make(chan int64, numWorkers) - var wg sync.WaitGroup for i := 0; i < numWorkers; i++ { - wg.Add(1) - go func() { - defer wg.Done() - worker(finalSizes) - }() + b.Add(func(ctx context.Context) (err error) { + err = worker(finalSizes) + return + }) } - wg.Wait() + err = b.Join() + AssertEq(nil, err) + close(finalSizes) // The final size should be consistent. From 45e2ef765c69f90f87003d1907b1e1d936476460 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:34:12 +0000 Subject: [PATCH 04/20] Clarify mtime semantics. --- docs/semantics.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/semantics.md b/docs/semantics.md index 6cd1e421af..2ff740e6ea 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -224,8 +224,10 @@ actor in the meantime.) There are no guarantees about whether local modifications are reflected in GCS after writing but before syncing or closing. Modification time (`stat::st_mtime` on Linux) is tracked for file inodes, but -only for modifications to contents (not, for example, by utimes(2)). No other -times are tracked. +only for modifications to contents (not, for example, by utimes(2)). +Additionally, modification time is only approximate -- it is affected by the use +of kernel writeback caching, and is reset to the time of creation of the backing +GCS object when the file is flushed. No other times (ctime, etc.) are tracked. ### Identity From b4daebb09136e3521d47730851885ee7bcc57d54 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:35:15 +0000 Subject: [PATCH 05/20] Fixed an emdash. --- docs/semantics.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/semantics.md b/docs/semantics.md index 2ff740e6ea..66266b4444 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -225,8 +225,8 @@ modifications are reflected in GCS after writing but before syncing or closing. Modification time (`stat::st_mtime` on Linux) is tracked for file inodes, but only for modifications to contents (not, for example, by utimes(2)). -Additionally, modification time is only approximate -- it is affected by the use -of kernel writeback caching, and is reset to the time of creation of the backing +Additionally, modification time is only approximate—it is affected by the use of +kernel writeback caching, and is reset to the time of creation of the backing GCS object when the file is flushed. No other times (ctime, etc.) are tracked. From dbe61e2db5dc4028aa0734981c437c35e03cbbea Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:36:58 +0000 Subject: [PATCH 06/20] Specify that we ignore setattr requests for mtime. We must do this because writeback caching causes the kernel to send a setattr for mtime every time a file is closed after being written to. --- docs/semantics.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/semantics.md b/docs/semantics.md index 66266b4444..44b3160050 100644 --- a/docs/semantics.md +++ b/docs/semantics.md @@ -224,10 +224,12 @@ actor in the meantime.) There are no guarantees about whether local modifications are reflected in GCS after writing but before syncing or closing. Modification time (`stat::st_mtime` on Linux) is tracked for file inodes, but -only for modifications to contents (not, for example, by utimes(2)). -Additionally, modification time is only approximate—it is affected by the use of -kernel writeback caching, and is reset to the time of creation of the backing -GCS object when the file is flushed. No other times (ctime, etc.) are tracked. +only for modifications to contents. For example, it cannot be updated by a call +to `utimes(2)`. Additionally, modification time is only approximate—it is +affected by the use of kernel writeback caching, and is reset to the time of +creation of the backing GCS object when the file is flushed. + +No other times (atime, ctime, etc.) are tracked. ### Identity @@ -481,5 +483,5 @@ Not all of the usual file system features are supported. Most prominently: * File and directory permissions and ownership cannot be changed. See the [section](#permissions-and-ownership) above. -* Modification times cannot be changed. See the - [section](#modifications) above. +* Modification times cannot be changed, and attempt to do so is simply + ignored. See the [section](#modifications) above. From ffe1c93b32e1f2f1abd1dd842b125ec4e0dd6ad6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:39:58 +0000 Subject: [PATCH 07/20] Don't return ENOSYS for setattr(mtime) requests. --- internal/fs/fs.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index dabd500a7d..3f6d3eb32a 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -886,14 +886,20 @@ func (fs *fileSystem) SetInodeAttributes( in.Lock() defer in.Unlock() - // The only thing we support changing is size, and then only for directories. - if op.Mode != nil || op.Atime != nil || op.Mtime != nil { + // We don't support changing attributes of anything but files. + file, ok := in.(*inode.FileInode) + if !ok { err = fuse.ENOSYS return } - file, ok := in.(*inode.FileInode) - if !ok { + // The only thing we support changing is size. Return an error for attempts + // to change anything else. + // + // Special case: ignore changes to mtime which are sent by the Linux kernel + // before flushing a file that has been written to when writeback caching is + // enabled. + if op.Mode != nil || op.Atime != nil { err = fuse.ENOSYS return } From b8df8f07c4554f6811296197e519ac27a21aaa61 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Wed, 12 Aug 2015 04:41:51 +0000 Subject: [PATCH 08/20] Fixed a test bug. --- internal/fs/stress_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/fs/stress_test.go b/internal/fs/stress_test.go index 790d4c2c79..a5a87fc678 100644 --- a/internal/fs/stress_test.go +++ b/internal/fs/stress_test.go @@ -69,7 +69,10 @@ func forEachName(names []string, f func(string) error) (err error) { wg.Wait() + // Read the first error, if any. + close(firstErr) err, _ = <-firstErr + return } From 7f95ef46f712e76046b8c717ef2422fdd017e32b Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 02:51:04 +0000 Subject: [PATCH 09/20] Fixed FileTest.Sync_Clobbered under writeback caching. --- internal/fs/local_modifications_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index 647c7b8d62..7784c74028 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1871,12 +1871,15 @@ func (t *FileTest) Sync_Clobbered() { "foo", []byte("foobar")) - // Sync the file. This should not result in an error, but the new generation - // should not be replaced. + // Attempt to sync the file. This may result in an error if the OS has + // decided to hold back the writes from above until now (in which case the + // inode will fail to load the source object), or it may fail silently. + // Either way, this should not result in a new generation being created. err = t.f1.Sync() - AssertEq(nil, err) + if err != nil { + ExpectThat(err, Error(HasSubstr("input/output error"))) + } - // Check that the new generation was not replaced. contents, err := gcsutil.ReadObject(t.ctx, t.bucket, "foo") AssertEq(nil, err) ExpectEq("foobar", string(contents)) From e87cd852677708e474655cbe74644dee1df8d550 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 02:54:29 +0000 Subject: [PATCH 10/20] FileTest.Stat --- internal/fs/local_modifications_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index 7784c74028..f4baf92388 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -40,6 +40,11 @@ import ( "github.com/jacobsa/timeutil" ) +// The radius we use for "expect mtime is within"-style assertions. We can't +// share a synchronized clock with the ultimate source of mtimes because with +// writeback caching enabled the kernel manufactures them based on wall time. +const timeSlop = 25 * time.Millisecond + var fuseMaxNameLen int func init() { @@ -1449,14 +1454,14 @@ func (t *FileTest) Stat() { AssertEq(nil, err) // Give it some contents. - t.clock.AdvanceTime(time.Second) - writeTime := t.clock.Now() + time.Sleep(timeSlop + timeSlop/2) + writeTime := time.Now() n, err = t.f1.Write([]byte("taco")) AssertEq(nil, err) AssertEq(4, n) - t.clock.AdvanceTime(time.Second) + time.Sleep(timeSlop + timeSlop/2) // Stat it. fi, err := t.f1.Stat() @@ -1465,7 +1470,7 @@ func (t *FileTest) Stat() { ExpectEq("foo", fi.Name()) ExpectEq(len("taco"), fi.Size()) ExpectEq(filePerms, fi.Mode()) - ExpectThat(fi.ModTime(), timeutil.TimeEq(writeTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(writeTime, timeSlop)) ExpectFalse(fi.IsDir()) ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), fi.Sys().(*syscall.Stat_t).Uid) From e1884c268c2844e376f7421b195b4ffc1aeff549 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 02:55:46 +0000 Subject: [PATCH 11/20] FileTest.StatUnopenedFile --- internal/fs/local_modifications_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index f4baf92388..0e61e4156b 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1481,13 +1481,13 @@ func (t *FileTest) StatUnopenedFile() { var err error // Create and close a file. - t.clock.AdvanceTime(time.Second) - createTime := t.clock.Now() + time.Sleep(timeSlop + timeSlop/2) + createTime := time.Now() err = ioutil.WriteFile(path.Join(t.mfs.Dir(), "foo"), []byte("taco"), 0700) AssertEq(nil, err) - t.clock.AdvanceTime(time.Second) + time.Sleep(timeSlop + timeSlop/2) // Stat it. fi, err := os.Stat(path.Join(t.mfs.Dir(), "foo")) @@ -1496,7 +1496,7 @@ func (t *FileTest) StatUnopenedFile() { ExpectEq("foo", fi.Name()) ExpectEq(len("taco"), fi.Size()) ExpectEq(filePerms, fi.Mode()) - ExpectThat(fi.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(fi.IsDir()) ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), fi.Sys().(*syscall.Stat_t).Uid) From 2805d071e1651f5b2e6feda485a48ef205f62f4f Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 02:56:24 +0000 Subject: [PATCH 12/20] FileTest.LstatUnopenedFile --- internal/fs/local_modifications_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index 0e61e4156b..91637669c1 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1507,13 +1507,13 @@ func (t *FileTest) LstatUnopenedFile() { var err error // Create and close a file. - t.clock.AdvanceTime(time.Second) - createTime := t.clock.Now() + time.Sleep(timeSlop + timeSlop/2) + createTime := time.Now() err = ioutil.WriteFile(path.Join(t.mfs.Dir(), "foo"), []byte("taco"), 0700) AssertEq(nil, err) - t.clock.AdvanceTime(time.Second) + time.Sleep(timeSlop + timeSlop/2) // Lstat it. fi, err := os.Lstat(path.Join(t.mfs.Dir(), "foo")) @@ -1522,7 +1522,7 @@ func (t *FileTest) LstatUnopenedFile() { ExpectEq("foo", fi.Name()) ExpectEq(len("taco"), fi.Size()) ExpectEq(filePerms, fi.Mode()) - ExpectThat(fi.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(fi.IsDir()) ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), fi.Sys().(*syscall.Stat_t).Uid) From b1ca852779f60ae530e7364c9fef7b3047d00721 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:00:35 +0000 Subject: [PATCH 13/20] Use separate clocks for cache expiration and mtimes in DirInode. This is necessary for writeback caching support, where we want our mtime clock to agree with the kernel's clock when it is using writeback caching. --- internal/fs/inode/dir.go | 31 +++++++++++++++++-------------- internal/fs/inode/dir_test.go | 1 + internal/fs/inode/explicit_dir.go | 6 ++++-- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/internal/fs/inode/dir.go b/internal/fs/inode/dir.go index db710e9012..e968546ef8 100644 --- a/internal/fs/inode/dir.go +++ b/internal/fs/inode/dir.go @@ -150,8 +150,9 @@ type dirInode struct { // Dependencies ///////////////////////// - bucket gcs.Bucket - clock timeutil.Clock + bucket gcs.Bucket + mtimeClock timeutil.Clock + cacheClock timeutil.Clock ///////////////////////// // Constant data @@ -210,7 +211,8 @@ func NewDirInode( implicitDirs bool, typeCacheTTL time.Duration, bucket gcs.Bucket, - clock timeutil.Clock) (d DirInode) { + mtimeClock timeutil.Clock, + cacheClock timeutil.Clock) (d DirInode) { if !IsDirName(name) { panic(fmt.Sprintf("Unexpected name: %s", name)) } @@ -219,7 +221,8 @@ func NewDirInode( const typeCacheCapacity = 1 << 16 typed := &dirInode{ bucket: bucket, - clock: clock, + mtimeClock: mtimeClock, + cacheClock: cacheClock, id: id, implicitDirs: implicitDirs, name: name, @@ -463,7 +466,7 @@ func (d *dirInode) filterMissingChildDirs( // First add any names that we already know are directories according to our // cache, removing them from the input. - now := d.clock.Now() + now := d.cacheClock.Now() var tmp []string for _, name := range in { if d.cache.IsDir(now, name) { @@ -532,7 +535,7 @@ func (d *dirInode) filterMissingChildDirs( err = b.Join() // Update the cache with everything we learned. - now = d.clock.Now() + now = d.cacheClock.Now() for _, name := range filteredSlice { d.cache.NoteDir(now, name) } @@ -604,7 +607,7 @@ func (d *dirInode) LookUpChild( name string) (result LookUpResult, err error) { // Consult the cache about the type of the child. This may save us work // below. - now := d.clock.Now() + now := d.cacheClock.Now() cacheSaysFile := d.cache.IsFile(now, name) cacheSaysDir := d.cache.IsDir(now, name) @@ -651,7 +654,7 @@ func (d *dirInode) LookUpChild( } // Update the cache. - now = d.clock.Now() + now = d.cacheClock.Now() if fileResult.Exists() { d.cache.NoteFile(now, name) } @@ -727,7 +730,7 @@ func (d *dirInode) ReadEntries( newTok = listing.ContinuationToken // Update the type cache with everything we learned. - now := d.clock.Now() + now := d.cacheClock.Now() for _, e := range entries { switch e.Type { case fuseutil.DT_File: @@ -746,7 +749,7 @@ func (d *dirInode) CreateChildFile( ctx context.Context, name string) (o *gcs.Object, err error) { metadata := map[string]string{ - FileMtimeMetadataKey: d.clock.Now().UTC().Format(time.RFC3339Nano), + FileMtimeMetadataKey: d.mtimeClock.Now().UTC().Format(time.RFC3339Nano), } o, err = d.createNewObject(ctx, path.Join(d.Name(), name), metadata) @@ -754,7 +757,7 @@ func (d *dirInode) CreateChildFile( return } - d.cache.NoteFile(d.clock.Now(), name) + d.cache.NoteFile(d.cacheClock.Now(), name) return } @@ -781,7 +784,7 @@ func (d *dirInode) CloneToChildFile( } // Update the type cache. - d.cache.NoteFile(d.clock.Now(), name) + d.cache.NoteFile(d.cacheClock.Now(), name) return } @@ -800,7 +803,7 @@ func (d *dirInode) CreateChildSymlink( return } - d.cache.NoteFile(d.clock.Now(), name) + d.cache.NoteFile(d.cacheClock.Now(), name) return } @@ -814,7 +817,7 @@ func (d *dirInode) CreateChildDir( return } - d.cache.NoteDir(d.clock.Now(), name) + d.cache.NoteDir(d.cacheClock.Now(), name) return } diff --git a/internal/fs/inode/dir_test.go b/internal/fs/inode/dir_test.go index 2a34926a19..c94f942bfb 100644 --- a/internal/fs/inode/dir_test.go +++ b/internal/fs/inode/dir_test.go @@ -97,6 +97,7 @@ func (t *DirTest) resetInode(implicitDirs bool) { implicitDirs, typeCacheTTL, t.bucket, + &t.clock, &t.clock) t.in.Lock() diff --git a/internal/fs/inode/explicit_dir.go b/internal/fs/inode/explicit_dir.go index b7b087c7eb..7993fdd9b4 100644 --- a/internal/fs/inode/explicit_dir.go +++ b/internal/fs/inode/explicit_dir.go @@ -40,7 +40,8 @@ func NewExplicitDirInode( implicitDirs bool, typeCacheTTL time.Duration, bucket gcs.Bucket, - clock timeutil.Clock) (d ExplicitDirInode) { + mtimeClock timeutil.Clock, + cacheClock timeutil.Clock) (d ExplicitDirInode) { wrapped := NewDirInode( id, o.Name, @@ -48,7 +49,8 @@ func NewExplicitDirInode( implicitDirs, typeCacheTTL, bucket, - clock) + mtimeClock, + cacheClock) d = &explicitDirInode{ dirInode: wrapped.(*dirInode), From 5741c014c2e08f88b16d22def2164bd1e6922aec Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:02:38 +0000 Subject: [PATCH 14/20] Be explicit about the purpose of FileInode.Clock. --- internal/fs/inode/file.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/fs/inode/file.go b/internal/fs/inode/file.go index b03005de93..8f955812b6 100644 --- a/internal/fs/inode/file.go +++ b/internal/fs/inode/file.go @@ -36,9 +36,9 @@ type FileInode struct { // Dependencies ///////////////////////// - bucket gcs.Bucket - syncer gcsx.Syncer - clock timeutil.Clock + bucket gcs.Bucket + syncer gcsx.Syncer + mtimeClock timeutil.Clock ///////////////////////// // Constant data @@ -93,17 +93,17 @@ func NewFileInode( bucket gcs.Bucket, syncer gcsx.Syncer, tempDir string, - clock timeutil.Clock) (f *FileInode) { + mtimeClock timeutil.Clock) (f *FileInode) { // Set up the basic struct. f = &FileInode{ - bucket: bucket, - syncer: syncer, - clock: clock, - id: id, - name: o.Name, - attrs: attrs, - tempDir: tempDir, - src: *o, + bucket: bucket, + syncer: syncer, + mtimeClock: mtimeClock, + id: id, + name: o.Name, + attrs: attrs, + tempDir: tempDir, + src: *o, } f.lc.Init(id) @@ -191,7 +191,7 @@ func (f *FileInode) ensureContent(ctx context.Context) (err error) { defer rc.Close() // Create a temporary file with its contents. - tf, err := gcsx.NewTempFile(rc, f.tempDir, f.clock) + tf, err := gcsx.NewTempFile(rc, f.tempDir, f.mtimeClock) if err != nil { err = fmt.Errorf("NewTempFile: %v", err) return From 4b836de28367c834efaade0018f8b0c154456673 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:04:03 +0000 Subject: [PATCH 15/20] Fixed clock configuration in package fs. --- internal/fs/fs.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 6066add760..d1dbd8749e 100644 --- a/internal/fs/fs.go +++ b/internal/fs/fs.go @@ -36,8 +36,9 @@ import ( ) type ServerConfig struct { - // A clock used for modification times and cache expiration. - Clock timeutil.Clock + // A clock used for cache expiration. It is *not* used for inode times, for + // which we use the wall clock. + CacheClock timeutil.Clock // The bucket that the file system is to export. Bucket gcs.Bucket @@ -121,7 +122,8 @@ func NewServer(cfg *ServerConfig) (server fuse.Server, err error) { // Set up the basic struct. fs := &fileSystem{ - clock: cfg.Clock, + mtimeClock: timeutil.RealClock(), + cacheClock: cfg.CacheClock, bucket: cfg.Bucket, syncer: syncer, tempDir: cfg.TempDir, @@ -150,7 +152,8 @@ func NewServer(cfg *ServerConfig) (server fuse.Server, err error) { fs.implicitDirs, fs.dirTypeCacheTTL, cfg.Bucket, - fs.clock) + fs.mtimeClock, + fs.cacheClock) root.Lock() root.IncrementLookupCount() @@ -203,9 +206,10 @@ type fileSystem struct { // Dependencies ///////////////////////// - clock timeutil.Clock - bucket gcs.Bucket - syncer gcsx.Syncer + mtimeClock timeutil.Clock + cacheClock timeutil.Clock + bucket gcs.Bucket + syncer gcsx.Syncer ///////////////////////// // Constant data @@ -494,7 +498,8 @@ func (fs *fileSystem) mintInode(name string, o *gcs.Object) (in inode.Inode) { fs.implicitDirs, fs.dirTypeCacheTTL, fs.bucket, - fs.clock) + fs.mtimeClock, + fs.cacheClock) // Implicit directories case inode.IsDirName(name): @@ -509,7 +514,8 @@ func (fs *fileSystem) mintInode(name string, o *gcs.Object) (in inode.Inode) { fs.implicitDirs, fs.dirTypeCacheTTL, fs.bucket, - fs.clock) + fs.mtimeClock, + fs.cacheClock) case inode.IsSymlink(o): in = inode.NewSymlinkInode( @@ -533,7 +539,7 @@ func (fs *fileSystem) mintInode(name string, o *gcs.Object) (in inode.Inode) { fs.bucket, fs.syncer, fs.tempDir, - fs.clock) + fs.mtimeClock) } // Place it in our map of IDs to inodes. From 918d5060483e5ae7745761163bc8985ad2112bd7 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:07:24 +0000 Subject: [PATCH 16/20] Fixed up some fs test clocks. --- internal/fs/caching_test.go | 14 +++++++------- internal/fs/fs_test.go | 14 ++++++++------ internal/fs/local_modifications_test.go | 6 +++--- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/internal/fs/caching_test.go b/internal/fs/caching_test.go index c09118c77a..472989b75e 100644 --- a/internal/fs/caching_test.go +++ b/internal/fs/caching_test.go @@ -44,14 +44,14 @@ type cachingTestCommon struct { func (t *cachingTestCommon) SetUp(ti *TestInfo) { // Wrap the bucket in a stat caching layer for the purposes of the file // system. - t.uncachedBucket = gcsfake.NewFakeBucket(&t.clock, "some_bucket") + t.uncachedBucket = gcsfake.NewFakeBucket(t.mtimeClock, "some_bucket") const statCacheCapacity = 1000 statCache := gcscaching.NewStatCache(statCacheCapacity) t.bucket = gcscaching.NewFastStatBucket( ttl, statCache, - &t.clock, + &t.cacheClock, t.uncachedBucket) // Enable directory type caching. @@ -149,7 +149,7 @@ func (t *CachingTest) FileChangedRemotely() { ExpectEq(len("taco"), fi.Size()) // After the TTL elapses, we should see the new version. - t.clock.AdvanceTime(ttl + time.Millisecond) + t.cacheClock.AdvanceTime(ttl + time.Millisecond) fi, err = os.Stat(path.Join(t.Dir, name)) AssertEq(nil, err) @@ -183,7 +183,7 @@ func (t *CachingTest) DirectoryRemovedRemotely() { ExpectTrue(fi.IsDir()) // After the TTL elapses, we should see it disappear. - t.clock.AdvanceTime(ttl + time.Millisecond) + t.cacheClock.AdvanceTime(ttl + time.Millisecond) _, err = os.Stat(path.Join(t.Dir, name)) ExpectTrue(os.IsNotExist(err), "err: %v", err) @@ -217,7 +217,7 @@ func (t *CachingTest) ConflictingNames_RemoteModifier() { ExpectTrue(os.IsNotExist(err), "err: %v", err) // After the TTL elapses, we should see both. - t.clock.AdvanceTime(ttl + time.Millisecond) + t.cacheClock.AdvanceTime(ttl + time.Millisecond) fi, err = os.Stat(path.Join(t.Dir, name)) AssertEq(nil, err) @@ -282,7 +282,7 @@ func (t *CachingTest) TypeOfNameChanges_RemoteModifier() { ExpectTrue(os.IsNotExist(err), "err: %v", err) // After the TTL elapses, we should see it turn into a file. - t.clock.AdvanceTime(ttl + time.Millisecond) + t.cacheClock.AdvanceTime(ttl + time.Millisecond) fi, err = os.Stat(path.Join(t.Dir, name)) AssertEq(nil, err) @@ -405,7 +405,7 @@ func (t *CachingWithImplicitDirsTest) SymlinksAreTypeCached() { ExpectEq(filePerms|os.ModeSymlink, fi.Mode()) // After the TTL elapses, we should see the directory. - t.clock.AdvanceTime(ttl + time.Millisecond) + t.cacheClock.AdvanceTime(ttl + time.Millisecond) fi, err = os.Lstat(path.Join(t.Dir, "foo")) AssertEq(nil, err) diff --git a/internal/fs/fs_test.go b/internal/fs/fs_test.go index dc4722711d..52678dec0e 100644 --- a/internal/fs/fs_test.go +++ b/internal/fs/fs_test.go @@ -77,8 +77,9 @@ type fsTest struct { // Dependencies. If bucket is set before SetUp is called, it will be used // rather than creating a default one. - clock timeutil.SimulatedClock - bucket gcs.Bucket + mtimeClock timeutil.Clock + cacheClock timeutil.SimulatedClock + bucket gcs.Bucket // Mount information mfs *fuse.MountedFileSystem @@ -96,13 +97,14 @@ func (t *fsTest) SetUp(ti *TestInfo) { var err error t.ctx = ti.Ctx - // Set up the clock. - t.clock.SetTime(time.Date(2015, 4, 5, 2, 15, 0, 0, time.Local)) - t.serverCfg.Clock = &t.clock + // Set up the clocks. + t.mtimeClock = timeutil.RealClock() + t.cacheClock.SetTime(time.Date(2015, 4, 5, 2, 15, 0, 0, time.Local)) + t.serverCfg.CacheClock = &t.cacheClock // And the bucket. if t.bucket == nil { - t.bucket = gcsfake.NewFakeBucket(&t.clock, "some_bucket") + t.bucket = gcsfake.NewFakeBucket(t.mtimeClock, "some_bucket") } t.serverCfg.Bucket = t.bucket diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index 91637669c1..ddd027983d 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -1455,7 +1455,7 @@ func (t *FileTest) Stat() { // Give it some contents. time.Sleep(timeSlop + timeSlop/2) - writeTime := time.Now() + writeTime := t.mtimeClock.Now() n, err = t.f1.Write([]byte("taco")) AssertEq(nil, err) @@ -1482,7 +1482,7 @@ func (t *FileTest) StatUnopenedFile() { // Create and close a file. time.Sleep(timeSlop + timeSlop/2) - createTime := time.Now() + createTime := t.mtimeClock.Now() err = ioutil.WriteFile(path.Join(t.mfs.Dir(), "foo"), []byte("taco"), 0700) AssertEq(nil, err) @@ -1508,7 +1508,7 @@ func (t *FileTest) LstatUnopenedFile() { // Create and close a file. time.Sleep(timeSlop + timeSlop/2) - createTime := time.Now() + createTime := t.mtimeClock.Now() err = ioutil.WriteFile(path.Join(t.mfs.Dir(), "foo"), []byte("taco"), 0700) AssertEq(nil, err) From fd354bd6c9393a89c3701661d5fbbc562808dcd3 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:09:43 +0000 Subject: [PATCH 17/20] Fixed local_modifications_test.go. --- internal/fs/local_modifications_test.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index ddd027983d..f389cbdbd5 100644 --- a/internal/fs/local_modifications_test.go +++ b/internal/fs/local_modifications_test.go @@ -952,17 +952,13 @@ func (t *DirectoryTest) ReadDir_Root() { var fi os.FileInfo // Create a file and a directory. - t.clock.AdvanceTime(time.Second) - createTime := t.clock.Now() - + createTime := t.mtimeClock.Now() err = ioutil.WriteFile(path.Join(t.mfs.Dir(), "bar"), []byte("taco"), 0700) AssertEq(nil, err) err = os.Mkdir(path.Join(t.mfs.Dir(), "foo"), 0700) AssertEq(nil, err) - t.clock.AdvanceTime(time.Second) - // ReadDir entries, err := fusetesting.ReadDirPicky(t.mfs.Dir()) AssertEq(nil, err) @@ -973,7 +969,7 @@ func (t *DirectoryTest) ReadDir_Root() { ExpectEq("bar", fi.Name()) ExpectEq(len("taco"), fi.Size()) ExpectEq(filePerms, fi.Mode()) - ExpectThat(fi.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(fi.IsDir()) ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), fi.Sys().(*syscall.Stat_t).Uid) @@ -1000,17 +996,13 @@ func (t *DirectoryTest) ReadDir_SubDirectory() { AssertEq(nil, err) // Create a file and a directory within it. - t.clock.AdvanceTime(time.Second) - createTime := t.clock.Now() - + createTime := t.mtimeClock.Now() err = ioutil.WriteFile(path.Join(parent, "bar"), []byte("taco"), 0700) AssertEq(nil, err) err = os.Mkdir(path.Join(parent, "foo"), 0700) AssertEq(nil, err) - t.clock.AdvanceTime(time.Second) - // ReadDir entries, err := fusetesting.ReadDirPicky(parent) AssertEq(nil, err) @@ -1021,7 +1013,7 @@ func (t *DirectoryTest) ReadDir_SubDirectory() { ExpectEq("bar", fi.Name()) ExpectEq(len("taco"), fi.Size()) ExpectEq(filePerms, fi.Mode()) - ExpectThat(fi.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(fi, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(fi.IsDir()) ExpectEq(1, fi.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), fi.Sys().(*syscall.Stat_t).Uid) From a0b6954b1fde84a679088e7b20046e8fcde461f6 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:11:14 +0000 Subject: [PATCH 18/20] Fixed foreign_modifications_test.go. --- internal/fs/foreign_modifications_test.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/internal/fs/foreign_modifications_test.go b/internal/fs/foreign_modifications_test.go index 1bfe888959..32b7617a4f 100644 --- a/internal/fs/foreign_modifications_test.go +++ b/internal/fs/foreign_modifications_test.go @@ -98,7 +98,7 @@ func (t *ForeignModsTest) ReadDir_EmptyRoot() { func (t *ForeignModsTest) ReadDir_ContentsInRoot() { // Set up contents. - createTime := t.clock.Now() + createTime := t.mtimeClock.Now() AssertEq( nil, t.createObjects( @@ -113,9 +113,6 @@ func (t *ForeignModsTest) ReadDir_ContentsInRoot() { "baz": "burrito", })) - // Make sure the time below doesn't match. - t.clock.AdvanceTime(time.Second) - ///////////////////////// // ReadDir ///////////////////////// @@ -141,7 +138,7 @@ func (t *ForeignModsTest) ReadDir_ContentsInRoot() { ExpectEq("baz", e.Name()) ExpectEq(len("burrito"), e.Size()) ExpectEq(filePerms, e.Mode()) - ExpectThat(e.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(e, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(e.IsDir()) ExpectEq(1, e.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), e.Sys().(*syscall.Stat_t).Uid) @@ -152,7 +149,7 @@ func (t *ForeignModsTest) ReadDir_ContentsInRoot() { ExpectEq("foo", e.Name()) ExpectEq(len("taco"), e.Size()) ExpectEq(filePerms, e.Mode()) - ExpectThat(e.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(e, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(e.IsDir()) ExpectEq(1, e.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), e.Sys().(*syscall.Stat_t).Uid) @@ -176,7 +173,7 @@ func (t *ForeignModsTest) ReadDir_EmptySubDirectory() { func (t *ForeignModsTest) ReadDir_ContentsInSubDirectory() { // Set up contents. - createTime := t.clock.Now() + createTime := t.mtimeClock.Now() AssertEq( nil, t.createObjects( @@ -194,9 +191,6 @@ func (t *ForeignModsTest) ReadDir_ContentsInSubDirectory() { "dir/baz": "burrito", })) - // Make sure the time below doesn't match. - t.clock.AdvanceTime(time.Second) - // Wait for the directory to show up in the file system. _, err := fusetesting.ReadDirPicky(path.Join(t.mfs.Dir())) AssertEq(nil, err) @@ -223,7 +217,7 @@ func (t *ForeignModsTest) ReadDir_ContentsInSubDirectory() { ExpectEq("baz", e.Name()) ExpectEq(len("burrito"), e.Size()) ExpectEq(filePerms, e.Mode()) - ExpectThat(e.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(e, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(e.IsDir()) ExpectEq(1, e.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), e.Sys().(*syscall.Stat_t).Uid) @@ -234,7 +228,7 @@ func (t *ForeignModsTest) ReadDir_ContentsInSubDirectory() { ExpectEq("foo", e.Name()) ExpectEq(len("taco"), e.Size()) ExpectEq(filePerms, e.Mode()) - ExpectThat(e.ModTime(), timeutil.TimeEq(createTime)) + ExpectThat(e, fusetesting.MtimeIsWithin(createTime, timeSlop)) ExpectFalse(e.IsDir()) ExpectEq(1, e.Sys().(*syscall.Stat_t).Nlink) ExpectEq(currentUid(), e.Sys().(*syscall.Stat_t).Uid) From 21c9bde10eb3dd2f58f9d4a766aa9e84e362a2ea Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:12:42 +0000 Subject: [PATCH 19/20] Fixed a panic. --- internal/fs/caching_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/fs/caching_test.go b/internal/fs/caching_test.go index 472989b75e..395bec2406 100644 --- a/internal/fs/caching_test.go +++ b/internal/fs/caching_test.go @@ -28,6 +28,7 @@ import ( "github.com/jacobsa/gcloud/gcs/gcsutil" . "github.com/jacobsa/oglematchers" . "github.com/jacobsa/ogletest" + "github.com/jacobsa/timeutil" ) //////////////////////////////////////////////////////////////////////// @@ -44,7 +45,7 @@ type cachingTestCommon struct { func (t *cachingTestCommon) SetUp(ti *TestInfo) { // Wrap the bucket in a stat caching layer for the purposes of the file // system. - t.uncachedBucket = gcsfake.NewFakeBucket(t.mtimeClock, "some_bucket") + t.uncachedBucket = gcsfake.NewFakeBucket(timeutil.RealClock(), "some_bucket") const statCacheCapacity = 1000 statCache := gcscaching.NewStatCache(statCacheCapacity) From 21b3f88ed5e93bd35cb9e1f1072631bb85263b92 Mon Sep 17 00:00:00 2001 From: Aaron Jacobs Date: Thu, 13 Aug 2015 03:13:10 +0000 Subject: [PATCH 20/20] Fixed mount.go. --- mount.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mount.go b/mount.go index 87d879b580..8d699f66fb 100644 --- a/mount.go +++ b/mount.go @@ -83,7 +83,7 @@ func mount( // Create a file system server. serverCfg := &fs.ServerConfig{ - Clock: timeutil.RealClock(), + CacheClock: timeutil.RealClock(), Bucket: bucket, TempDir: flags.TempDir, ImplicitDirectories: flags.ImplicitDirs,