diff --git a/internal/fs/caching_test.go b/internal/fs/caching_test.go index c09118c77a..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,14 +45,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(timeutil.RealClock(), "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 +150,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 +184,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 +218,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 +283,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 +406,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/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) diff --git a/internal/fs/fs.go b/internal/fs/fs.go index 4fa0f7030a..fdab709cb2 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 @@ -133,7 +134,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, @@ -163,7 +165,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() @@ -216,9 +219,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 @@ -508,7 +512,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): @@ -523,7 +528,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( @@ -547,7 +553,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. 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/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), 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 diff --git a/internal/fs/local_modifications_test.go b/internal/fs/local_modifications_test.go index 647c7b8d62..f389cbdbd5 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() { @@ -947,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) @@ -968,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) @@ -995,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) @@ -1016,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) @@ -1449,14 +1446,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 := t.mtimeClock.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 +1462,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) @@ -1476,13 +1473,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 := t.mtimeClock.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")) @@ -1491,7 +1488,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) @@ -1502,13 +1499,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 := t.mtimeClock.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")) @@ -1517,7 +1514,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) @@ -1871,12 +1868,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)) diff --git a/internal/fs/stress_test.go b/internal/fs/stress_test.go index fe6d742e72..a5a87fc678 100644 --- a/internal/fs/stress_test.go +++ b/internal/fs/stress_test.go @@ -24,16 +24,20 @@ import ( "sync" "time" + "golang.org/x/net/context" + "github.com/jacobsa/fuse/fusetesting" . "github.com/jacobsa/ogletest" + "github.com/jacobsa/syncutil" ) //////////////////////////////////////////////////////////////////////// // 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 +48,32 @@ 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() + + // Read the first error, if any. + close(firstErr) + err, _ = <-firstErr + + return } //////////////////////////////////////////////////////////////////////// @@ -69,6 +87,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 +101,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() { @@ -109,7 +142,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 @@ -117,28 +150,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. diff --git a/mount.go b/mount.go index 8fff259494..2fd4f2efdf 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, diff --git a/vendor.json b/vendor.json index 3884196625..e83d55a70c 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