Skip to content

Commit

Permalink
fix for kernel list cache remove directory when files are created on GCS
Browse files Browse the repository at this point in the history
  • Loading branch information
ashmeenkaur committed Jul 12, 2024
1 parent bf0babe commit d98eca8
Show file tree
Hide file tree
Showing 6 changed files with 117 additions and 0 deletions.
3 changes: 3 additions & 0 deletions internal/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -1829,6 +1829,9 @@ func (fs *fileSystem) RmDir(
err = fmt.Errorf("ReadEntries: %w", err)
return err
}
//Clear kernel list cache after removing a directory. This ensures remote
//GCS files are included in future directory listings for unlinking.
childDir.InvalidateKernelListCache()

// Are there any entries?
if len(entries) != 0 {
Expand Down
5 changes: 5 additions & 0 deletions internal/fs/inode/base_dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,3 +242,8 @@ func (d *baseDirInode) ShouldInvalidateKernelListCache(ttl time.Duration) bool {
// for baseDirInode.
return true
}

func (d *baseDirInode) InvalidateKernelListCache() {
// Kernel list cache is not supported for baseDirInode.
return
}
9 changes: 9 additions & 0 deletions internal/fs/inode/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ type DirInode interface {
// should be invalidated or not.
ShouldInvalidateKernelListCache(ttl time.Duration) bool

//InvalidateKernelListCache sets the prevDirListingTimeStamp to zero so that
//cache is invalidated for next list call.
InvalidateKernelListCache()

// RLock readonly lock.
RLock()

Expand Down Expand Up @@ -903,3 +907,8 @@ func (d *dirInode) ShouldInvalidateKernelListCache(ttl time.Duration) bool {
cachedDuration := d.cacheClock.Now().Sub(d.prevDirListingTimeStamp)
return cachedDuration >= ttl
}

func (d *dirInode) InvalidateKernelListCache() {
// Set prevDirListingTimeStamp to Zero time so that cache is invalidated.
d.prevDirListingTimeStamp = time.Time{}
}
9 changes: 9 additions & 0 deletions internal/fs/inode/dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1523,3 +1523,12 @@ func (t *DirTest) Test_ShouldInvalidateKernelListCache_ZeroTtl() {

AssertEq(true, shouldInvalidate)
}

func (t *DirTest) Test_InvalidateKernelListCache() {
d := t.in.(*dirInode)
d.prevDirListingTimeStamp = d.cacheClock.Now()

t.in.InvalidateKernelListCache()

AssertTrue(d.prevDirListingTimeStamp.IsZero())
}
61 changes: 61 additions & 0 deletions internal/fs/kernel_list_cache_inifinite_ttl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package fs_test

import (
"errors"
"os"
"path"
"testing"
Expand Down Expand Up @@ -86,3 +87,63 @@ func (t *KernelListCacheTestWithInfiniteTtl) TestKernelListCache_AlwaysCacheHit(
assert.Equal(t.T(), "file1.txt", names2[0])
assert.Equal(t.T(), "file2.txt", names2[1])
}

func (t *KernelListCacheTestWithInfiniteTtl) TestKernelListCache_RemoveDirAfterListCachedWorks() {
// First read, kernel will cache the dir response.
f, err := os.Open(path.Join(mntDir, "explicitDir"))
assert.Nil(t.T(), err)
names1, err := f.Readdirnames(-1)
assert.Nil(t.T(), err)
require.Equal(t.T(), 2, len(names1))
assert.Equal(t.T(), "file1.txt", names1[0])
assert.Equal(t.T(), "file2.txt", names1[1])
err = f.Close()
assert.Nil(t.T(), err)
// Adding one object to make sure to change the ReadDir() response.
assert.Nil(t.T(), t.createObjects(map[string]string{
"explicitDir/file3.txt": "123456",
}))
// Advancing time by 5 years (157800000 seconds).
cacheClock.AdvanceTime(157800000 * time.Second)

// os.RemoveDir should delete all files and invalidate cache.
err = os.RemoveAll(path.Join(mntDir, "explicitDir"))
assert.NoError(t.T(), err)

_, err = os.ReadDir(path.Join(mntDir, "explicitDir"))
var pathError *os.PathError
assert.True(t.T(), errors.As(err, &pathError))
}

func (t *KernelListCacheTestWithInfiniteTtl) TestKernelListCache_RemoveDirAfterListCacheInvalidatesCache() {
// First read, kernel will cache the dir response.
f, err := os.Open(path.Join(mntDir, "explicitDir"))
assert.Nil(t.T(), err)
names1, err := f.Readdirnames(-1)
assert.Nil(t.T(), err)
require.Equal(t.T(), 2, len(names1))
assert.Equal(t.T(), "file1.txt", names1[0])
assert.Equal(t.T(), "file2.txt", names1[1])
err = f.Close()
assert.Nil(t.T(), err)
// Adding one object to make sure to change the ReadDir() response.
assert.Nil(t.T(), t.createObjects(map[string]string{
"explicitDir/file3.txt": "123456",
}))
// Advancing time by 5 years (157800000 seconds).
cacheClock.AdvanceTime(157800000 * time.Second)
// os.RemoveDir should delete all files and invalidate cache.
err = os.RemoveAll(path.Join(mntDir, "explicitDir"))
assert.NoError(t.T(), err)

// Adding one more object to make sure to change the ReadDir() response.
assert.Nil(t.T(), t.createObjects(map[string]string{
"explicitDir/file4.txt": "123456",
}))
names2, err := os.ReadDir(path.Join(mntDir, "explicitDir"))
assert.Nil(t.T(), err)

assert.Nil(t.T(), err)
require.Equal(t.T(), 1, len(names2))
assert.Equal(t.T(), "file4.txt", names2[0].Name())
}
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,36 @@ func (s *infiniteKernelListCacheTest) TestKernelListCache_CacheMissOnFileRename(
assert.Equal(t, "renamed_file2.txt", names2[2])
}

// (a) First ReadDir() will be served from GCSFuse filesystem.
// (b) Second RmDIr() will also be served from GCSFuse filesystem, but it will cache the response.
// (c) Third ReadDir() will serve the read request from cache.
func (s *infiniteKernelListCacheTest) TestKernelListCache_ListAndDeleteDirectory(t *testing.T) {
targetDir := path.Join(testDirPath, "explicit_dir")
operations.CreateDirectory(targetDir, t)
// Create test data
f1 := operations.CreateFile(path.Join(targetDir, "file1.txt"), setup.FilePermission_0600, t)
operations.CloseFile(f1)
f2 := operations.CreateFile(path.Join(targetDir, "file2.txt"), setup.FilePermission_0600, t)
operations.CloseFile(f2)

// (a) First read served from GCS, kernel will cache the dir response.
f, err := os.Open(targetDir)
assert.NoError(t, err)
names1, err := f.Readdirnames(-1)
assert.NoError(t, err)
require.Equal(t, 2, len(names1))
require.Equal(t, "file1.txt", names1[0])
require.Equal(t, "file2.txt", names1[1])
err = f.Close()
assert.NoError(t, err)

// Adding one object to make sure to change the ReadDir() response.
client.CreateObjectInGCSTestDir(ctx, storageClient, testDirName, path.Join("explicit_dir", "file3.txt"), "", t)

err = os.RemoveAll(targetDir)
require.NoError(t, err)
}

////////////////////////////////////////////////////////////////////////
// Test Function (Runs once before all tests)
////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit d98eca8

Please sign in to comment.