Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions common/lock_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ func NewLockMap() *LockMap {

// Get the lock item based on file name, if item does not exists create it
func (l *LockMap) Get(name string) *LockMapItem {
// avoid allocating a new item when one already exists (the common case)
if lockIntf, found := l.locks.Load(name); found {
return lockIntf.(*LockMapItem)
}
lockIntf, _ := l.locks.LoadOrStore(name, &LockMapItem{handleCount: 0})
item := lockIntf.(*LockMapItem)
Comment thread
foodprocessor marked this conversation as resolved.
return item
Expand Down
18 changes: 13 additions & 5 deletions component/file_cache/file_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -2205,12 +2205,8 @@ func (fc *FileCache) GetAttr(options internal.GetAttrOptions) (*internal.ObjAttr

// Path in local cache, open, and dirty so cache is the source of truth for attributes.
localPath := filepath.Join(fc.tmpPath, options.Name)
info, localErr := os.Stat(localPath)
if localErr != nil && !isNotExist(localErr) {
log.Warn("FileCache::GetAttr : %s unexpected stat error [%v]", options.Name, localErr)
}
if flock.Count() > 0 && flock.DirtyCount() > 0 {
if localErr == nil && !info.IsDir() {
if info, err := os.Stat(localPath); err == nil && !info.IsDir() {
flock.RUnlock()
return newObjAttr(options.Name, info), nil
}
Expand All @@ -2219,7 +2215,19 @@ func (fc *FileCache) GetAttr(options internal.GetAttrOptions) (*internal.ObjAttr
// To cover case 1, get attributes from storage
inCloud := false
attrs, remoteErr := fc.NextComponent().GetAttr(options)

// Only stat the local copy when it can change the answer: the file is tracked by the cache policy
// (case 3), or cloud storage did not return it (case 2). This avoids a syscall on most lookups.
var info os.FileInfo
localErr := os.ErrNotExist
if remoteErr != nil || attrs == nil || (!attrs.IsDir() && fc.policy.IsCached(localPath)) {
info, localErr = os.Stat(localPath)
if localErr != nil && !isNotExist(localErr) {
log.Warn("FileCache::GetAttr : %s unexpected stat error [%v]", options.Name, localErr)
}
}
flock.RUnlock()

switch {
case remoteErr == nil: // object found
inCloud = true
Expand Down
56 changes: 56 additions & 0 deletions component/file_cache/file_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3330,6 +3330,62 @@ func (suite *fileCacheTestSuite) TestGetAttrDirtyOpenHandle() {
suite.assert.NoError(err)
}

func (suite *fileCacheTestSuite) TestGetAttrLocalOverlayRequiresCachePolicy() {
// enable mock component
suite.cleanupTest()
defaultConfig := fmt.Sprintf(
"file_cache:\n path: %s\n offload-io: true",
suite.cache_path,
)
suite.useMock = true
suite.setupTestHelper(defaultConfig)
defer suite.cleanupTest()

file := "overlay-file"
localPath := filepath.Join(suite.cache_path, file)
err := os.WriteFile(localPath, []byte("local data"), 0777)
suite.assert.NoError(err)
cloudAttr := &internal.ObjAttr{Path: file, Name: file, Size: 3, Mtime: time.Now()}
suite.mock.EXPECT().
GetAttr(internal.GetAttrOptions{Name: file}).
Return(cloudAttr, nil).
Times(2)

// a stray local file the cache policy does not track would be re-downloaded, so cloud wins
attr, err := suite.fileCache.GetAttr(internal.GetAttrOptions{Name: file})
suite.assert.NoError(err)
suite.assert.EqualValues(3, attr.Size)

// a tracked local file overrides the cloud size and mtime
suite.fileCache.policy.CacheValid(localPath)
attr, err = suite.fileCache.GetAttr(internal.GetAttrOptions{Name: file})
suite.assert.NoError(err)
suite.assert.EqualValues(len("local data"), attr.Size)
suite.assert.EqualValues(3, cloudAttr.Size, "cloud attributes must not be modified")
}

func (suite *fileCacheTestSuite) TestGetAttrDirectoryIgnoresLocalCopy() {
// enable mock component
suite.cleanupTest()
defaultConfig := fmt.Sprintf(
"file_cache:\n path: %s\n offload-io: true",
suite.cache_path,
)
suite.useMock = true
suite.setupTestHelper(defaultConfig)
defer suite.cleanupTest()

dir := "overlay-dir"
err := os.Mkdir(filepath.Join(suite.cache_path, dir), 0777)
suite.assert.NoError(err)
cloudAttr := internal.CreateObjAttrDir(dir)
suite.mock.EXPECT().GetAttr(internal.GetAttrOptions{Name: dir}).Return(cloudAttr, nil)

attr, err := suite.fileCache.GetAttr(internal.GetAttrOptions{Name: dir})
suite.assert.NoError(err)
suite.assert.Same(cloudAttr, attr)
}

func (suite *fileCacheTestSuite) TestGetAttrCase4() {
defer suite.cleanupTest()

Expand Down
15 changes: 1 addition & 14 deletions component/file_cache/lru_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,21 +354,8 @@ func (p *lruPolicy) CachePurge(name string) {
}

func (p *lruPolicy) IsCached(name string) bool {
log.Trace("lruPolicy::IsCached : %s", name)

val, found := p.nodeMap.Load(name)
if found {
node := val.(*lruNode)
node.RLock()
defer node.RUnlock()
deleted := node.deleted.Load()
log.Debug("lruPolicy::IsCached : %s, deleted:%t", name, deleted)
if !deleted {
return true
}
}
log.Trace("lruPolicy::IsCached : %s, found %t", name, found)
return false
return found && !val.(*lruNode).deleted.Load()
}

func (p *lruPolicy) Name() string {
Expand Down
Loading