From ee3d7ad2b3b0fab9e2ddf78801fb0e00d8609c09 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 1 Feb 2024 14:42:24 -0700 Subject: [PATCH 01/85] Added check on Atime on item to condition --- component/file_cache/file_cache_linux.go | 3 ++- component/file_cache/file_cache_windows.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache_linux.go b/component/file_cache/file_cache_linux.go index d991cc953..f7df4e432 100644 --- a/component/file_cache/file_cache_linux.go +++ b/component/file_cache/file_cache_linux.go @@ -69,6 +69,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock downloadRequired := false lmt := time.Time{} var stat *syscall.Stat_t = nil + accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // The file is not cached then we need to download if !fc.policy.IsCached(localPath) { @@ -96,7 +97,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) { + } else if os.IsNotExist(err) && !accessTime.IsZero() { // The file does not exist in the local cache so it needs to be downloaded log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) downloadRequired = true diff --git a/component/file_cache/file_cache_windows.go b/component/file_cache/file_cache_windows.go index 0b1e3bb38..f0e15b7d3 100644 --- a/component/file_cache/file_cache_windows.go +++ b/component/file_cache/file_cache_windows.go @@ -77,6 +77,8 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock } finfo, err := os.Stat(localPath) + accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if err == nil { // The file exists in local cache // The file needs to be downloaded if the cacheTimeout elapsed (check last change time and last modified time) @@ -96,7 +98,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) { + } else if os.IsNotExist(err) && !accessTime.IsZero() { // The file does not exist in the local cache so it needs to be downloaded log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) downloadRequired = true From 6391dd52c93878ce32a9bcb975eb4978d12e35a8 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 1 Feb 2024 14:50:12 -0700 Subject: [PATCH 02/85] WIP nested if to check Atime --- component/file_cache/file_cache_linux.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/component/file_cache/file_cache_linux.go b/component/file_cache/file_cache_linux.go index f7df4e432..d677ed98c 100644 --- a/component/file_cache/file_cache_linux.go +++ b/component/file_cache/file_cache_linux.go @@ -69,7 +69,6 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock downloadRequired := false lmt := time.Time{} var stat *syscall.Stat_t = nil - accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // The file is not cached then we need to download if !fc.policy.IsCached(localPath) { @@ -97,10 +96,15 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) && !accessTime.IsZero() { - // The file does not exist in the local cache so it needs to be downloaded - log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) - downloadRequired = true + } else if os.IsNotExist(err) { + stat = finfo.Sys().(*syscall.Stat_t) + accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) + if !accessTime.IsZero() { + // The file does not exist in the local cache so it needs to be downloaded + log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) + downloadRequired = true + } + } else { // Catch all, the file needs to be downloaded log.Debug("FileCache::isDownloadRequired : error calling stat %s [%s]", localPath, err.Error()) From 5934ceff95d6bd2541790d8bb6de855cd7188da5 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 13 Feb 2024 08:03:16 -0700 Subject: [PATCH 03/85] Revert "WIP nested if to check Atime" This reverts commit 6391dd52c93878ce32a9bcb975eb4978d12e35a8. --- component/file_cache/file_cache_linux.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/component/file_cache/file_cache_linux.go b/component/file_cache/file_cache_linux.go index d677ed98c..f7df4e432 100644 --- a/component/file_cache/file_cache_linux.go +++ b/component/file_cache/file_cache_linux.go @@ -69,6 +69,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock downloadRequired := false lmt := time.Time{} var stat *syscall.Stat_t = nil + accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // The file is not cached then we need to download if !fc.policy.IsCached(localPath) { @@ -96,15 +97,10 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) { - stat = finfo.Sys().(*syscall.Stat_t) - accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) - if !accessTime.IsZero() { - // The file does not exist in the local cache so it needs to be downloaded - log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) - downloadRequired = true - } - + } else if os.IsNotExist(err) && !accessTime.IsZero() { + // The file does not exist in the local cache so it needs to be downloaded + log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) + downloadRequired = true } else { // Catch all, the file needs to be downloaded log.Debug("FileCache::isDownloadRequired : error calling stat %s [%s]", localPath, err.Error()) From df7a8f34ee8959698ba78cc7d338b9f98f7a9696 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 13 Feb 2024 08:04:59 -0700 Subject: [PATCH 04/85] Revert "Added check on Atime on item to condition" This reverts commit ee3d7ad2b3b0fab9e2ddf78801fb0e00d8609c09. --- component/file_cache/file_cache_linux.go | 3 +-- component/file_cache/file_cache_windows.go | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/component/file_cache/file_cache_linux.go b/component/file_cache/file_cache_linux.go index f7df4e432..d991cc953 100644 --- a/component/file_cache/file_cache_linux.go +++ b/component/file_cache/file_cache_linux.go @@ -69,7 +69,6 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock downloadRequired := false lmt := time.Time{} var stat *syscall.Stat_t = nil - accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) // The file is not cached then we need to download if !fc.policy.IsCached(localPath) { @@ -97,7 +96,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) && !accessTime.IsZero() { + } else if os.IsNotExist(err) { // The file does not exist in the local cache so it needs to be downloaded log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) downloadRequired = true diff --git a/component/file_cache/file_cache_windows.go b/component/file_cache/file_cache_windows.go index f0e15b7d3..0b1e3bb38 100644 --- a/component/file_cache/file_cache_windows.go +++ b/component/file_cache/file_cache_windows.go @@ -77,8 +77,6 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock } finfo, err := os.Stat(localPath) - accessTime := time.Unix(int64(stat.Atim.Sec), int64(stat.Atim.Nsec)) - if err == nil { // The file exists in local cache // The file needs to be downloaded if the cacheTimeout elapsed (check last change time and last modified time) @@ -98,7 +96,7 @@ func (fc *FileCache) isDownloadRequired(localPath string, blobPath string, flock log.Debug("FileCache::isDownloadRequired : %s not valid as per time checks", localPath) downloadRequired = true } - } else if os.IsNotExist(err) && !accessTime.IsZero() { + } else if os.IsNotExist(err) { // The file does not exist in the local cache so it needs to be downloaded log.Debug("FileCache::isDownloadRequired : %s not present in local cache", localPath) downloadRequired = true From dafa99614a5a0d915d12cbb15c78c93ee6f8bc42 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 5 Mar 2024 15:13:39 -0700 Subject: [PATCH 05/85] added debug lines in Read() and Write() --- component/libfuse/libfuse2_handler.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index ef8a19a15..cb58244e4 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -579,6 +579,7 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { // Read reads data from a file into the buffer with the given offset. func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { + log.Debug("Libfuse::Read : reading path %s, handle: %d", path, fh) // Get the filehandle handle, exists := handlemap.Load(handlemap.HandleID(fh)) if !exists { @@ -617,6 +618,7 @@ func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { // Write writes data to a file from the buffer with the given offset. func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int { + log.Debug("Libfuse::Write : Writing path %s, handle: %d", path, fh) // Get the filehandle handle, exists := handlemap.Load(handlemap.HandleID(fh)) if !exists { From 8483de974ff214c498b1237ea8962a79c611eafb Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 18 Mar 2024 13:18:54 -0600 Subject: [PATCH 06/85] WIP Moved file_cache : Openfile call to Read() and Write() --- component/libfuse/libfuse2_handler.go | 46 +++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index cb58244e4..d5d49ccad 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -544,12 +544,26 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { name = common.NormalizeObjectName(name) log.Trace("Libfuse::Open : %s", name) - handle, err := fuseFS.NextComponent().OpenFile( - internal.OpenFileOptions{ - Name: name, - Flags: flags, - Mode: fs.FileMode(fuseFS.filePermission), - }) + //Create an empty file and use a handle with it for now + err := os.MkdirAll(path, common.DefaultAllowOtherPermissionBits) + if err != nil { + log.Err("FileCache::OpenFile : error creating directory structure for file %s [%s]", name, err.Error()) + } + f, err := common.OpenFile(path, os.O_CREATE|os.O_RDWR, common.DefaultAllowOtherPermissionBits) + if err != nil { + log.Err("FileCache::OpenFile : error creating new file %s [%s]", name, err.Error()) + } + + handle := handlemap.NewHandle(name) + inf, err := f.Stat() + if err == nil { + handle.Size = inf.Size() + } + handle.UnixFD = uint64(f.Fd()) + handle.Flags.Set(handlemap.HandleFlagCached) + + log.Info("FileCache::OpenFile : file=%s, fd=%d", name, f.Fd()) + handle.SetFileObject(f) if err != nil { log.Err("Libfuse::Open : Failed to open %s [%s]", name, err.Error()) @@ -587,6 +601,16 @@ func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { return -fuse.EBADF } + name := trimFusePath(path) + name = common.NormalizeObjectName(name) + + handle, _ = fuseFS.NextComponent().OpenFile( + internal.OpenFileOptions{ + Name: name, + Flags: 0, + Mode: fs.FileMode(fuseFS.filePermission), + }) + offset := uint64(ofst) var err error @@ -626,6 +650,16 @@ func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int return -fuse.EBADF } + name := trimFusePath(path) + name = common.NormalizeObjectName(name) + + handle, _ = fuseFS.NextComponent().OpenFile( + internal.OpenFileOptions{ + Name: name, + Flags: 0, + Mode: fs.FileMode(fuseFS.filePermission), + }) + bytesWritten, err := fuseFS.NextComponent().WriteFile( internal.WriteFileOptions{ Handle: handle, From bd0b50b03d5acad00d4b23bd4bec42fe7cada76d Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 18 Mar 2024 16:01:59 -0600 Subject: [PATCH 07/85] WIP Removed open() call in Write(). Troubleshooting file interface lag and write issues. --- component/libfuse/libfuse2_handler.go | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index d5d49ccad..a5829184c 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -545,14 +545,10 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { log.Trace("Libfuse::Open : %s", name) //Create an empty file and use a handle with it for now - err := os.MkdirAll(path, common.DefaultAllowOtherPermissionBits) + f, err := os.Create(name) if err != nil { log.Err("FileCache::OpenFile : error creating directory structure for file %s [%s]", name, err.Error()) } - f, err := common.OpenFile(path, os.O_CREATE|os.O_RDWR, common.DefaultAllowOtherPermissionBits) - if err != nil { - log.Err("FileCache::OpenFile : error creating new file %s [%s]", name, err.Error()) - } handle := handlemap.NewHandle(name) inf, err := f.Stat() @@ -650,16 +646,6 @@ func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int return -fuse.EBADF } - name := trimFusePath(path) - name = common.NormalizeObjectName(name) - - handle, _ = fuseFS.NextComponent().OpenFile( - internal.OpenFileOptions{ - Name: name, - Flags: 0, - Mode: fs.FileMode(fuseFS.filePermission), - }) - bytesWritten, err := fuseFS.NextComponent().WriteFile( internal.WriteFileOptions{ Handle: handle, From 3edf4303ff7ab397b5be97ddd26ea9d06b71b07c Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 29 Apr 2024 10:41:59 -0600 Subject: [PATCH 08/85] Revert "WIP Removed open() call in Write(). Troubleshooting file interface lag and write issues." This reverts commit bd0b50b03d5acad00d4b23bd4bec42fe7cada76d. --- component/libfuse/libfuse2_handler.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index a5829184c..d5d49ccad 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -545,10 +545,14 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { log.Trace("Libfuse::Open : %s", name) //Create an empty file and use a handle with it for now - f, err := os.Create(name) + err := os.MkdirAll(path, common.DefaultAllowOtherPermissionBits) if err != nil { log.Err("FileCache::OpenFile : error creating directory structure for file %s [%s]", name, err.Error()) } + f, err := common.OpenFile(path, os.O_CREATE|os.O_RDWR, common.DefaultAllowOtherPermissionBits) + if err != nil { + log.Err("FileCache::OpenFile : error creating new file %s [%s]", name, err.Error()) + } handle := handlemap.NewHandle(name) inf, err := f.Stat() @@ -646,6 +650,16 @@ func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int return -fuse.EBADF } + name := trimFusePath(path) + name = common.NormalizeObjectName(name) + + handle, _ = fuseFS.NextComponent().OpenFile( + internal.OpenFileOptions{ + Name: name, + Flags: 0, + Mode: fs.FileMode(fuseFS.filePermission), + }) + bytesWritten, err := fuseFS.NextComponent().WriteFile( internal.WriteFileOptions{ Handle: handle, From 771c3294c5c489d40343c332b6ac0b165fb20807 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 29 Apr 2024 10:43:31 -0600 Subject: [PATCH 09/85] Revert "WIP Moved file_cache : Openfile call to Read() and Write()" This reverts commit 8483de974ff214c498b1237ea8962a79c611eafb. --- component/libfuse/libfuse2_handler.go | 46 ++++----------------------- 1 file changed, 6 insertions(+), 40 deletions(-) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index d5d49ccad..cb58244e4 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -544,26 +544,12 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { name = common.NormalizeObjectName(name) log.Trace("Libfuse::Open : %s", name) - //Create an empty file and use a handle with it for now - err := os.MkdirAll(path, common.DefaultAllowOtherPermissionBits) - if err != nil { - log.Err("FileCache::OpenFile : error creating directory structure for file %s [%s]", name, err.Error()) - } - f, err := common.OpenFile(path, os.O_CREATE|os.O_RDWR, common.DefaultAllowOtherPermissionBits) - if err != nil { - log.Err("FileCache::OpenFile : error creating new file %s [%s]", name, err.Error()) - } - - handle := handlemap.NewHandle(name) - inf, err := f.Stat() - if err == nil { - handle.Size = inf.Size() - } - handle.UnixFD = uint64(f.Fd()) - handle.Flags.Set(handlemap.HandleFlagCached) - - log.Info("FileCache::OpenFile : file=%s, fd=%d", name, f.Fd()) - handle.SetFileObject(f) + handle, err := fuseFS.NextComponent().OpenFile( + internal.OpenFileOptions{ + Name: name, + Flags: flags, + Mode: fs.FileMode(fuseFS.filePermission), + }) if err != nil { log.Err("Libfuse::Open : Failed to open %s [%s]", name, err.Error()) @@ -601,16 +587,6 @@ func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { return -fuse.EBADF } - name := trimFusePath(path) - name = common.NormalizeObjectName(name) - - handle, _ = fuseFS.NextComponent().OpenFile( - internal.OpenFileOptions{ - Name: name, - Flags: 0, - Mode: fs.FileMode(fuseFS.filePermission), - }) - offset := uint64(ofst) var err error @@ -650,16 +626,6 @@ func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int return -fuse.EBADF } - name := trimFusePath(path) - name = common.NormalizeObjectName(name) - - handle, _ = fuseFS.NextComponent().OpenFile( - internal.OpenFileOptions{ - Name: name, - Flags: 0, - Mode: fs.FileMode(fuseFS.filePermission), - }) - bytesWritten, err := fuseFS.NextComponent().WriteFile( internal.WriteFileOptions{ Handle: handle, From dc3640d88852558e55a3e3ee562d934a5c765cfd Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 15 May 2024 15:18:41 -0600 Subject: [PATCH 10/85] modified ReadInBuffer() to recieve handleID and load handleID --- component/file_cache/file_cache.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index e51848e94..2d157e654 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -915,11 +915,18 @@ func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) } // ReadInBuffer: Read the local file into a buffer -func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, error) { +func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions, fh handlemap.HandleID) (int, error) { //defer exectime.StatTimeCurrentBlock("FileCache::ReadInBuffer")() // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) + handle, ok := handlemap.Load(fh) + if !ok { + log.Err("FileCache::ReadInBuffer : error [couldn't load handle from handlemap] %s", options.Handle.Path) + } else { + options.Handle = handle + } + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::ReadInBuffer : error [couldn't find fd in handle] %s", options.Handle.Path) From 8aa6960b7dda3584fbf31729a33bff7da3da067c Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 21 May 2024 13:52:53 -0600 Subject: [PATCH 11/85] Revert "modified ReadInBuffer() to recieve handleID and load handleID" This reverts commit dc3640d88852558e55a3e3ee562d934a5c765cfd. --- component/file_cache/file_cache.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 2d157e654..e51848e94 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -915,18 +915,11 @@ func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) } // ReadInBuffer: Read the local file into a buffer -func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions, fh handlemap.HandleID) (int, error) { +func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, error) { //defer exectime.StatTimeCurrentBlock("FileCache::ReadInBuffer")() // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) - handle, ok := handlemap.Load(fh) - if !ok { - log.Err("FileCache::ReadInBuffer : error [couldn't load handle from handlemap] %s", options.Handle.Path) - } else { - options.Handle = handle - } - f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::ReadInBuffer : error [couldn't find fd in handle] %s", options.Handle.Path) From 35712ba62ec1c6d7966cb9831e9d0820063a8f85 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 22 May 2024 16:26:10 -0600 Subject: [PATCH 12/85] set up download() and getHandleData() --- component/file_cache/file_cache.go | 252 ++++++++++++++++++++--------- 1 file changed, 174 insertions(+), 78 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index e51848e94..18a6bc1bd 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -691,120 +691,216 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { return nil } -// OpenFile: Makes the file available in the local cache for further file operations. -func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Handle, error) { - log.Trace("FileCache::OpenFile : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) +func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { + if handle.GetFileObject() == nil { + log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) + + //exctract the flags out of handle values + flags, _ := handle.GetValue("flag") + flagsStruct, ok := flags.(struct{ Number int }) + if !ok { + fmt.Println("Type assertion failed") + return nil + } - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) + // Extract the integer as a separate variable + flag := flagsStruct.Number + + // extract the filemode out of handle values + mode, _ := handle.GetValue("mode") + fileModeValue, ok := mode.(os.FileMode) + if !ok { + fmt.Println("Type assertion failed") + return nil + } + + handle, err := fc.download(handle.Path, flag, fileModeValue) + if err != nil { + log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) + } + + } + return handle +} + +func (fc *FileCache) download(name string, flag int, mode os.FileMode) (*handlemap.Handle, error) { + log.Trace("FileCache::download : name=%s, flags=%d, mode=%s", name, flag, mode) + + localPath := common.JoinUnixFilepath(fc.tmpPath, name) var f *os.File var err error - flock := fc.fileLocks.Get(options.Name) + flock := fc.fileLocks.Get(name) flock.Lock() defer flock.Unlock() fc.policy.CacheValid(localPath) - downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) + + attr, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: name}) + if err != nil { + log.Err("FileCache::download : Failed to get attr of %s [%s]", name, err.Error()) + } // return err in case of authorization permission mismatch if err != nil && err == syscall.EACCES { return nil, err } - if downloadRequired { - log.Debug("FileCache::OpenFile : Need to download %s", options.Name) + log.Debug("FileCache::download : Need to download %s", name) - fileSize := int64(0) - if attr != nil { - fileSize = int64(attr.Size) - } + fileSize := int64(0) + if attr != nil { + fileSize = int64(attr.Size) + } + _, err = os.Stat(localPath) + var fileExists bool + if err == nil { + // The file exists in local cache + fileExists = true + } - if fileExists { - log.Debug("FileCache::OpenFile : Delete cached file %s", options.Name) + if fileExists { + log.Debug("FileCache::download : Delete cached file %s", name) - err := deleteFile(localPath) - if err != nil && !os.IsNotExist(err) { - log.Err("FileCache::OpenFile : Failed to delete old file %s", options.Name) - } - } else { - // Create the file if if doesn't already exist. - err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) - if err != nil { - log.Err("FileCache::OpenFile : error creating directory structure for file %s [%s]", options.Name, err.Error()) - return nil, err - } + err := deleteFile(localPath) + if err != nil && !os.IsNotExist(err) { + log.Err("FileCache::download : Failed to delete old file %s", name) } - - // Open the file in write mode. - f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, options.Mode) + } else { + // Create the file if if doesn't already exist. + err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) if err != nil { - log.Err("FileCache::OpenFile : error creating new file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : error creating directory structure for file %s [%s]", name, err.Error()) return nil, err } + } - if options.Flags&os.O_TRUNC != 0 { - fileSize = 0 - } + // Open the file in write mode. + f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, mode) + if err != nil { + log.Err("FileCache::download : error creating new file %s [%s]", name, err.Error()) + return nil, err + } - if fileSize > 0 { - if fc.diskHighWaterMark != 0 { - currSize, err := common.GetUsage(fc.tmpPath) - if err != nil { - log.Err("FileCache::OpenFile : error getting current usage of cache [%s]", err.Error()) - } else { - if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::OpenFile : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) - return nil, syscall.ENOSPC - } - } + if flag&os.O_TRUNC != 0 { + fileSize = 0 + } - } - // Download/Copy the file from storage to the local file. - // We pass a count of 0 to get the entire object - err = fc.NextComponent().CopyToFile( - internal.CopyToFileOptions{ - Name: options.Name, - Offset: 0, - Count: 0, - File: f, - }) + if fileSize > 0 { + if fc.diskHighWaterMark != 0 { + currSize, err := common.GetUsage(fc.tmpPath) if err != nil { - // File was created locally and now download has failed so we need to delete it back from local cache - log.Err("FileCache::OpenFile : error downloading file from storage %s [%s]", options.Name, err.Error()) - _ = f.Close() - _ = os.Remove(localPath) - return nil, err + log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) + } else { + if (currSize + float64(fileSize)) > fc.diskHighWaterMark { + log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, name) + return nil, syscall.ENOSPC + } } + + } + // Download/Copy the file from storage to the local file. + // We pass a count of 0 to get the entire object + err = fc.NextComponent().CopyToFile( + internal.CopyToFileOptions{ + Name: name, + Offset: 0, + Count: 0, + File: f, + }) + if err != nil { + // File was created locally and now download has failed so we need to delete it back from local cache + log.Err("FileCache::download : error downloading file from storage %s [%s]", name, err.Error()) + _ = f.Close() + _ = os.Remove(localPath) + return nil, err } + } - // Update the last download time of this file - flock.SetDownloadTime() + // Update the last download time of this file + flock.SetDownloadTime() - log.Debug("FileCache::OpenFile : Download of %s is complete", options.Name) - f.Close() + log.Debug("FileCache::download : Download of %s is complete", name) + f.Close() - // After downloading the file, update the modified times and mode of the file. - fileMode := fc.defaultPermission - if attr != nil && !attr.IsModeDefault() { - fileMode = attr.Mode - } + // After downloading the file, update the modified times and mode of the file. + fileMode := fc.defaultPermission + if attr != nil && !attr.IsModeDefault() { + fileMode = attr.Mode + } - // If user has selected some non default mode in config then every local file shall be created with that mode only - err = os.Chmod(localPath, fileMode) + // If user has selected some non default mode in config then every local file shall be created with that mode only + err = os.Chmod(localPath, fileMode) + if err != nil { + log.Err("FileCache::download : Failed to change mode of file %s [%s]", name, err.Error()) + } + // TODO: When chown is supported should we update that? + + if attr != nil { + // chtimes shall be the last api otherwise calling chmod/chown will update the last change time + err = os.Chtimes(localPath, attr.Atime, attr.Mtime) if err != nil { - log.Err("FileCache::OpenFile : Failed to change mode of file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : Failed to change times of file %s [%s]", name, err.Error()) } - // TODO: When chown is supported should we update that? + } - if attr != nil { - // chtimes shall be the last api otherwise calling chmod/chown will update the last change time - err = os.Chtimes(localPath, attr.Atime, attr.Mtime) - if err != nil { - log.Err("FileCache::OpenFile : Failed to change times of file %s [%s]", options.Name, err.Error()) - } - } + fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) + + // Open the file and grab a shared lock to prevent deletion by the cache policy. + f, err = common.OpenFile(localPath, flag, mode) + if err != nil { + log.Err("FileCache::download : error opening cached file %s [%s]", name, err.Error()) + return nil, err + } + + // Increment the handle count in this lock item as there is one handle open for this now + flock.Inc() + + handle := handlemap.NewHandle(name) + inf, err := f.Stat() + if err == nil { + handle.Size = inf.Size() + } + + handle.UnixFD = uint64(f.Fd()) + if !fc.offloadIO { + handle.Flags.Set(handlemap.HandleFlagCached) + } + log.Info("FileCache::download : file=%s, fd=%d", name, f.Fd()) + handle.SetFileObject(f) + + return handle, nil +} + +// OpenFile: Makes the file available in the local cache for further file operations. +func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Handle, error) { + log.Trace("FileCache::OpenFile : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) + + localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) + var f *os.File + var err error + + flock := fc.fileLocks.Get(options.Name) + flock.Lock() + defer flock.Unlock() + + fc.policy.CacheValid(localPath) + downloadRequired, _, _, err := fc.isDownloadRequired(localPath, options.Name, flock) + + // return err in case of authorization permission mismatch + if err != nil && err == syscall.EACCES { + return nil, err + } + + if downloadRequired { fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) + handle := handlemap.NewHandle(options.Name) + handle.SetValue("flag", struct{ flags int }{flags: options.Flags}) + handle.SetValue("mode", options.Mode) + + return handle, nil + } else { log.Debug("FileCache::OpenFile : %s will be served from cache", options.Name) fileCacheStatsCollector.UpdateStats(stats_manager.Increment, cacheServed, (int64)(1)) From 1ea547162e5ceb73c5a77abe0dd73ae669b20348 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 22 May 2024 16:29:06 -0600 Subject: [PATCH 13/85] added call to getHandleData() from ReadInBuffer() --- component/file_cache/file_cache.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 18a6bc1bd..b50112f16 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1016,6 +1016,10 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) + if options.Handle.GetFileObject() == nil { + options.Handle = fc.getHandleData(options.Handle) + } + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::ReadInBuffer : error [couldn't find fd in handle] %s", options.Handle.Path) From cacc0b3360ff24e3609a0e0c9d2651a9ff61c65a Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 22 May 2024 19:20:00 -0600 Subject: [PATCH 14/85] added call to getHandleData() in WriteFile() --- component/file_cache/file_cache.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index b50112f16..cac878a88 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1050,6 +1050,9 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) + if options.Handle.GetFileObject() == nil { + options.Handle = fc.getHandleData(options.Handle) + } f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::WriteFile : error [couldn't find fd in handle] %s", options.Handle.Path) From 08ac9d9892e08545c9f4b5e2f3e1b6c299b8bb9c Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 23 May 2024 11:54:56 -0600 Subject: [PATCH 15/85] removed redundant fileObj nil check --- component/file_cache/file_cache.go | 46 +++++++++++++++--------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index cac878a88..5b263f7bf 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -692,34 +692,33 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { } func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { - if handle.GetFileObject() == nil { - log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) - - //exctract the flags out of handle values - flags, _ := handle.GetValue("flag") - flagsStruct, ok := flags.(struct{ Number int }) - if !ok { - fmt.Println("Type assertion failed") - return nil - } - // Extract the integer as a separate variable - flag := flagsStruct.Number + log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) - // extract the filemode out of handle values - mode, _ := handle.GetValue("mode") - fileModeValue, ok := mode.(os.FileMode) - if !ok { - fmt.Println("Type assertion failed") - return nil - } + //exctract the flags out of handle values + flags, _ := handle.GetValue("flag") + flagsStruct, ok := flags.(struct{ flags int }) + if !ok { + fmt.Println("Type assertion failed") + return nil + } - handle, err := fc.download(handle.Path, flag, fileModeValue) - if err != nil { - log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) - } + // Extract the integer as a separate variable + flag := flagsStruct.flags + + // extract the filemode out of handle values + mode, _ := handle.GetValue("mode") + fileModeValue, ok := mode.(os.FileMode) + if !ok { + fmt.Println("Type assertion failed") + return nil + } + handle, err := fc.download(handle.Path, flag, fileModeValue) + if err != nil { + log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) } + return handle } @@ -1016,6 +1015,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) + //TODO: troubleshoot. Why is options.Handle is still empty? if options.Handle.GetFileObject() == nil { options.Handle = fc.getHandleData(options.Handle) } From cdd2d771340dcb47c39520f9306ab1de3e8be7ec Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 23 May 2024 12:09:25 -0600 Subject: [PATCH 16/85] added additional handlemap.Handle var to replace options.Handle for ReadInBuffer() --- component/file_cache/file_cache.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 5b263f7bf..135d57aa5 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1015,11 +1015,14 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) + var newHandle *handlemap.Handle //TODO: troubleshoot. Why is options.Handle is still empty? if options.Handle.GetFileObject() == nil { - options.Handle = fc.getHandleData(options.Handle) + newHandle = fc.getHandleData(options.Handle) } + options.Handle = newHandle + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::ReadInBuffer : error [couldn't find fd in handle] %s", options.Handle.Path) From b5d1dfce22ba34377666748768be134fba4b53a5 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 23 May 2024 12:10:28 -0600 Subject: [PATCH 17/85] added additional handlemap.Handle var to replace options.Handle for WriteFile() --- component/file_cache/file_cache.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 135d57aa5..d0c866f74 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1053,9 +1053,13 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) + var newHandle *handlemap.Handle if options.Handle.GetFileObject() == nil { - options.Handle = fc.getHandleData(options.Handle) + newHandle = fc.getHandleData(options.Handle) } + + options.Handle = newHandle + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::WriteFile : error [couldn't find fd in handle] %s", options.Handle.Path) From e58cc12fa381210ff515fbeaeb6188bf8b94489c Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 23 May 2024 14:56:11 -0600 Subject: [PATCH 18/85] updated println() into log.Err and log.Debug --- component/file_cache/file_cache.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index d0c866f74..1e2dd4392 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -699,7 +699,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { flags, _ := handle.GetValue("flag") flagsStruct, ok := flags.(struct{ flags int }) if !ok { - fmt.Println("Type assertion failed") + log.Debug("FileCache::getHandleData : error Type assertion failed on getting flag for %s [%s]", handle.Path) return nil } @@ -710,7 +710,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { mode, _ := handle.GetValue("mode") fileModeValue, ok := mode.(os.FileMode) if !ok { - fmt.Println("Type assertion failed") + log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", handle.Path) return nil } @@ -1016,12 +1016,18 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) var newHandle *handlemap.Handle - //TODO: troubleshoot. Why is options.Handle is still empty? + var swapped bool + //TODO: troubleshoot. on the next offset instance to read the same file, options.Handle is empty after it was previously downloaded. + // the file is being downloaded over and over for each next offset to read from. if options.Handle.GetFileObject() == nil { newHandle = fc.getHandleData(options.Handle) + swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) + options.Handle = newHandle } - options.Handle = newHandle + if !swapped { + log.Err("FileCache::ReadInBuffer : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + } f := options.Handle.GetFileObject() if f == nil { @@ -1054,11 +1060,16 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) var newHandle *handlemap.Handle + var swapped bool if options.Handle.GetFileObject() == nil { newHandle = fc.getHandleData(options.Handle) + swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) + options.Handle = newHandle } - options.Handle = newHandle + if !swapped { + log.Err("FileCache::WriteFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + } f := options.Handle.GetFileObject() if f == nil { From 246bcf7b445e7388a24176d170995357c3f9d11c Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 23 May 2024 16:25:24 -0600 Subject: [PATCH 19/85] -changed test to call DownloadFile() -exported download() to DownlaodFile() for tests -added isDownloadRequired logic in TruncateFile() --- component/file_cache/file_cache.go | 24 ++++++++++++++++++------ component/file_cache/file_cache_test.go | 8 +++----- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 1e2dd4392..74df23842 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -714,7 +714,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { return nil } - handle, err := fc.download(handle.Path, flag, fileModeValue) + handle, err := fc.DownloadFile(handle.Path, flag, fileModeValue) if err != nil { log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) } @@ -722,7 +722,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { return handle } -func (fc *FileCache) download(name string, flag int, mode os.FileMode) (*handlemap.Handle, error) { +func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*handlemap.Handle, error) { log.Trace("FileCache::download : name=%s, flags=%d, mode=%s", name, flag, mode) localPath := common.JoinUnixFilepath(fc.tmpPath, name) @@ -1389,11 +1389,23 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } else { // If size is not 0 then we need to open the file and then truncate it // Open will force download if file was not present in local system - h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) - if err != nil { - log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) - return err + localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) + flock := fc.fileLocks.Get(options.Name) + downloadRequired, _, _, err := fc.isDownloadRequired(localPath, options.Name, flock) + if downloadRequired { + _, err := fc.DownloadFile(options.Name, os.O_RDWR, fc.defaultPermission) + if err != nil { + log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) + return err + } + } else { + h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) + if err != nil { + log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) + return err + } } + } // Update the size of the file in the local cache diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index a142b9e2b..28552d889 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -765,7 +765,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { } // Download is required - handle, err = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) + handle, err = suite.fileCache.DownloadFile(path, 0, os.FileMode(0777)) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1589,16 +1589,14 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { suite.assert.NoError(err) // try opening small file - options := internal.OpenFileOptions{Name: pathsmall, Mode: 0777} - f, err := suite.fileCache.OpenFile(options) + f, err := suite.fileCache.DownloadFile(pathsmall, 0, os.FileMode(0777)) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - options = internal.OpenFileOptions{Name: pathbig, Mode: 0777} - f, err = suite.fileCache.OpenFile(options) + f, err = suite.fileCache.DownloadFile(pathbig, 0, os.FileMode(0777)) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) From 0a69842e74d87ec5ad547d5d7c4c68014a14e867 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Fri, 24 May 2024 12:20:41 -0600 Subject: [PATCH 20/85] added DownloadFileOptions struct and adjusted function parameters and calls respectively --- component/file_cache/file_cache.go | 56 ++++++++++++++----------- component/file_cache/file_cache_test.go | 6 +-- internal/component_options.go | 6 +++ 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 74df23842..16eb2ea6f 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -714,7 +714,8 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { return nil } - handle, err := fc.DownloadFile(handle.Path, flag, fileModeValue) + handle, err := fc.DownloadFile(internal.DownloadFileOptions{Name: handle.Path, Flags: flag, Mode: fileModeValue}) + if err != nil { log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) } @@ -722,22 +723,22 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { return handle } -func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*handlemap.Handle, error) { - log.Trace("FileCache::download : name=%s, flags=%d, mode=%s", name, flag, mode) +func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { + log.Trace("FileCache::download : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) - localPath := common.JoinUnixFilepath(fc.tmpPath, name) + localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) var f *os.File var err error - flock := fc.fileLocks.Get(name) + flock := fc.fileLocks.Get(options.Name) flock.Lock() defer flock.Unlock() fc.policy.CacheValid(localPath) - attr, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: name}) + attr, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name}) if err != nil { - log.Err("FileCache::download : Failed to get attr of %s [%s]", name, err.Error()) + log.Err("FileCache::download : Failed to get attr of %s [%s]", options.Name, err.Error()) } // return err in case of authorization permission mismatch @@ -745,7 +746,7 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han return nil, err } - log.Debug("FileCache::download : Need to download %s", name) + log.Debug("FileCache::download : Need to download %s", options.Name) fileSize := int64(0) if attr != nil { @@ -759,29 +760,29 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han } if fileExists { - log.Debug("FileCache::download : Delete cached file %s", name) + log.Debug("FileCache::download : Delete cached file %s", options.Name) err := deleteFile(localPath) if err != nil && !os.IsNotExist(err) { - log.Err("FileCache::download : Failed to delete old file %s", name) + log.Err("FileCache::download : Failed to delete old file %s", options.Name) } } else { // Create the file if if doesn't already exist. err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) if err != nil { - log.Err("FileCache::download : error creating directory structure for file %s [%s]", name, err.Error()) + log.Err("FileCache::download : error creating directory structure for file %s [%s]", options.Name, err.Error()) return nil, err } } // Open the file in write mode. - f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, mode) + f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, options.Mode) if err != nil { - log.Err("FileCache::download : error creating new file %s [%s]", name, err.Error()) + log.Err("FileCache::download : error creating new file %s [%s]", options.Name, err.Error()) return nil, err } - if flag&os.O_TRUNC != 0 { + if options.Flags&os.O_TRUNC != 0 { fileSize = 0 } @@ -792,7 +793,7 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) } else { if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, name) + log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) return nil, syscall.ENOSPC } } @@ -802,14 +803,14 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han // We pass a count of 0 to get the entire object err = fc.NextComponent().CopyToFile( internal.CopyToFileOptions{ - Name: name, + Name: options.Name, Offset: 0, Count: 0, File: f, }) if err != nil { // File was created locally and now download has failed so we need to delete it back from local cache - log.Err("FileCache::download : error downloading file from storage %s [%s]", name, err.Error()) + log.Err("FileCache::download : error downloading file from storage %s [%s]", options.Name, err.Error()) _ = f.Close() _ = os.Remove(localPath) return nil, err @@ -819,7 +820,7 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han // Update the last download time of this file flock.SetDownloadTime() - log.Debug("FileCache::download : Download of %s is complete", name) + log.Debug("FileCache::download : Download of %s is complete", options.Name) f.Close() // After downloading the file, update the modified times and mode of the file. @@ -831,7 +832,7 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han // If user has selected some non default mode in config then every local file shall be created with that mode only err = os.Chmod(localPath, fileMode) if err != nil { - log.Err("FileCache::download : Failed to change mode of file %s [%s]", name, err.Error()) + log.Err("FileCache::download : Failed to change mode of file %s [%s]", options.Name, err.Error()) } // TODO: When chown is supported should we update that? @@ -839,23 +840,23 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han // chtimes shall be the last api otherwise calling chmod/chown will update the last change time err = os.Chtimes(localPath, attr.Atime, attr.Mtime) if err != nil { - log.Err("FileCache::download : Failed to change times of file %s [%s]", name, err.Error()) + log.Err("FileCache::download : Failed to change times of file %s [%s]", options.Name, err.Error()) } } fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) // Open the file and grab a shared lock to prevent deletion by the cache policy. - f, err = common.OpenFile(localPath, flag, mode) + f, err = common.OpenFile(localPath, options.Flags, options.Mode) if err != nil { - log.Err("FileCache::download : error opening cached file %s [%s]", name, err.Error()) + log.Err("FileCache::download : error opening cached file %s [%s]", options.Name, err.Error()) return nil, err } // Increment the handle count in this lock item as there is one handle open for this now flock.Inc() - handle := handlemap.NewHandle(name) + handle := handlemap.NewHandle(options.Name) inf, err := f.Stat() if err == nil { handle.Size = inf.Size() @@ -866,7 +867,7 @@ func (fc *FileCache) DownloadFile(name string, flag int, mode os.FileMode) (*han handle.Flags.Set(handlemap.HandleFlagCached) } - log.Info("FileCache::download : file=%s, fd=%d", name, f.Fd()) + log.Info("FileCache::download : file=%s, fd=%d", options.Name, f.Fd()) handle.SetFileObject(f) return handle, nil @@ -1392,8 +1393,13 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) flock := fc.fileLocks.Get(options.Name) downloadRequired, _, _, err := fc.isDownloadRequired(localPath, options.Name, flock) + // return err in case of authorization permission mismatch + if err != nil && err == syscall.EACCES { + return err + } + if downloadRequired { - _, err := fc.DownloadFile(options.Name, os.O_RDWR, fc.defaultPermission) + _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 28552d889..8896b44cc 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -765,7 +765,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { } // Download is required - handle, err = suite.fileCache.DownloadFile(path, 0, os.FileMode(0777)) + handle, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Flags: 0, Mode: os.FileMode(0777)}) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1589,14 +1589,14 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { suite.assert.NoError(err) // try opening small file - f, err := suite.fileCache.DownloadFile(pathsmall, 0, os.FileMode(0777)) + f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Flags: 0, Mode: os.FileMode(0777)}) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.DownloadFile(pathbig, 0, os.FileMode(0777)) + f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Flags: 0, Mode: os.FileMode(0777)}) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) diff --git a/internal/component_options.go b/internal/component_options.go index ba7e831e9..01f4537da 100644 --- a/internal/component_options.go +++ b/internal/component_options.go @@ -77,6 +77,12 @@ type DeleteFileOptions struct { Name string } +type DownloadFileOptions struct { + Name string + Flags int + Mode os.FileMode +} + type OpenFileOptions struct { Name string Flags int From 0719b0e95a170442a425eac2fe1501c2df119f80 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Fri, 24 May 2024 12:54:34 -0600 Subject: [PATCH 21/85] chanced condition to be more specific for new handle produced by OpenFile() --- component/file_cache/file_cache.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 16eb2ea6f..57442c5ec 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1020,14 +1020,13 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er var swapped bool //TODO: troubleshoot. on the next offset instance to read the same file, options.Handle is empty after it was previously downloaded. // the file is being downloaded over and over for each next offset to read from. - if options.Handle.GetFileObject() == nil { + if _, ok := options.Handle.GetValue("flag"); ok { newHandle = fc.getHandleData(options.Handle) swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) options.Handle = newHandle - } - - if !swapped { - log.Err("FileCache::ReadInBuffer : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + if !swapped { + log.Err("FileCache::ReadInBuffer : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + } } f := options.Handle.GetFileObject() @@ -1062,14 +1061,14 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { var newHandle *handlemap.Handle var swapped bool - if options.Handle.GetFileObject() == nil { + if _, ok := options.Handle.GetValue("flag"); ok { newHandle = fc.getHandleData(options.Handle) swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) options.Handle = newHandle - } + if !swapped { + log.Err("FileCache::WriteFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + } - if !swapped { - log.Err("FileCache::WriteFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) } f := options.Handle.GetFileObject() From 9cfec2a8d79d4f597717e9659f363c1832960d88 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Fri, 24 May 2024 13:16:41 -0600 Subject: [PATCH 22/85] get handle out of download in TruncateFile() to fix test --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 57442c5ec..f2383eee2 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1398,7 +1398,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } if downloadRequired { - _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) + h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err From 963954a275b050f193d561ec3fec02ca446c4ce5 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Fri, 24 May 2024 19:01:57 -0600 Subject: [PATCH 23/85] adjusted TestSyncFile(), WriteFile(), and ReadInBuffer() to hold flag sync state --- component/file_cache/file_cache.go | 6 +++++- component/file_cache/file_cache_test.go | 10 +++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index f2383eee2..1468e6d12 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1021,7 +1021,9 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er //TODO: troubleshoot. on the next offset instance to read the same file, options.Handle is empty after it was previously downloaded. // the file is being downloaded over and over for each next offset to read from. if _, ok := options.Handle.GetValue("flag"); ok { + latestFlag := options.Handle.Flags newHandle = fc.getHandleData(options.Handle) + newHandle.Flags = latestFlag swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) options.Handle = newHandle if !swapped { @@ -1062,7 +1064,9 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { var newHandle *handlemap.Handle var swapped bool if _, ok := options.Handle.GetValue("flag"); ok { + latestFlag := options.Handle.Flags newHandle = fc.getHandleData(options.Handle) + newHandle.Flags = latestFlag swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) options.Handle = newHandle if !swapped { @@ -1099,12 +1103,12 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // Removing Pwrite as it is not supported on Windows // bytesWritten, err := syscall.Pwrite(options.Handle.FD(), options.Data, options.Offset) + bytesWritten, err := f.WriteAt(options.Data, options.Offset) if err == nil { // Mark the handle dirty so the file is written back to storage on FlushFile. options.Handle.Flags.Set(handlemap.HandleFlagDirty) - } else { log.Err("FileCache::WriteFile : failed to write %s [%s]", options.Handle.Path, err.Error()) } diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 8896b44cc..53752d325 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -664,15 +664,19 @@ func (suite *fileCacheTestSuite) TestSyncFile() { suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle}) // On a sync we open, sync, flush and close - handle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) + handle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: 0777}) + handlemap.Add(handle) suite.assert.NoError(err) - err = suite.fileCache.SyncFile(internal.SyncFileOptions{Handle: handle}) + err = suite.fileCache.SyncFile(internal.SyncFileOptions{Handle: handle}) //sync flag set here gets wiped in WriteFile suite.assert.NoError(err) testData := "test data" data := []byte(testData) + suite.fileCache.WriteFile(internal.WriteFileOptions{Handle: handle, Offset: 0, Data: data}) + handle, loaded := handlemap.Load(handle.ID) + suite.assert.True(loaded) suite.fileCache.FlushFile(internal.FlushFileOptions{Handle: handle}) - suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle}) + suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle}) //file isn't getting deleted Handle.Fsynced() is false // Path should not be in file cache _, err = os.Stat(common.JoinUnixFilepath(suite.cache_path, path)) From bc04cdcbf767652a984afee85e811b0fc3291c4b Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 28 May 2024 13:28:53 -0600 Subject: [PATCH 24/85] Added handle storing and retrieval in TestReadFileWithRefresh() --- component/file_cache/file_cache_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 53752d325..ee912032a 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1537,13 +1537,16 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) data := make([]byte, 20) - options := internal.OpenFileOptions{Name: path, Mode: 0777} + options := internal.OpenFileOptions{Name: path, Flags: os.O_RDONLY, Mode: 0777} // Read file once and we shall get the same data f, err := suite.fileCache.OpenFile(options) + handlemap.Add(f) suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) + f, loaded := handlemap.Load(f.ID) + suite.assert.True(loaded) suite.assert.NoError(err) suite.assert.Equal(9, n) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) @@ -1566,9 +1569,12 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) time.Sleep(12 * time.Second) f, err = suite.fileCache.OpenFile(options) + handlemap.Add(f) suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) + f, loaded = handlemap.Load(f.ID) + suite.assert.True(loaded) suite.assert.NoError(err) suite.assert.Equal(15, n) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) From 64a0d6b523ddc2f39cbf05cba68ced2f6092e6b7 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 29 May 2024 11:09:27 -0600 Subject: [PATCH 25/85] added sync.mutex thread safe logic into getHandleData() --- component/file_cache/file_cache.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 1468e6d12..53e3993bb 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -73,6 +73,7 @@ type FileCache struct { refreshSec uint32 hardLimit bool diskHighWaterMark float64 + mu sync.Mutex } // Structure defining your config parameters @@ -695,6 +696,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) + fc.mu.Lock() //exctract the flags out of handle values flags, _ := handle.GetValue("flag") flagsStruct, ok := flags.(struct{ flags int }) @@ -718,8 +720,14 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { if err != nil { log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) + return handle } + handle.RemoveValue("flag") + handle.RemoveValue("mode") + + fc.mu.Unlock() + return handle } From ef2234f9ec67a74d613ef6b0bb1ee65bb5f9b57d Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 29 May 2024 11:12:10 -0600 Subject: [PATCH 26/85] added download logic into ReadFile() --- component/file_cache/file_cache.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 53e3993bb..05b49fb69 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -992,6 +992,20 @@ func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { // ReadFile: Read the local file func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. + + var newHandle *handlemap.Handle + var swapped bool + if _, ok := options.Handle.GetValue("flag"); ok { + latestFlag := options.Handle.Flags + newHandle = fc.getHandleData(options.Handle) + newHandle.Flags = latestFlag + swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) + options.Handle = newHandle + if !swapped { + log.Err("FileCache::ReadFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) + } + } + localPath := common.JoinUnixFilepath(fc.tmpPath, options.Handle.Path) fc.policy.CacheValid(localPath) @@ -1026,8 +1040,6 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er var newHandle *handlemap.Handle var swapped bool - //TODO: troubleshoot. on the next offset instance to read the same file, options.Handle is empty after it was previously downloaded. - // the file is being downloaded over and over for each next offset to read from. if _, ok := options.Handle.GetValue("flag"); ok { latestFlag := options.Handle.Flags newHandle = fc.getHandleData(options.Handle) From 40e2ae41a30edeffb9882190bcbbecc486285fec Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 29 May 2024 15:47:45 -0600 Subject: [PATCH 27/85] removed download portion of ReadFile() --- component/file_cache/file_cache.go | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 05b49fb69..d58bdd7c9 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -993,19 +993,6 @@ func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. - var newHandle *handlemap.Handle - var swapped bool - if _, ok := options.Handle.GetValue("flag"); ok { - latestFlag := options.Handle.Flags - newHandle = fc.getHandleData(options.Handle) - newHandle.Flags = latestFlag - swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) - options.Handle = newHandle - if !swapped { - log.Err("FileCache::ReadFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) - } - } - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Handle.Path) fc.policy.CacheValid(localPath) From be9ecbec6eab39593c4bd6693dbbc42c40e89305 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 30 May 2024 13:42:33 -0600 Subject: [PATCH 28/85] -removed extra handle being used to swap into handlemap -added handle option in DownloadFileOptions -fixed all references to DownloadFile() to include handle in options. --- component/file_cache/file_cache.go | 40 +++++++------------------ component/file_cache/file_cache_test.go | 38 +++++++++++++++++++++-- internal/component_options.go | 7 +++-- 3 files changed, 49 insertions(+), 36 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index d58bdd7c9..4bfab8ab7 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -716,7 +716,7 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { return nil } - handle, err := fc.DownloadFile(internal.DownloadFileOptions{Name: handle.Path, Flags: flag, Mode: fileModeValue}) + handle, err := fc.DownloadFile(internal.DownloadFileOptions{Name: handle.Path, Flags: flag, Mode: fileModeValue, Handle: handle}) if err != nil { log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) @@ -864,21 +864,20 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // Increment the handle count in this lock item as there is one handle open for this now flock.Inc() - handle := handlemap.NewHandle(options.Name) inf, err := f.Stat() if err == nil { - handle.Size = inf.Size() + options.Handle.Size = inf.Size() } - handle.UnixFD = uint64(f.Fd()) + options.Handle.UnixFD = uint64(f.Fd()) if !fc.offloadIO { - handle.Flags.Set(handlemap.HandleFlagCached) + options.Handle.Flags.Set(handlemap.HandleFlagCached) } log.Info("FileCache::download : file=%s, fd=%d", options.Name, f.Fd()) - handle.SetFileObject(f) + options.Handle.SetFileObject(f) - return handle, nil + return options.Handle, nil } // OpenFile: Makes the file available in the local cache for further file operations. @@ -1025,17 +1024,8 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) - var newHandle *handlemap.Handle - var swapped bool if _, ok := options.Handle.GetValue("flag"); ok { - latestFlag := options.Handle.Flags - newHandle = fc.getHandleData(options.Handle) - newHandle.Flags = latestFlag - swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) - options.Handle = newHandle - if !swapped { - log.Err("FileCache::ReadInBuffer : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) - } + fc.getHandleData(options.Handle) } f := options.Handle.GetFileObject() @@ -1068,18 +1058,8 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) - var newHandle *handlemap.Handle - var swapped bool if _, ok := options.Handle.GetValue("flag"); ok { - latestFlag := options.Handle.Flags - newHandle = fc.getHandleData(options.Handle) - newHandle.Flags = latestFlag - swapped = handlemap.GetHandles().CompareAndSwap(options.Handle.ID, options.Handle, newHandle) - options.Handle = newHandle - if !swapped { - log.Err("FileCache::WriteFile : error [couldn't swap updated handle into handlemap] %s", options.Handle.Path) - } - + fc.getHandleData(options.Handle) } f := options.Handle.GetFileObject() @@ -1407,9 +1387,9 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { if err != nil && err == syscall.EACCES { return err } - + h = handlemap.NewHandle(options.Name) if downloadRequired { - h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) + _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission, Handle: h}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index ee912032a..5a52286c4 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -769,7 +769,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { } // Download is required - handle, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Flags: 0, Mode: os.FileMode(0777)}) + _, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Flags: 0, Mode: os.FileMode(0777), Handle: handle}) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1598,15 +1598,17 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { err = os.WriteFile(suite.fake_storage_path+"/"+pathsmall, data, 0777) suite.assert.NoError(err) + smallHandle := handlemap.NewHandle(pathsmall) // try opening small file - f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Flags: 0, Mode: os.FileMode(0777)}) + f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Flags: 0, Mode: os.FileMode(0777), Handle: smallHandle}) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) + bigHandle := handlemap.NewHandle(pathbig) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Flags: 0, Mode: os.FileMode(0777)}) + f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Flags: 0, Mode: os.FileMode(0777), Handle: bigHandle}) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) @@ -1642,6 +1644,36 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { suite.assert.Error(err) } +func (suite *fileCacheTestSuite) TestHandleDataChange() { + defer suite.cleanupTest() + // Configure to create empty files so we create the file in cloud storage + createEmptyFile := true + config := fmt.Sprintf("file_cache:\n path: %s\n offload-io: true\n create-empty-file: %t\n timeout-sec: 1000\n refresh-sec: 10\n\nloopbackfs:\n path: %s", + suite.cache_path, createEmptyFile, suite.fake_storage_path) + suite.setupTestHelper(config) // setup a new file cache with a custom config (teardown will occur after the test as usual) + + path := "file43" + err := os.WriteFile(suite.fake_storage_path+"/"+path, []byte("test data"), 0777) + suite.assert.NoError(err) + + data := make([]byte, 20) + options := internal.OpenFileOptions{Name: path, Flags: os.O_RDONLY, Mode: 0777} + + // Read file once and we shall get the same data + f, err := suite.fileCache.OpenFile(options) + handlemap.Add(f) + suite.assert.NoError(err) + suite.assert.False(f.Dirty()) + n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) + f, loaded := handlemap.Load(f.ID) + suite.assert.True(loaded) + suite.assert.NoError(err) + suite.assert.Equal(9, n) + err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) + suite.assert.NoError(err) + +} + // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run func TestFileCacheTestSuite(t *testing.T) { diff --git a/internal/component_options.go b/internal/component_options.go index 01f4537da..fb6e8ead0 100644 --- a/internal/component_options.go +++ b/internal/component_options.go @@ -78,9 +78,10 @@ type DeleteFileOptions struct { } type DownloadFileOptions struct { - Name string - Flags int - Mode os.FileMode + Name string + Flags int + Mode os.FileMode + Handle *handlemap.Handle } type OpenFileOptions struct { From d66e3d946d7a54752e9036a130f15ae2a897b89e Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 30 May 2024 15:50:25 -0600 Subject: [PATCH 29/85] used sync.RWMutex from handle to lock and unlock handle instead of file cache. --- component/file_cache/file_cache.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 4bfab8ab7..dcd48b18e 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -73,7 +73,6 @@ type FileCache struct { refreshSec uint32 hardLimit bool diskHighWaterMark float64 - mu sync.Mutex } // Structure defining your config parameters @@ -696,7 +695,6 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) - fc.mu.Lock() //exctract the flags out of handle values flags, _ := handle.GetValue("flag") flagsStruct, ok := flags.(struct{ flags int }) @@ -726,8 +724,6 @@ func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { handle.RemoveValue("flag") handle.RemoveValue("mode") - fc.mu.Unlock() - return handle } @@ -1024,9 +1020,11 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) + options.Handle.Lock() if _, ok := options.Handle.GetValue("flag"); ok { fc.getHandleData(options.Handle) } + options.Handle.Unlock() f := options.Handle.GetFileObject() if f == nil { @@ -1058,9 +1056,11 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) + options.Handle.Lock() if _, ok := options.Handle.GetValue("flag"); ok { fc.getHandleData(options.Handle) } + options.Handle.Unlock() f := options.Handle.GetFileObject() if f == nil { From 9828186038195c018c897ad2b132c0423385fcfd Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 14:07:51 -0600 Subject: [PATCH 30/85] -moved getHandleData() logic into DownloadFile() -removed DownloadFile() flag and mode parameters --- component/file_cache/file_cache.go | 59 +++++++++++++----------------- internal/component_options.go | 2 - 2 files changed, 25 insertions(+), 36 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index dcd48b18e..9fddd2c65 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -691,51 +691,38 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { return nil } -func (fc *FileCache) getHandleData(handle *handlemap.Handle) *handlemap.Handle { +func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { - log.Debug("FileCache::getHandleData : Need to download %s", handle.Path) + //TODO: use error output properly for flags and mode extraction. + var err error - //exctract the flags out of handle values - flags, _ := handle.GetValue("flag") + //extract flag out of the values from handle + flags, _ := options.Handle.GetValue("flag") flagsStruct, ok := flags.(struct{ flags int }) if !ok { - log.Debug("FileCache::getHandleData : error Type assertion failed on getting flag for %s [%s]", handle.Path) - return nil + log.Err("FileCache::getHandleData : error Type assertion failed on getting flag for %s [%s]", options.Handle.Path) + return nil, err } - - // Extract the integer as a separate variable flag := flagsStruct.flags // extract the filemode out of handle values - mode, _ := handle.GetValue("mode") - fileModeValue, ok := mode.(os.FileMode) + mode, _ := options.Handle.GetValue("mode") + fMode, ok := mode.(os.FileMode) if !ok { - log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", handle.Path) - return nil - } - - handle, err := fc.DownloadFile(internal.DownloadFileOptions{Name: handle.Path, Flags: flag, Mode: fileModeValue, Handle: handle}) - - if err != nil { - log.Err("FileCache::getHandleData : error downloading data for file %s [%s]", handle.Path, err.Error()) - return handle + log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) + return nil, err } - handle.RemoveValue("flag") - handle.RemoveValue("mode") - - return handle -} - -func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { - log.Trace("FileCache::download : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) + log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) var f *os.File - var err error flock := fc.fileLocks.Get(options.Name) flock.Lock() + + // TODO: check if file needs downloading again. use the attr from isDownloadRequired() + defer flock.Unlock() fc.policy.CacheValid(localPath) @@ -780,13 +767,13 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle } // Open the file in write mode. - f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, options.Mode) + f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) if err != nil { log.Err("FileCache::download : error creating new file %s [%s]", options.Name, err.Error()) return nil, err } - if options.Flags&os.O_TRUNC != 0 { + if flag&os.O_TRUNC != 0 { fileSize = 0 } @@ -851,7 +838,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) // Open the file and grab a shared lock to prevent deletion by the cache policy. - f, err = common.OpenFile(localPath, options.Flags, options.Mode) + f, err = common.OpenFile(localPath, flag, fMode) if err != nil { log.Err("FileCache::download : error opening cached file %s [%s]", options.Name, err.Error()) return nil, err @@ -873,6 +860,10 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle log.Info("FileCache::download : file=%s, fd=%d", options.Name, f.Fd()) options.Handle.SetFileObject(f) + //remove values as a kind of signal that the file has been downloaded + options.Handle.RemoveValue("flag") + options.Handle.RemoveValue("mode") + return options.Handle, nil } @@ -1022,7 +1013,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() if _, ok := options.Handle.GetValue("flag"); ok { - fc.getHandleData(options.Handle) + fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) } options.Handle.Unlock() @@ -1058,7 +1049,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() if _, ok := options.Handle.GetValue("flag"); ok { - fc.getHandleData(options.Handle) + fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) } options.Handle.Unlock() @@ -1389,7 +1380,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } h = handlemap.NewHandle(options.Name) if downloadRequired { - _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission, Handle: h}) + _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err diff --git a/internal/component_options.go b/internal/component_options.go index fb6e8ead0..b430cc5f1 100644 --- a/internal/component_options.go +++ b/internal/component_options.go @@ -79,8 +79,6 @@ type DeleteFileOptions struct { type DownloadFileOptions struct { Name string - Flags int - Mode os.FileMode Handle *handlemap.Handle } From d9876b6a01049283b1e210cffc282b88a9259887 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 14:37:44 -0600 Subject: [PATCH 31/85] -moved download logic into a downloadRequired conditional -removed syscall.EACCES error check --- component/file_cache/file_cache.go | 147 ++++++++++++++--------------- 1 file changed, 71 insertions(+), 76 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 9fddd2c65..a98f2334c 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -720,104 +720,99 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle flock := fc.fileLocks.Get(options.Name) flock.Lock() - - // TODO: check if file needs downloading again. use the attr from isDownloadRequired() - defer flock.Unlock() - fc.policy.CacheValid(localPath) - - attr, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name}) + downloadRequired, _, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) if err != nil { - log.Err("FileCache::download : Failed to get attr of %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", options.Name, err.Error()) } - // return err in case of authorization permission mismatch - if err != nil && err == syscall.EACCES { - return nil, err - } + fc.policy.CacheValid(localPath) - log.Debug("FileCache::download : Need to download %s", options.Name) + // redundantly check if file is needs downloading + fileMode := fc.defaultPermission + if downloadRequired { + log.Debug("FileCache::download : Need to download %s", options.Name) - fileSize := int64(0) - if attr != nil { - fileSize = int64(attr.Size) - } - _, err = os.Stat(localPath) - var fileExists bool - if err == nil { - // The file exists in local cache - fileExists = true - } + fileSize := int64(0) + if attr != nil { + fileSize = int64(attr.Size) + } + _, err = os.Stat(localPath) + var fileExists bool + if err == nil { + // The file exists in local cache + fileExists = true + } - if fileExists { - log.Debug("FileCache::download : Delete cached file %s", options.Name) + if fileExists { + log.Debug("FileCache::download : Delete cached file %s", options.Name) - err := deleteFile(localPath) - if err != nil && !os.IsNotExist(err) { - log.Err("FileCache::download : Failed to delete old file %s", options.Name) + err := deleteFile(localPath) + if err != nil && !os.IsNotExist(err) { + log.Err("FileCache::download : Failed to delete old file %s", options.Name) + } + } else { + // Create the file if if doesn't already exist. + err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) + if err != nil { + log.Err("FileCache::download : error creating directory structure for file %s [%s]", options.Name, err.Error()) + return nil, err + } } - } else { - // Create the file if if doesn't already exist. - err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) + + // Open the file in write mode. + f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) if err != nil { - log.Err("FileCache::download : error creating directory structure for file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : error creating new file %s [%s]", options.Name, err.Error()) return nil, err } - } - // Open the file in write mode. - f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) - if err != nil { - log.Err("FileCache::download : error creating new file %s [%s]", options.Name, err.Error()) - return nil, err - } + if flag&os.O_TRUNC != 0 { + fileSize = 0 + } - if flag&os.O_TRUNC != 0 { - fileSize = 0 - } + if fileSize > 0 { + if fc.diskHighWaterMark != 0 { + currSize, err := common.GetUsage(fc.tmpPath) + if err != nil { + log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) + } else { + if (currSize + float64(fileSize)) > fc.diskHighWaterMark { + log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) + return nil, syscall.ENOSPC + } + } - if fileSize > 0 { - if fc.diskHighWaterMark != 0 { - currSize, err := common.GetUsage(fc.tmpPath) + } + // Download/Copy the file from storage to the local file. + // We pass a count of 0 to get the entire object + err = fc.NextComponent().CopyToFile( + internal.CopyToFileOptions{ + Name: options.Name, + Offset: 0, + Count: 0, + File: f, + }) if err != nil { - log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) - } else { - if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) - return nil, syscall.ENOSPC - } + // File was created locally and now download has failed so we need to delete it back from local cache + log.Err("FileCache::download : error downloading file from storage %s [%s]", options.Name, err.Error()) + _ = f.Close() + _ = os.Remove(localPath) + return nil, err } - } - // Download/Copy the file from storage to the local file. - // We pass a count of 0 to get the entire object - err = fc.NextComponent().CopyToFile( - internal.CopyToFileOptions{ - Name: options.Name, - Offset: 0, - Count: 0, - File: f, - }) - if err != nil { - // File was created locally and now download has failed so we need to delete it back from local cache - log.Err("FileCache::download : error downloading file from storage %s [%s]", options.Name, err.Error()) - _ = f.Close() - _ = os.Remove(localPath) - return nil, err - } - } - // Update the last download time of this file - flock.SetDownloadTime() + // Update the last download time of this file + flock.SetDownloadTime() - log.Debug("FileCache::download : Download of %s is complete", options.Name) - f.Close() + log.Debug("FileCache::download : Download of %s is complete", options.Name) + f.Close() - // After downloading the file, update the modified times and mode of the file. - fileMode := fc.defaultPermission - if attr != nil && !attr.IsModeDefault() { - fileMode = attr.Mode + // After downloading the file, update the modified times and mode of the file. + if attr != nil && !attr.IsModeDefault() { + fileMode = attr.Mode + } } // If user has selected some non default mode in config then every local file shall be created with that mode only From e35e2cd8a78322dc78aae769785108393c1b0a46 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 14:44:17 -0600 Subject: [PATCH 32/85] -used OpenFile() instead of creating a new handle direclty -changed conditional to check if handle has value "flag" signifying that it is new and empty. --- component/file_cache/file_cache.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index a98f2334c..b153f64ab 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1353,7 +1353,6 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } } - var h *handlemap.Handle = nil var err error = nil if options.Size == 0 { @@ -1366,15 +1365,14 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } else { // If size is not 0 then we need to open the file and then truncate it // Open will force download if file was not present in local system - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) - flock := fc.fileLocks.Get(options.Name) - downloadRequired, _, _, err := fc.isDownloadRequired(localPath, options.Name, flock) + // return err in case of authorization permission mismatch if err != nil && err == syscall.EACCES { return err } - h = handlemap.NewHandle(options.Name) - if downloadRequired { + + h, err := fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: 0, Mode: fc.defaultPermission}) + if _, loaded := h.GetValue("flag"); loaded { _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) From b68acddeead8e73c8f641db009e24d06fafc19e9 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 14:51:18 -0600 Subject: [PATCH 33/85] added error handle to OpenFile() call in TruncateFile() --- component/file_cache/file_cache.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index b153f64ab..11d96bb4e 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -693,15 +693,12 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { - //TODO: use error output properly for flags and mode extraction. - var err error - //extract flag out of the values from handle flags, _ := options.Handle.GetValue("flag") flagsStruct, ok := flags.(struct{ flags int }) if !ok { log.Err("FileCache::getHandleData : error Type assertion failed on getting flag for %s [%s]", options.Handle.Path) - return nil, err + return options.Handle, fmt.Errorf("Type assertion failed on getting flag forfor %s [%s]", options.Handle.Path) } flag := flagsStruct.flags @@ -710,7 +707,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle fMode, ok := mode.(os.FileMode) if !ok { log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) - return nil, err + return options.Handle, fmt.Errorf("Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) } log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) @@ -1355,6 +1352,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { var err error = nil + var h *handlemap.Handle if options.Size == 0 { // If size is 0 then no need to download any file we can just create an empty file h, err = fc.CreateFile(internal.CreateFileOptions{Name: options.Name, Mode: fc.defaultPermission}) @@ -1371,7 +1369,10 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { return err } - h, err := fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: 0, Mode: fc.defaultPermission}) + h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: 0, Mode: fc.defaultPermission}) + if err != nil { + log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) + } if _, loaded := h.GetValue("flag"); loaded { _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) if err != nil { From fcf037302c2f12f895ae61dd1b7496bbdd9cdbdf Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 14:52:15 -0600 Subject: [PATCH 34/85] fixed parameters calling DownloadFile() --- component/file_cache/file_cache_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 5a52286c4..498b97ba5 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -769,7 +769,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { } // Download is required - _, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Flags: 0, Mode: os.FileMode(0777), Handle: handle}) + _, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Handle: handle}) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1600,7 +1600,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { smallHandle := handlemap.NewHandle(pathsmall) // try opening small file - f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Flags: 0, Mode: os.FileMode(0777), Handle: smallHandle}) + f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Handle: smallHandle}) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) @@ -1608,7 +1608,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { bigHandle := handlemap.NewHandle(pathbig) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Flags: 0, Mode: os.FileMode(0777), Handle: bigHandle}) + f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Handle: bigHandle}) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) From 83e8fcc5ebfa3d6f87125dbae5253b58f5e8baf4 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 3 Jun 2024 17:15:36 -0600 Subject: [PATCH 35/85] -set default values for flag and mode for DownloadFile() -removed handlemap loading from test --- component/file_cache/file_cache.go | 40 ++++++++++++++----------- component/file_cache/file_cache_test.go | 8 +---- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 11d96bb4e..2f35a02af 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -693,21 +693,28 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { + flag := 0 //extract flag out of the values from handle - flags, _ := options.Handle.GetValue("flag") - flagsStruct, ok := flags.(struct{ flags int }) - if !ok { - log.Err("FileCache::getHandleData : error Type assertion failed on getting flag for %s [%s]", options.Handle.Path) - return options.Handle, fmt.Errorf("Type assertion failed on getting flag forfor %s [%s]", options.Handle.Path) + flags, found := options.Handle.GetValue("flag") + if found { + flagsStruct, ok := flags.(struct{ flags int }) + if !ok { + log.Err("FileCache::getHandleData : error Type assertion failed on getting flag for %s", options.Handle.Path) + return options.Handle, fmt.Errorf("type assertion failed on getting flag forfor %s", options.Handle.Path) + } + flag = flagsStruct.flags } - flag := flagsStruct.flags // extract the filemode out of handle values - mode, _ := options.Handle.GetValue("mode") - fMode, ok := mode.(os.FileMode) - if !ok { - log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) - return options.Handle, fmt.Errorf("Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) + fMode := fc.defaultPermission + var ok bool + mode, found := options.Handle.GetValue("mode") + if found { + fMode, ok = mode.(os.FileMode) + if !ok { + log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) + return options.Handle, fmt.Errorf("type assertion failed on getting file mode for %s", options.Handle.Path) + } } log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) @@ -1004,8 +1011,12 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) options.Handle.Lock() + var err error if _, ok := options.Handle.GetValue("flag"); ok { - fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, error.Error(err)) + } } options.Handle.Unlock() @@ -1364,11 +1375,6 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { // If size is not 0 then we need to open the file and then truncate it // Open will force download if file was not present in local system - // return err in case of authorization permission mismatch - if err != nil && err == syscall.EACCES { - return err - } - h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: 0, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 498b97ba5..0a2a36562 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1537,16 +1537,13 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) data := make([]byte, 20) - options := internal.OpenFileOptions{Name: path, Flags: os.O_RDONLY, Mode: 0777} + options := internal.OpenFileOptions{Name: path, Mode: 0777} // Read file once and we shall get the same data f, err := suite.fileCache.OpenFile(options) - handlemap.Add(f) suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) - f, loaded := handlemap.Load(f.ID) - suite.assert.True(loaded) suite.assert.NoError(err) suite.assert.Equal(9, n) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) @@ -1569,12 +1566,9 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) time.Sleep(12 * time.Second) f, err = suite.fileCache.OpenFile(options) - handlemap.Add(f) suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) - f, loaded = handlemap.Load(f.ID) - suite.assert.True(loaded) suite.assert.NoError(err) suite.assert.Equal(15, n) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) From bea8da5bac116e6ba6d9d064156cdc9712d2825f Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 11:37:09 -0600 Subject: [PATCH 36/85] added another 12 second wait for file to expire in TestReadFileWithRefresh() --- component/file_cache/file_cache_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 0a2a36562..47f2ff0ba 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1566,6 +1566,7 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) time.Sleep(12 * time.Second) f, err = suite.fileCache.OpenFile(options) + time.Sleep(12 * time.Second) //OpenFile() reset the timer for expiration. suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) From a4584201f1a7c366363f6de3221dc8542cf357df Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 12:29:37 -0600 Subject: [PATCH 37/85] put os.O_RDWR back into OpenFile() call in TruncateFile() to allow FlushFile()'s f.Sync() call to succeed --- component/file_cache/file_cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 2f35a02af..18d030c47 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1375,12 +1375,12 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { // If size is not 0 then we need to open the file and then truncate it // Open will force download if file was not present in local system - h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: 0, Mode: fc.defaultPermission}) + h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } if _, loaded := h.GetValue("flag"); loaded { - _, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) + h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err From fb079736b8ef6975c96b3fdbb82dcb5e6c14148e Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 12:43:54 -0600 Subject: [PATCH 38/85] moved fileExists assigned from isDownloadRequired() in DownlaodFile() --- component/file_cache/file_cache.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 18d030c47..045455d3f 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -726,7 +726,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle flock.Lock() defer flock.Unlock() - downloadRequired, _, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) + downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) if err != nil { log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", options.Name, err.Error()) } @@ -742,12 +742,6 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle if attr != nil { fileSize = int64(attr.Size) } - _, err = os.Stat(localPath) - var fileExists bool - if err == nil { - // The file exists in local cache - fileExists = true - } if fileExists { log.Debug("FileCache::download : Delete cached file %s", options.Name) From 235e6fa3b182a7c7412ffd41cd39062554326024 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 13:58:15 -0600 Subject: [PATCH 39/85] added error handle in WriteFile() --- component/file_cache/file_cache.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 045455d3f..a5ef93732 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1046,7 +1046,10 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() if _, ok := options.Handle.GetValue("flag"); ok { - fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("Error occured during download for file %s [%s]", options.Handle.Path, err) + } } options.Handle.Unlock() From 3e9238ecbe2527c2df1d8a9c036e91348631ac11 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 15:11:59 -0600 Subject: [PATCH 40/85] spell correction on error string --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index a5ef93732..470a2120c 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1048,7 +1048,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { if _, ok := options.Handle.GetValue("flag"); ok { _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) if err != nil { - return 0, fmt.Errorf("Error occured during download for file %s [%s]", options.Handle.Path, err) + return 0, fmt.Errorf("error occurred during download for file %s [%s]", options.Handle.Path, err) } } options.Handle.Unlock() From 80f21dabf2bdb7b047c25c2fe8d450724ca6b9fb Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 4 Jun 2024 16:08:24 -0600 Subject: [PATCH 41/85] Added isDownloadNeeded value in handle and adjusted download conditionals respectively --- component/file_cache/file_cache.go | 60 +++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 470a2120c..9fa943c63 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -699,7 +699,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle if found { flagsStruct, ok := flags.(struct{ flags int }) if !ok { - log.Err("FileCache::getHandleData : error Type assertion failed on getting flag for %s", options.Handle.Path) + log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) return options.Handle, fmt.Errorf("type assertion failed on getting flag forfor %s", options.Handle.Path) } flag = flagsStruct.flags @@ -712,11 +712,28 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle if found { fMode, ok = mode.(os.FileMode) if !ok { - log.Debug("FileCache::getHandleData : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) + log.Debug("FileCache::DownloadFile : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) return options.Handle, fmt.Errorf("type assertion failed on getting file mode for %s", options.Handle.Path) } } + // extract the download bool out of handle values + var downloadNeeded bool + dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded") + if found { + downloadNeedStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) + if !ok { + log.Debug("FileCache::DownloadFile : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) + return options.Handle, fmt.Errorf("type assertion failed on getting file mode for %s", options.Handle.Path) + } + downloadNeeded = downloadNeedStruct.downloadNeeded + + if !downloadNeeded { + log.Debug("FileCache::DownloadFile : isDownloadNeeded value in handle contains false for %s", options.Handle.Path) + return options.Handle, nil + } + } + log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) @@ -853,9 +870,9 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle log.Info("FileCache::download : file=%s, fd=%d", options.Name, f.Fd()) options.Handle.SetFileObject(f) - //remove values as a kind of signal that the file has been downloaded - options.Handle.RemoveValue("flag") - options.Handle.RemoveValue("mode") + //set boolean in isDownloadNeeded value to signal that the file has been downloaded + noDownloadNeeded := struct{ downloadNeeded bool }{downloadNeeded: false} + options.Handle.SetValue("isDownloadNeeded", noDownloadNeeded) return options.Handle, nil } @@ -885,6 +902,7 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand handle := handlemap.NewHandle(options.Name) handle.SetValue("flag", struct{ flags int }{flags: options.Flags}) handle.SetValue("mode", options.Mode) + handle.SetValue("isDownloadNeeded", struct{ downloadNeeded bool }{downloadNeeded: downloadRequired}) return handle, nil @@ -1006,10 +1024,18 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - if _, ok := options.Handle.GetValue("flag"); ok { - options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) - if err != nil { - return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, error.Error(err)) + if dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded"); found { + downloadNeededStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) + if !ok { + log.Err("FileCache::WriteFile : error type assertion failed on checking if download is needed for %s", options.Handle.Path) + return 0, fmt.Errorf("type assertion failed on checking if download is needed for %s", options.Handle.Path) + } + downloadNeeded := downloadNeededStruct.downloadNeeded + if downloadNeeded { + _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) + } } } options.Handle.Unlock() @@ -1045,10 +1071,18 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) options.Handle.Lock() - if _, ok := options.Handle.GetValue("flag"); ok { - _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) - if err != nil { - return 0, fmt.Errorf("error occurred during download for file %s [%s]", options.Handle.Path, err) + if dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded"); found { + downloadNeededStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) + if !ok { + log.Err("FileCache::WriteFile : error type assertion failed on checking if download is needed for %s", options.Handle.Path) + return 0, fmt.Errorf("type assertion failed on checking if download is needed for %s", options.Handle.Path) + } + downloadNeeded := downloadNeededStruct.downloadNeeded + if downloadNeeded { + _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) + } } } options.Handle.Unlock() From d7c1b16070f00e0a10caf8806042cc50090fd738 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 5 Jun 2024 13:47:07 -0600 Subject: [PATCH 42/85] removed redundant code from OpenFile(). --- component/file_cache/file_cache.go | 63 +++++-------------------- component/file_cache/file_cache_test.go | 13 +++-- 2 files changed, 17 insertions(+), 59 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 9fa943c63..bd38f2069 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -743,13 +743,12 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle flock.Lock() defer flock.Unlock() + fc.policy.CacheValid(localPath) downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) if err != nil { log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", options.Name, err.Error()) } - fc.policy.CacheValid(localPath) - // redundantly check if file is needs downloading fileMode := fc.defaultPermission if downloadRequired { @@ -833,7 +832,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // If user has selected some non default mode in config then every local file shall be created with that mode only err = os.Chmod(localPath, fileMode) if err != nil { - log.Err("FileCache::download : Failed to change mode of file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to change mode of file %s [%s]", options.Name, err.Error()) } // TODO: When chown is supported should we update that? @@ -841,7 +840,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // chtimes shall be the last api otherwise calling chmod/chown will update the last change time err = os.Chtimes(localPath, attr.Atime, attr.Mtime) if err != nil { - log.Err("FileCache::download : Failed to change times of file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to change times of file %s [%s]", options.Name, err.Error()) } } @@ -850,7 +849,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // Open the file and grab a shared lock to prevent deletion by the cache policy. f, err = common.OpenFile(localPath, flag, fMode) if err != nil { - log.Err("FileCache::download : error opening cached file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : error opening cached file %s [%s]", options.Name, err.Error()) return nil, err } @@ -867,7 +866,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle options.Handle.Flags.Set(handlemap.HandleFlagCached) } - log.Info("FileCache::download : file=%s, fd=%d", options.Name, f.Fd()) + log.Info("FileCache::DownloadFile : file=%s, fd=%d", options.Name, f.Fd()) options.Handle.SetFileObject(f) //set boolean in isDownloadNeeded value to signal that the file has been downloaded @@ -881,61 +880,21 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Handle, error) { log.Trace("FileCache::OpenFile : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) - var f *os.File - var err error - - flock := fc.fileLocks.Get(options.Name) - flock.Lock() - defer flock.Unlock() - - fc.policy.CacheValid(localPath) - downloadRequired, _, _, err := fc.isDownloadRequired(localPath, options.Name, flock) + _, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name}) // return err in case of authorization permission mismatch if err != nil && err == syscall.EACCES { return nil, err } - if downloadRequired { - fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) - handle := handlemap.NewHandle(options.Name) - handle.SetValue("flag", struct{ flags int }{flags: options.Flags}) - handle.SetValue("mode", options.Mode) - handle.SetValue("isDownloadNeeded", struct{ downloadNeeded bool }{downloadNeeded: downloadRequired}) - - return handle, nil - - } else { - log.Debug("FileCache::OpenFile : %s will be served from cache", options.Name) - fileCacheStatsCollector.UpdateStats(stats_manager.Increment, cacheServed, (int64)(1)) - } - - // Open the file and grab a shared lock to prevent deletion by the cache policy. - f, err = common.OpenFile(localPath, options.Flags, options.Mode) - if err != nil { - log.Err("FileCache::OpenFile : error opening cached file %s [%s]", options.Name, err.Error()) - return nil, err - } - - // Increment the handle count in this lock item as there is one handle open for this now - flock.Inc() - + fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) handle := handlemap.NewHandle(options.Name) - inf, err := f.Stat() - if err == nil { - handle.Size = inf.Size() - } - - handle.UnixFD = uint64(f.Fd()) - if !fc.offloadIO { - handle.Flags.Set(handlemap.HandleFlagCached) - } - - log.Info("FileCache::OpenFile : file=%s, fd=%d", options.Name, f.Fd()) - handle.SetFileObject(f) + handle.SetValue("flag", struct{ flags int }{flags: options.Flags}) + handle.SetValue("mode", options.Mode) + handle.SetValue("isDownloadNeeded", struct{ downloadNeeded bool }{downloadNeeded: true}) return handle, nil + } // CloseFile: Flush the file and invalidate it from the cache. diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 47f2ff0ba..f3162e630 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -895,9 +895,9 @@ func (suite *fileCacheTestSuite) TestReadFile() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) + d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) suite.assert.NoError(err) - suite.assert.EqualValues(data, d) + suite.assert.EqualValues(9, d) } func (suite *fileCacheTestSuite) TestReadFileNoFlush() { @@ -911,9 +911,9 @@ func (suite *fileCacheTestSuite) TestReadFileNoFlush() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) + d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) suite.assert.NoError(err) - suite.assert.EqualValues(data, d) + suite.assert.EqualValues(9, d) } func (suite *fileCacheTestSuite) TestReadFileErrorBadFd() { @@ -1477,9 +1477,9 @@ func (suite *fileCacheTestSuite) TestCachePathSymlink() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) + d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) suite.assert.NoError(err) - suite.assert.EqualValues(data, d) + suite.assert.EqualValues(9, d) } func (suite *fileCacheTestSuite) TestZZOffloadIO() { @@ -1566,7 +1566,6 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.assert.NoError(err) time.Sleep(12 * time.Second) f, err = suite.fileCache.OpenFile(options) - time.Sleep(12 * time.Second) //OpenFile() reset the timer for expiration. suite.assert.NoError(err) suite.assert.False(f.Dirty()) n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) From a35a18bb64194c2ccf48335b05eb9717e915d442 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 5 Jun 2024 16:07:37 -0600 Subject: [PATCH 43/85] condensed values in handle into a single value. changed conditions checking for this one value. --- component/file_cache/file_cache.go | 106 +++++++----------------- component/file_cache/file_cache_test.go | 8 +- 2 files changed, 36 insertions(+), 78 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index bd38f2069..a2b739cac 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -693,48 +693,27 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { - flag := 0 - //extract flag out of the values from handle - flags, found := options.Handle.GetValue("flag") + //extract flag and mode out of the value from handle + var flag int + var fMode fs.FileMode + + flagMode, found := options.Handle.GetValue("fileFlagMode") if found { - flagsStruct, ok := flags.(struct{ flags int }) + flagModeStruct, ok := flagMode.(struct { + flag int + fMode fs.FileMode + }) if !ok { log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) return options.Handle, fmt.Errorf("type assertion failed on getting flag forfor %s", options.Handle.Path) } - flag = flagsStruct.flags - } - - // extract the filemode out of handle values - fMode := fc.defaultPermission - var ok bool - mode, found := options.Handle.GetValue("mode") - if found { - fMode, ok = mode.(os.FileMode) - if !ok { - log.Debug("FileCache::DownloadFile : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) - return options.Handle, fmt.Errorf("type assertion failed on getting file mode for %s", options.Handle.Path) - } - } - - // extract the download bool out of handle values - var downloadNeeded bool - dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded") - if found { - downloadNeedStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) - if !ok { - log.Debug("FileCache::DownloadFile : error Type assertion failed on getting file mode for %s [%s]", options.Handle.Path) - return options.Handle, fmt.Errorf("type assertion failed on getting file mode for %s", options.Handle.Path) - } - downloadNeeded = downloadNeedStruct.downloadNeeded - - if !downloadNeeded { - log.Debug("FileCache::DownloadFile : isDownloadNeeded value in handle contains false for %s", options.Handle.Path) - return options.Handle, nil - } + flag = flagModeStruct.flag + fMode = flagModeStruct.fMode + } else { + return options.Handle, nil } - log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) + log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flag, fMode) localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) var f *os.File @@ -889,9 +868,10 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) handle := handlemap.NewHandle(options.Name) - handle.SetValue("flag", struct{ flags int }{flags: options.Flags}) - handle.SetValue("mode", options.Mode) - handle.SetValue("isDownloadNeeded", struct{ downloadNeeded bool }{downloadNeeded: true}) + handle.SetValue("fileFlagMode", struct { + flag int + fMode fs.FileMode + }{flag: options.Flags, fMode: options.Mode}) return handle, nil @@ -983,19 +963,9 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - if dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded"); found { - downloadNeededStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) - if !ok { - log.Err("FileCache::WriteFile : error type assertion failed on checking if download is needed for %s", options.Handle.Path) - return 0, fmt.Errorf("type assertion failed on checking if download is needed for %s", options.Handle.Path) - } - downloadNeeded := downloadNeededStruct.downloadNeeded - if downloadNeeded { - _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) - if err != nil { - return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) - } - } + options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } options.Handle.Unlock() @@ -1030,19 +1000,10 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) options.Handle.Lock() - if dwnldIntfce, found := options.Handle.GetValue("isDownloadNeeded"); found { - downloadNeededStruct, ok := dwnldIntfce.(struct{ downloadNeeded bool }) - if !ok { - log.Err("FileCache::WriteFile : error type assertion failed on checking if download is needed for %s", options.Handle.Path) - return 0, fmt.Errorf("type assertion failed on checking if download is needed for %s", options.Handle.Path) - } - downloadNeeded := downloadNeededStruct.downloadNeeded - if downloadNeeded { - _, err := fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) - if err != nil { - return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) - } - } + var err error + options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + if err != nil { + return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } options.Handle.Unlock() @@ -1369,18 +1330,11 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } - if _, loaded := h.GetValue("flag"); loaded { - h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) - if err != nil { - log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) - return err - } - } else { - h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) - if err != nil { - log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) - return err - } + + h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) + if err != nil { + log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) + return err } } diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index f3162e630..5ab87c8fa 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -768,6 +768,8 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { suite.assert.True(os.IsNotExist(err)) } + handle, err = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: suite.fileCache.defaultPermission}) + suite.assert.NoError(err) // Download is required _, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Handle: handle}) suite.assert.NoError(err) @@ -1592,7 +1594,8 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { err = os.WriteFile(suite.fake_storage_path+"/"+pathsmall, data, 0777) suite.assert.NoError(err) - smallHandle := handlemap.NewHandle(pathsmall) + smallHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathsmall, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) + suite.assert.NoError(err) // try opening small file f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Handle: smallHandle}) suite.assert.NoError(err) @@ -1600,7 +1603,8 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) - bigHandle := handlemap.NewHandle(pathbig) + bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) + suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Handle: bigHandle}) suite.assert.Error(err) From adaf9047b449eaf910664527ea4d442114568662 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 10:52:06 -0600 Subject: [PATCH 44/85] fix typo in DownlaodFile() log --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index a2b739cac..dd27f8cea 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -705,7 +705,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle }) if !ok { log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) - return options.Handle, fmt.Errorf("type assertion failed on getting flag forfor %s", options.Handle.Path) + return options.Handle, fmt.Errorf("type assertion failed on getting flag for %s", options.Handle.Path) } flag = flagModeStruct.flag fMode = flagModeStruct.fMode From cc4f0f7416f603cc057598352079edb7e07077ca Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 10:54:24 -0600 Subject: [PATCH 45/85] renamed rename flagModeStruct to openFileOptions in DownloadFile() --- component/file_cache/file_cache.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index dd27f8cea..cd1ddc61f 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -699,7 +699,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle flagMode, found := options.Handle.GetValue("fileFlagMode") if found { - flagModeStruct, ok := flagMode.(struct { + openFileOptions, ok := flagMode.(struct { flag int fMode fs.FileMode }) @@ -707,8 +707,8 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) return options.Handle, fmt.Errorf("type assertion failed on getting flag for %s", options.Handle.Path) } - flag = flagModeStruct.flag - fMode = flagModeStruct.fMode + flag = openFileOptions.flag + fMode = openFileOptions.fMode } else { return options.Handle, nil } From a966cdf3423dd92d1d5829c766b6ba28b664847a Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 10:59:53 -0600 Subject: [PATCH 46/85] renamed flag to flags in DownloadFile() --- component/file_cache/file_cache.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index cd1ddc61f..2761b6618 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -693,8 +693,8 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { - //extract flag and mode out of the value from handle - var flag int + //extract flags and mode out of the value from handle + var flags int var fMode fs.FileMode flagMode, found := options.Handle.GetValue("fileFlagMode") @@ -707,13 +707,13 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) return options.Handle, fmt.Errorf("type assertion failed on getting flag for %s", options.Handle.Path) } - flag = openFileOptions.flag + flags = openFileOptions.flag fMode = openFileOptions.fMode } else { return options.Handle, nil } - log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flag, fMode) + log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) var f *os.File @@ -761,7 +761,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle return nil, err } - if flag&os.O_TRUNC != 0 { + if flags&os.O_TRUNC != 0 { fileSize = 0 } @@ -826,7 +826,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) // Open the file and grab a shared lock to prevent deletion by the cache policy. - f, err = common.OpenFile(localPath, flag, fMode) + f, err = common.OpenFile(localPath, flags, fMode) if err != nil { log.Err("FileCache::DownloadFile : error opening cached file %s [%s]", options.Name, err.Error()) return nil, err From 00b832bc0a77a18d87ca9bdb2136ee16c7eeb484 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:04:34 -0600 Subject: [PATCH 47/85] removed DownloadFileOptions struct. refactored all function calls --- component/file_cache/file_cache.go | 64 +++++++++++++++--------------- internal/component_options.go | 5 --- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 2761b6618..6d036ec8e 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -691,47 +691,47 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { return nil } -func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handlemap.Handle, error) { +func (fc *FileCache) DownloadFile(handle *handlemap.Handle) (*handlemap.Handle, error) { //extract flags and mode out of the value from handle var flags int var fMode fs.FileMode - flagMode, found := options.Handle.GetValue("fileFlagMode") + flagMode, found := handle.GetValue("fileFlagMode") if found { openFileOptions, ok := flagMode.(struct { flag int fMode fs.FileMode }) if !ok { - log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", options.Handle.Path) - return options.Handle, fmt.Errorf("type assertion failed on getting flag for %s", options.Handle.Path) + log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", handle.Path) + return handle, fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) } flags = openFileOptions.flag fMode = openFileOptions.fMode } else { - return options.Handle, nil + return handle, nil } - log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", options.Name, flags, fMode) + log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", handle.Path, flags, fMode) - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Name) + localPath := common.JoinUnixFilepath(fc.tmpPath, handle.Path) var f *os.File - flock := fc.fileLocks.Get(options.Name) + flock := fc.fileLocks.Get(handle.Path) flock.Lock() defer flock.Unlock() fc.policy.CacheValid(localPath) - downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, options.Name, flock) + downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, handle.Path, flock) if err != nil { - log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", handle.Path, err.Error()) } // redundantly check if file is needs downloading fileMode := fc.defaultPermission if downloadRequired { - log.Debug("FileCache::download : Need to download %s", options.Name) + log.Debug("FileCache::download : Need to download %s", handle.Path) fileSize := int64(0) if attr != nil { @@ -739,17 +739,17 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle } if fileExists { - log.Debug("FileCache::download : Delete cached file %s", options.Name) + log.Debug("FileCache::download : Delete cached file %s", handle.Path) err := deleteFile(localPath) if err != nil && !os.IsNotExist(err) { - log.Err("FileCache::download : Failed to delete old file %s", options.Name) + log.Err("FileCache::download : Failed to delete old file %s", handle.Path) } } else { // Create the file if if doesn't already exist. err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) if err != nil { - log.Err("FileCache::download : error creating directory structure for file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : error creating directory structure for file %s [%s]", handle.Path, err.Error()) return nil, err } } @@ -757,7 +757,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // Open the file in write mode. f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) if err != nil { - log.Err("FileCache::download : error creating new file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : error creating new file %s [%s]", handle.Path, err.Error()) return nil, err } @@ -772,7 +772,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) } else { if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) + log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, handle.Path) return nil, syscall.ENOSPC } } @@ -782,14 +782,14 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // We pass a count of 0 to get the entire object err = fc.NextComponent().CopyToFile( internal.CopyToFileOptions{ - Name: options.Name, + Name: handle.Path, Offset: 0, Count: 0, File: f, }) if err != nil { // File was created locally and now download has failed so we need to delete it back from local cache - log.Err("FileCache::download : error downloading file from storage %s [%s]", options.Name, err.Error()) + log.Err("FileCache::download : error downloading file from storage %s [%s]", handle.Path, err.Error()) _ = f.Close() _ = os.Remove(localPath) return nil, err @@ -799,7 +799,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // Update the last download time of this file flock.SetDownloadTime() - log.Debug("FileCache::download : Download of %s is complete", options.Name) + log.Debug("FileCache::download : Download of %s is complete", handle.Path) f.Close() // After downloading the file, update the modified times and mode of the file. @@ -811,7 +811,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // If user has selected some non default mode in config then every local file shall be created with that mode only err = os.Chmod(localPath, fileMode) if err != nil { - log.Err("FileCache::DownloadFile : Failed to change mode of file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to change mode of file %s [%s]", handle.Path, err.Error()) } // TODO: When chown is supported should we update that? @@ -819,7 +819,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // chtimes shall be the last api otherwise calling chmod/chown will update the last change time err = os.Chtimes(localPath, attr.Atime, attr.Mtime) if err != nil { - log.Err("FileCache::DownloadFile : Failed to change times of file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : Failed to change times of file %s [%s]", handle.Path, err.Error()) } } @@ -828,7 +828,7 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle // Open the file and grab a shared lock to prevent deletion by the cache policy. f, err = common.OpenFile(localPath, flags, fMode) if err != nil { - log.Err("FileCache::DownloadFile : error opening cached file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::DownloadFile : error opening cached file %s [%s]", handle.Path, err.Error()) return nil, err } @@ -837,22 +837,22 @@ func (fc *FileCache) DownloadFile(options internal.DownloadFileOptions) (*handle inf, err := f.Stat() if err == nil { - options.Handle.Size = inf.Size() + handle.Size = inf.Size() } - options.Handle.UnixFD = uint64(f.Fd()) + handle.UnixFD = uint64(f.Fd()) if !fc.offloadIO { - options.Handle.Flags.Set(handlemap.HandleFlagCached) + handle.Flags.Set(handlemap.HandleFlagCached) } - log.Info("FileCache::DownloadFile : file=%s, fd=%d", options.Name, f.Fd()) - options.Handle.SetFileObject(f) + log.Info("FileCache::DownloadFile : file=%s, fd=%d", handle.Path, f.Fd()) + handle.SetFileObject(f) //set boolean in isDownloadNeeded value to signal that the file has been downloaded noDownloadNeeded := struct{ downloadNeeded bool }{downloadNeeded: false} - options.Handle.SetValue("isDownloadNeeded", noDownloadNeeded) + handle.SetValue("isDownloadNeeded", noDownloadNeeded) - return options.Handle, nil + return handle, nil } // OpenFile: Makes the file available in the local cache for further file operations. @@ -963,7 +963,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + options.Handle, err = fc.DownloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -1001,7 +1001,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() var err error - options.Handle, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Handle.Path, Handle: options.Handle}) + options.Handle, err = fc.DownloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -1331,7 +1331,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } - h, err = fc.DownloadFile(internal.DownloadFileOptions{Name: options.Name, Handle: h}) + h, err = fc.DownloadFile(h) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err diff --git a/internal/component_options.go b/internal/component_options.go index b430cc5f1..ba7e831e9 100644 --- a/internal/component_options.go +++ b/internal/component_options.go @@ -77,11 +77,6 @@ type DeleteFileOptions struct { Name string } -type DownloadFileOptions struct { - Name string - Handle *handlemap.Handle -} - type OpenFileOptions struct { Name string Flags int From e001990b2277dc91ef03ee3d999343b007eaaa1e Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:05:58 -0600 Subject: [PATCH 48/85] adjusted function call parameters in tests for DownloadFile() --- component/file_cache/file_cache_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 5ab87c8fa..220915d23 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -771,7 +771,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { handle, err = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // Download is required - _, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: path, Handle: handle}) + _, err = suite.fileCache.DownloadFile(handle) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1597,7 +1597,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { smallHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathsmall, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening small file - f, err := suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathsmall, Handle: smallHandle}) + f, err := suite.fileCache.DownloadFile(smallHandle) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) @@ -1606,7 +1606,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.DownloadFile(internal.DownloadFileOptions{Name: pathbig, Handle: bigHandle}) + f, err = suite.fileCache.DownloadFile(bigHandle) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) From ff2da1c302a81b9ad2e0282ffe69dc01d766bb1d Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:08:42 -0600 Subject: [PATCH 49/85] commented out debug log for Read() and Write() --- component/libfuse/libfuse2_handler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/component/libfuse/libfuse2_handler.go b/component/libfuse/libfuse2_handler.go index 5a9a995d1..df1b9396b 100644 --- a/component/libfuse/libfuse2_handler.go +++ b/component/libfuse/libfuse2_handler.go @@ -642,7 +642,8 @@ func (cf *CgofuseFS) Open(path string, flags int) (int, uint64) { // Read reads data from a file into the buffer with the given offset. func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { - log.Debug("Libfuse::Read : reading path %s, handle: %d", path, fh) + //skipping the logging to avoid creating log noise and the performance costs from huge number of calls. + //log.Debug("Libfuse::Read : reading path %s, handle: %d", path, fh) // Get the filehandle handle, exists := handlemap.Load(handlemap.HandleID(fh)) if !exists { @@ -681,7 +682,8 @@ func (cf *CgofuseFS) Read(path string, buff []byte, ofst int64, fh uint64) int { // Write writes data to a file from the buffer with the given offset. func (cf *CgofuseFS) Write(path string, buff []byte, ofst int64, fh uint64) int { - log.Debug("Libfuse::Write : Writing path %s, handle: %d", path, fh) + //skipping the logging to avoid creating log noise and the performance costs from huge number of calls + //log.Debug("Libfuse::Write : Writing path %s, handle: %d", path, fh) // Get the filehandle handle, exists := handlemap.Load(handlemap.HandleID(fh)) if !exists { From b74de0c6ef703a33fbbc403caf360327af2a35b4 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:11:36 -0600 Subject: [PATCH 50/85] replaced value adjustment of handle with removing single fileFlagMode value at the end of DownloadFile() --- component/file_cache/file_cache.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 6d036ec8e..c616d73bd 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -849,8 +849,7 @@ func (fc *FileCache) DownloadFile(handle *handlemap.Handle) (*handlemap.Handle, handle.SetFileObject(f) //set boolean in isDownloadNeeded value to signal that the file has been downloaded - noDownloadNeeded := struct{ downloadNeeded bool }{downloadNeeded: false} - handle.SetValue("isDownloadNeeded", noDownloadNeeded) + handle.RemoveValue("fileFlagMode") return handle, nil } From ff7a6b5f3617d9eb0b8edda955af2ca3c8276178 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:12:48 -0600 Subject: [PATCH 51/85] renamed flag to flags for value struct set in OpenFile() --- component/file_cache/file_cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index c616d73bd..5f4df4205 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -868,9 +868,9 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) handle := handlemap.NewHandle(options.Name) handle.SetValue("fileFlagMode", struct { - flag int + flags int fMode fs.FileMode - }{flag: options.Flags, fMode: options.Mode}) + }{flags: options.Flags, fMode: options.Mode}) return handle, nil From 844fb0978f4617d04669d41eb5e0c3320cbb24cc Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:14:08 -0600 Subject: [PATCH 52/85] renamed flag to flags in type assertion in DownloadFile() --- component/file_cache/file_cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 5f4df4205..df7dc3d8c 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -700,14 +700,14 @@ func (fc *FileCache) DownloadFile(handle *handlemap.Handle) (*handlemap.Handle, flagMode, found := handle.GetValue("fileFlagMode") if found { openFileOptions, ok := flagMode.(struct { - flag int + flags int fMode fs.FileMode }) if !ok { log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", handle.Path) return handle, fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) } - flags = openFileOptions.flag + flags = openFileOptions.flags fMode = openFileOptions.fMode } else { return handle, nil From b4c599270c08e96ceaf90a7c827db036fee021eb Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:32:12 -0600 Subject: [PATCH 53/85] moved openFileOptions to it's own defined struct and refactored code respecively. --- component/file_cache/file_cache.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index df7dc3d8c..dcfe79418 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -105,6 +105,11 @@ type FileCacheOptions struct { HardLimit bool `config:"hard-limit" yaml:"hard-limit,omitempty"` } +type openFileOptions struct { + flags int + fMode fs.FileMode +} + const ( compName = "file_cache" defaultMaxEviction = 5000 @@ -699,16 +704,13 @@ func (fc *FileCache) DownloadFile(handle *handlemap.Handle) (*handlemap.Handle, flagMode, found := handle.GetValue("fileFlagMode") if found { - openFileOptions, ok := flagMode.(struct { - flags int - fMode fs.FileMode - }) + fileOptions, ok := flagMode.(openFileOptions) if !ok { log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", handle.Path) return handle, fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) } - flags = openFileOptions.flags - fMode = openFileOptions.fMode + flags = fileOptions.flags + fMode = fileOptions.fMode } else { return handle, nil } @@ -867,10 +869,7 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) handle := handlemap.NewHandle(options.Name) - handle.SetValue("fileFlagMode", struct { - flags int - fMode fs.FileMode - }{flags: options.Flags, fMode: options.Mode}) + handle.SetValue("fileFlagMode", openFileOptions{flags: options.Flags, fMode: options.Mode}) return handle, nil From 06842fb09d8814c5d633c92fd69cd62f9f356b72 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 11:33:19 -0600 Subject: [PATCH 54/85] unexported DownloadFile() and is now downloadFile() --- component/file_cache/file_cache.go | 8 ++++---- component/file_cache/file_cache_test.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index dcfe79418..7635d11f0 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -696,7 +696,7 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { return nil } -func (fc *FileCache) DownloadFile(handle *handlemap.Handle) (*handlemap.Handle, error) { +func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, error) { //extract flags and mode out of the value from handle var flags int @@ -961,7 +961,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - options.Handle, err = fc.DownloadFile(options.Handle) + options.Handle, err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -999,7 +999,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() var err error - options.Handle, err = fc.DownloadFile(options.Handle) + options.Handle, err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -1329,7 +1329,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } - h, err = fc.DownloadFile(h) + h, err = fc.downloadFile(h) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 220915d23..0f6269dec 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -771,7 +771,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { handle, err = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // Download is required - _, err = suite.fileCache.DownloadFile(handle) + _, err = suite.fileCache.downloadFile(handle) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1597,7 +1597,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { smallHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathsmall, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening small file - f, err := suite.fileCache.DownloadFile(smallHandle) + f, err := suite.fileCache.downloadFile(smallHandle) suite.assert.NoError(err) suite.assert.False(f.Dirty()) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) @@ -1606,7 +1606,7 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.DownloadFile(bigHandle) + f, err = suite.fileCache.downloadFile(bigHandle) suite.assert.Error(err) suite.assert.Nil(f) suite.assert.Equal(syscall.ENOSPC, err) From 2540776d227562a72213d95ae430ede26a1cf837 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:09:10 -0600 Subject: [PATCH 55/85] WIP add downloadFile() call after openFile In tests to resolve pipeline --- component/file_cache/file_cache_linux_test.go | 1 + component/file_cache/file_cache_test.go | 1 + 2 files changed, 2 insertions(+) diff --git a/component/file_cache/file_cache_linux_test.go b/component/file_cache/file_cache_linux_test.go index 1f1fd9163..9dd014ff3 100644 --- a/component/file_cache/file_cache_linux_test.go +++ b/component/file_cache/file_cache_linux_test.go @@ -242,6 +242,7 @@ func (suite *fileCacheLinuxTestSuite) TestChownInCache() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: path, Mode: 0777}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) + suite.fileCache.downloadFile(openHandle) // Path should be in the file cache _, err := os.Stat(suite.cache_path + "/" + path) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 0f6269dec..a31134c50 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1328,6 +1328,7 @@ func (suite *fileCacheTestSuite) TestRenameFileAndCacheCleanupWithNoTimeout() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: src, Mode: 0666}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: src, Mode: 0666}) + suite.fileCache.downloadFile(openHandle) // Path should be in the file cache _, err := os.Stat(suite.cache_path + "/" + src) From 447584d0e8ff8e613c6bb1ee21de06c6cc32c00a Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:11:31 -0600 Subject: [PATCH 56/85] removed var redefine for handle in TruncateFile() --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 7635d11f0..5ff88455e 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1329,7 +1329,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } - h, err = fc.downloadFile(h) + _, err = fc.downloadFile(h) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err From 87b126a9d20c3fe0ce7c1fba70bb497a408d0b35 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:12:55 -0600 Subject: [PATCH 57/85] removed handle redefine assignment for downloadFile() call in ReadInBuffer() and WriteFile() --- component/file_cache/file_cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 5ff88455e..d76ec954a 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -961,7 +961,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - options.Handle, err = fc.downloadFile(options.Handle) + _, err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -999,7 +999,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() var err error - options.Handle, err = fc.downloadFile(options.Handle) + _, err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } From f374a1ffb5f63d42b4408164b9c559d69e3fb5a2 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:23:17 -0600 Subject: [PATCH 58/85] add downlaodFile() after OpenFile() in TestRenameFileInCache() --- component/file_cache/file_cache_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index a31134c50..454ee3b23 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1224,6 +1224,8 @@ func (suite *fileCacheTestSuite) TestRenameFileInCache() { suite.assert.NoError(err) openHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: src, Mode: 0666}) suite.assert.NoError(err) + _, err = suite.fileCache.downloadFile(openHandle) + suite.assert.NoError(err) // Path should be in the file cache _, err = os.Stat(common.JoinUnixFilepath(suite.cache_path, src)) From c6a966c6b0a99924cf180d0a1c27539ccb7d9b87 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:25:36 -0600 Subject: [PATCH 59/85] added error check to downloadFile() call in TestRenameFileAndCacheCleanupWithNoTimeout() --- component/file_cache/file_cache_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 454ee3b23..91a40112c 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1330,10 +1330,11 @@ func (suite *fileCacheTestSuite) TestRenameFileAndCacheCleanupWithNoTimeout() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: src, Mode: 0666}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: src, Mode: 0666}) - suite.fileCache.downloadFile(openHandle) + _, err := suite.fileCache.downloadFile(openHandle) + suite.assert.NoError(err) // Path should be in the file cache - _, err := os.Stat(suite.cache_path + "/" + src) + _, err = os.Stat(suite.cache_path + "/" + src) suite.assert.True(err == nil || os.IsExist(err)) // Path should be in fake storage _, err = os.Stat(suite.fake_storage_path + "/" + src) From f9e645771d6957a887b5a33d1020b67de80a11f5 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Thu, 6 Jun 2024 17:43:48 -0600 Subject: [PATCH 60/85] Added downloadFile() call to test with error handle. --- component/file_cache/file_cache_linux_test.go | 5 +++-- component/file_cache/file_cache_windows_test.go | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/component/file_cache/file_cache_linux_test.go b/component/file_cache/file_cache_linux_test.go index 9dd014ff3..f43554882 100644 --- a/component/file_cache/file_cache_linux_test.go +++ b/component/file_cache/file_cache_linux_test.go @@ -242,10 +242,11 @@ func (suite *fileCacheLinuxTestSuite) TestChownInCache() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: path, Mode: 0777}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) - suite.fileCache.downloadFile(openHandle) + _, err := suite.fileCache.downloadFile(openHandle) + suite.assert.NoError(err) // Path should be in the file cache - _, err := os.Stat(suite.cache_path + "/" + path) + _, err = os.Stat(suite.cache_path + "/" + path) suite.assert.True(err == nil || os.IsExist(err)) // Path should be in fake storage _, err = os.Stat(suite.fake_storage_path + "/" + path) diff --git a/component/file_cache/file_cache_windows_test.go b/component/file_cache/file_cache_windows_test.go index 6e628553f..a0ffcf7c5 100644 --- a/component/file_cache/file_cache_windows_test.go +++ b/component/file_cache/file_cache_windows_test.go @@ -146,9 +146,11 @@ func (suite *fileCacheWindowsTestSuite) TestChownInCache() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: path, Mode: 0777}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) + _, err := suite.fileCache.downloadFile(openHandle) + suite.assert.NoError(err) // Path should be in the file cache - _, err := os.Stat(suite.cache_path + "/" + path) + _, err = os.Stat(suite.cache_path + "/" + path) suite.assert.True(err == nil || os.IsExist(err)) // Path should be in fake storage _, err = os.Stat(suite.fake_storage_path + "/" + path) @@ -158,7 +160,7 @@ func (suite *fileCacheWindowsTestSuite) TestChownInCache() { owner := os.Getuid() group := os.Getgid() err = suite.fileCache.Chown(internal.ChownOptions{Name: path, Owner: owner, Group: group}) - suite.assert.Nil(err) + suite.assert.NoError(err) // Checking that nothing changed with existing files _, err = os.Stat(suite.cache_path + "/" + path) From c20cabc70ccc7f8f9646459cb59d02e0713634b9 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 12:45:00 -0600 Subject: [PATCH 61/85] cleaned up empty lines --- component/file_cache/file_cache.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index d76ec954a..b1788b594 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -730,7 +730,6 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", handle.Path, err.Error()) } - // redundantly check if file is needs downloading fileMode := fc.defaultPermission if downloadRequired { log.Debug("FileCache::download : Need to download %s", handle.Path) @@ -872,7 +871,6 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand handle.SetValue("fileFlagMode", openFileOptions{flags: options.Flags, fMode: options.Mode}) return handle, nil - } // CloseFile: Flush the file and invalidate it from the cache. @@ -926,7 +924,6 @@ func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { // ReadFile: Read the local file func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. - localPath := common.JoinUnixFilepath(fc.tmpPath, options.Handle.Path) fc.policy.CacheValid(localPath) @@ -1033,7 +1030,6 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // Removing Pwrite as it is not supported on Windows // bytesWritten, err := syscall.Pwrite(options.Handle.FD(), options.Data, options.Offset) - bytesWritten, err := f.WriteAt(options.Data, options.Offset) if err == nil { @@ -1310,9 +1306,8 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } } - var err error = nil - var h *handlemap.Handle + var err error if options.Size == 0 { // If size is 0 then no need to download any file we can just create an empty file h, err = fc.CreateFile(internal.CreateFileOptions{Name: options.Name, Mode: fc.defaultPermission}) @@ -1323,7 +1318,6 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } else { // If size is not 0 then we need to open the file and then truncate it // Open will force download if file was not present in local system - h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) @@ -1334,7 +1328,6 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err } - } // Update the size of the file in the local cache From 158d0412530daadbf7420eb973436bd8ce459335 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 12:49:20 -0600 Subject: [PATCH 62/85] removed handlemap.Handle return from downloadFile --- component/file_cache/file_cache.go | 38 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index b1788b594..cf8e5dff1 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -696,7 +696,7 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { return nil } -func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, error) { +func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { //extract flags and mode out of the value from handle var flags int @@ -706,16 +706,16 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, if found { fileOptions, ok := flagMode.(openFileOptions) if !ok { - log.Err("FileCache::DownloadFile : error Type assertion failed on getting flag for %s", handle.Path) - return handle, fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) + log.Err("FileCache::downloadFile : error Type assertion failed on getting flag for %s", handle.Path) + return fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) } flags = fileOptions.flags fMode = fileOptions.fMode } else { - return handle, nil + return nil } - log.Trace("FileCache::DownloadFile : name=%s, flags=%d, mode=%s", handle.Path, flags, fMode) + log.Trace("FileCache::downloadFile : name=%s, flags=%d, mode=%s", handle.Path, flags, fMode) localPath := common.JoinUnixFilepath(fc.tmpPath, handle.Path) var f *os.File @@ -727,7 +727,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, fc.policy.CacheValid(localPath) downloadRequired, fileExists, attr, err := fc.isDownloadRequired(localPath, handle.Path, flock) if err != nil { - log.Err("FileCache::DownloadFile : Failed to check if download is required for %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : Failed to check if download is required for %s [%s]", handle.Path, err.Error()) } fileMode := fc.defaultPermission @@ -751,7 +751,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) if err != nil { log.Err("FileCache::download : error creating directory structure for file %s [%s]", handle.Path, err.Error()) - return nil, err + return err } } @@ -759,7 +759,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) if err != nil { log.Err("FileCache::download : error creating new file %s [%s]", handle.Path, err.Error()) - return nil, err + return err } if flags&os.O_TRUNC != 0 { @@ -774,7 +774,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, } else { if (currSize + float64(fileSize)) > fc.diskHighWaterMark { log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, handle.Path) - return nil, syscall.ENOSPC + return syscall.ENOSPC } } @@ -793,7 +793,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, log.Err("FileCache::download : error downloading file from storage %s [%s]", handle.Path, err.Error()) _ = f.Close() _ = os.Remove(localPath) - return nil, err + return err } } @@ -812,7 +812,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, // If user has selected some non default mode in config then every local file shall be created with that mode only err = os.Chmod(localPath, fileMode) if err != nil { - log.Err("FileCache::DownloadFile : Failed to change mode of file %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : Failed to change mode of file %s [%s]", handle.Path, err.Error()) } // TODO: When chown is supported should we update that? @@ -820,7 +820,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, // chtimes shall be the last api otherwise calling chmod/chown will update the last change time err = os.Chtimes(localPath, attr.Atime, attr.Mtime) if err != nil { - log.Err("FileCache::DownloadFile : Failed to change times of file %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : Failed to change times of file %s [%s]", handle.Path, err.Error()) } } @@ -829,8 +829,8 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, // Open the file and grab a shared lock to prevent deletion by the cache policy. f, err = common.OpenFile(localPath, flags, fMode) if err != nil { - log.Err("FileCache::DownloadFile : error opening cached file %s [%s]", handle.Path, err.Error()) - return nil, err + log.Err("FileCache::downloadFile : error opening cached file %s [%s]", handle.Path, err.Error()) + return err } // Increment the handle count in this lock item as there is one handle open for this now @@ -846,13 +846,13 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) (*handlemap.Handle, handle.Flags.Set(handlemap.HandleFlagCached) } - log.Info("FileCache::DownloadFile : file=%s, fd=%d", handle.Path, f.Fd()) + log.Info("FileCache::downloadFile : file=%s, fd=%d", handle.Path, f.Fd()) handle.SetFileObject(f) //set boolean in isDownloadNeeded value to signal that the file has been downloaded handle.RemoveValue("fileFlagMode") - return handle, nil + return nil } // OpenFile: Makes the file available in the local cache for further file operations. @@ -958,7 +958,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er options.Handle.Lock() var err error - _, err = fc.downloadFile(options.Handle) + err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -996,7 +996,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { options.Handle.Lock() var err error - _, err = fc.downloadFile(options.Handle) + err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -1323,7 +1323,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) } - _, err = fc.downloadFile(h) + err = fc.downloadFile(h) if err != nil { log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) return err From cb60394c27e38d03c526c41df22e7e456799713f Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 13:05:39 -0600 Subject: [PATCH 63/85] adjusted type assertion to maintain consistent style --- component/file_cache/file_cache.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index cf8e5dff1..ee4c4e567 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -697,25 +697,18 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { } func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { + log.Trace("FileCache::downloadFile : name=%s", handle.Path) //extract flags and mode out of the value from handle var flags int var fMode fs.FileMode - - flagMode, found := handle.GetValue("fileFlagMode") - if found { - fileOptions, ok := flagMode.(openFileOptions) - if !ok { - log.Err("FileCache::downloadFile : error Type assertion failed on getting flag for %s", handle.Path) - return fmt.Errorf("type assertion failed on getting flag for %s", handle.Path) - } - flags = fileOptions.flags - fMode = fileOptions.fMode - } else { + val, found := handle.GetValue("openFileOptions") + if !found { return nil } - - log.Trace("FileCache::downloadFile : name=%s, flags=%d, mode=%s", handle.Path, flags, fMode) + fileOptions := val.(openFileOptions) + flags = fileOptions.flags + fMode = fileOptions.fMode localPath := common.JoinUnixFilepath(fc.tmpPath, handle.Path) var f *os.File From 6e28d3d37954ef7f20fe212664caf12101c0d509 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 13:07:32 -0600 Subject: [PATCH 64/85] changed "fileFlagMode" key to "openFileOptions" for value in handle --- component/file_cache/file_cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index ee4c4e567..373c9cb4d 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -843,7 +843,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { handle.SetFileObject(f) //set boolean in isDownloadNeeded value to signal that the file has been downloaded - handle.RemoveValue("fileFlagMode") + handle.RemoveValue("openFileOptions") return nil } @@ -861,7 +861,7 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) handle := handlemap.NewHandle(options.Name) - handle.SetValue("fileFlagMode", openFileOptions{flags: options.Flags, fMode: options.Mode}) + handle.SetValue("openFileOptions", openFileOptions{flags: options.Flags, fMode: options.Mode}) return handle, nil } From d1527cd65d2ecb0c5e58f02d7829044547796994 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 13:22:14 -0600 Subject: [PATCH 65/85] deleted fileCacheStatsCollector from OpenFile --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 373c9cb4d..e0f5126fb 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -859,7 +859,7 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand return nil, err } - fileCacheStatsCollector.UpdateStats(stats_manager.Increment, dlFiles, (int64)(1)) + // create handle and set value handle := handlemap.NewHandle(options.Name) handle.SetValue("openFileOptions", openFileOptions{flags: options.Flags, fMode: options.Mode}) From eb2d029549c8c3edf85fa829a8108682844f5a68 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 13:25:44 -0600 Subject: [PATCH 66/85] corrected comment in TruncateFile --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index e0f5126fb..701ca5568 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1310,7 +1310,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } } else { // If size is not 0 then we need to open the file and then truncate it - // Open will force download if file was not present in local system + // downloadFile will provide download if file was not present in local system h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) From 7d9cfd4e9b03284f2f6be993bbcc9effe5c13bc1 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 13:33:13 -0600 Subject: [PATCH 67/85] corrected return type var assignment in TestChownInCache() --- component/file_cache/file_cache_windows_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache_windows_test.go b/component/file_cache/file_cache_windows_test.go index a0ffcf7c5..6b84c68ea 100644 --- a/component/file_cache/file_cache_windows_test.go +++ b/component/file_cache/file_cache_windows_test.go @@ -146,7 +146,7 @@ func (suite *fileCacheWindowsTestSuite) TestChownInCache() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: path, Mode: 0777}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) - _, err := suite.fileCache.downloadFile(openHandle) + err := suite.fileCache.downloadFile(openHandle) suite.assert.NoError(err) // Path should be in the file cache From 61b23188de846cbd7629180eff8445e9a974da2b Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 14:26:02 -0600 Subject: [PATCH 68/85] added value check for handle in CloseFile() --- component/file_cache/file_cache.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 701ca5568..328806e4b 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -878,6 +878,12 @@ func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { return err } + // if file has not been interactively read or written to by end user, then there is no cached file to close. + _, found := options.Handle.GetValue("openFileOptions") + if found { + return nil + } + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::CloseFile : error [missing fd in handle object] %s", options.Handle.Path) From a83cc3cf09fa20417d85850ce7dc075e23bd023b Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 15:48:02 -0600 Subject: [PATCH 69/85] fixed var assignemnts when calling downloadFile --- component/file_cache/file_cache_test.go | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 91a40112c..0ac18b64b 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -35,7 +35,6 @@ import ( "strconv" "strings" "syscall" - "testing" "time" "github.com/Seagate/cloudfuse/common" @@ -771,7 +770,7 @@ func (suite *fileCacheTestSuite) TestOpenFileNotInCache() { handle, err = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // Download is required - _, err = suite.fileCache.downloadFile(handle) + err = suite.fileCache.downloadFile(handle) suite.assert.NoError(err) suite.assert.EqualValues(path, handle.Path) suite.assert.False(handle.Dirty()) @@ -1224,7 +1223,7 @@ func (suite *fileCacheTestSuite) TestRenameFileInCache() { suite.assert.NoError(err) openHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: src, Mode: 0666}) suite.assert.NoError(err) - _, err = suite.fileCache.downloadFile(openHandle) + err = suite.fileCache.downloadFile(openHandle) suite.assert.NoError(err) // Path should be in the file cache @@ -1330,7 +1329,7 @@ func (suite *fileCacheTestSuite) TestRenameFileAndCacheCleanupWithNoTimeout() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: src, Mode: 0666}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: src, Mode: 0666}) - _, err := suite.fileCache.downloadFile(openHandle) + err := suite.fileCache.downloadFile(openHandle) suite.assert.NoError(err) // Path should be in the file cache @@ -1601,23 +1600,23 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { smallHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathsmall, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening small file - f, err := suite.fileCache.downloadFile(smallHandle) + err = suite.fileCache.downloadFile(smallHandle) suite.assert.NoError(err) - suite.assert.False(f.Dirty()) - err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) + suite.assert.False(smallHandle.Dirty()) + err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: smallHandle}) suite.assert.NoError(err) bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - f, err = suite.fileCache.downloadFile(bigHandle) + err = suite.fileCache.downloadFile(bigHandle) suite.assert.Error(err) - suite.assert.Nil(f) + suite.assert.Nil(bigHandle) suite.assert.Equal(syscall.ENOSPC, err) // try writing a small file options1 := internal.CreateFileOptions{Name: pathsmall + "_new", Mode: 0777} - f, err = suite.fileCache.CreateFile(options1) + f, err := suite.fileCache.CreateFile(options1) suite.assert.NoError(err) data = make([]byte, 1*MB) n, err := suite.fileCache.WriteFile(internal.WriteFileOptions{Handle: f, Offset: 0, Data: data}) @@ -1678,6 +1677,3 @@ func (suite *fileCacheTestSuite) TestHandleDataChange() { // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run -func TestFileCacheTestSuite(t *testing.T) { - suite.Run(t, new(fileCacheTestSuite)) -} From d2486c68bbda5ac1090f834443eb482213c2a0c5 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Mon, 10 Jun 2024 15:54:26 -0600 Subject: [PATCH 70/85] adjsuted var assignements when calling downloadFile --- component/file_cache/file_cache_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache_linux_test.go b/component/file_cache/file_cache_linux_test.go index f43554882..3bfaae782 100644 --- a/component/file_cache/file_cache_linux_test.go +++ b/component/file_cache/file_cache_linux_test.go @@ -242,7 +242,7 @@ func (suite *fileCacheLinuxTestSuite) TestChownInCache() { createHandle, _ := suite.fileCache.CreateFile(internal.CreateFileOptions{Name: path, Mode: 0777}) suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: createHandle}) openHandle, _ := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Mode: 0777}) - _, err := suite.fileCache.downloadFile(openHandle) + err := suite.fileCache.downloadFile(openHandle) suite.assert.NoError(err) // Path should be in the file cache From a4e1366801e69f6b36f1ff263073d74c07441bbc Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:23:37 -0600 Subject: [PATCH 71/85] changed commment and removed akward wording --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 328806e4b..e64a3fd93 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1316,7 +1316,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { } } else { // If size is not 0 then we need to open the file and then truncate it - // downloadFile will provide download if file was not present in local system + // downloadFile will download if file was not present in local system h, err = fc.OpenFile(internal.OpenFileOptions{Name: options.Name, Flags: os.O_RDWR, Mode: fc.defaultPermission}) if err != nil { log.Err("FileCache::TruncateFile : Error calling OpenFile with %s [%s]", options.Name, err.Error()) From 150b089b96a46e9b63f68c1758a327b3d3f59c40 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:25:33 -0600 Subject: [PATCH 72/85] moved handle lock into downloadFile --- component/file_cache/file_cache.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index e64a3fd93..84b6dd639 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -699,6 +699,9 @@ func (fc *FileCache) DeleteFile(options internal.DeleteFileOptions) error { func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { log.Trace("FileCache::downloadFile : name=%s", handle.Path) + handle.Lock() + defer handle.Unlock() + //extract flags and mode out of the value from handle var flags int var fMode fs.FileMode @@ -955,13 +958,11 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) - options.Handle.Lock() var err error err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } - options.Handle.Unlock() f := options.Handle.GetFileObject() if f == nil { @@ -993,13 +994,11 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) - options.Handle.Lock() var err error err = fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } - options.Handle.Unlock() f := options.Handle.GetFileObject() if f == nil { From 3af706f060c95e75d29137b4923a2dbfb17531cb Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:29:55 -0600 Subject: [PATCH 73/85] move handle value check further up CloseFile --- component/file_cache/file_cache.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 84b6dd639..588b87ef6 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -873,6 +873,12 @@ func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Hand func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { log.Trace("FileCache::CloseFile : name=%s, handle=%d", options.Handle.Path, options.Handle.ID) + // if file has not been interactively read or written to by end user, then there is no cached file to close. + _, found := options.Handle.GetValue("openFileOptions") + if found { + return nil + } + localPath := common.JoinUnixFilepath(fc.tmpPath, options.Handle.Path) err := fc.FlushFile(internal.FlushFileOptions{Handle: options.Handle}) //nolint @@ -881,12 +887,6 @@ func (fc *FileCache) CloseFile(options internal.CloseFileOptions) error { return err } - // if file has not been interactively read or written to by end user, then there is no cached file to close. - _, found := options.Handle.GetValue("openFileOptions") - if found { - return nil - } - f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::CloseFile : error [missing fd in handle object] %s", options.Handle.Path) From 704b13475fc9b617aa84f642b50b0e06b80b6f10 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:32:02 -0600 Subject: [PATCH 74/85] remove var err error declarations from ReadInBuffer and WriteFile --- component/file_cache/file_cache.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 588b87ef6..9f3f9d559 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -958,8 +958,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. // log.Debug("FileCache::ReadInBuffer : Reading %v bytes from %s", len(options.Data), options.Handle.Path) - var err error - err = fc.downloadFile(options.Handle) + err := fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } @@ -994,8 +993,7 @@ func (fc *FileCache) WriteFile(options internal.WriteFileOptions) (int, error) { // The file should already be in the cache since CreateFile/OpenFile was called before and a shared lock was acquired. //log.Debug("FileCache::WriteFile : Writing %v bytes from %s", len(options.Data), options.Handle.Path) - var err error - err = fc.downloadFile(options.Handle) + err := fc.downloadFile(options.Handle) if err != nil { return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) } From 7ac582f874a0f6e38c787038a98d1fb1b2bbb231 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:34:07 -0600 Subject: [PATCH 75/85] changed "FileCache::download" in erro stiring to "FileCache::downloadFile" --- component/file_cache/file_cache.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 9f3f9d559..570108fbf 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -728,7 +728,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { fileMode := fc.defaultPermission if downloadRequired { - log.Debug("FileCache::download : Need to download %s", handle.Path) + log.Debug("FileCache::downloadFile : Need to download %s", handle.Path) fileSize := int64(0) if attr != nil { @@ -736,17 +736,17 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { } if fileExists { - log.Debug("FileCache::download : Delete cached file %s", handle.Path) + log.Debug("FileCache::downloadFile : Delete cached file %s", handle.Path) err := deleteFile(localPath) if err != nil && !os.IsNotExist(err) { - log.Err("FileCache::download : Failed to delete old file %s", handle.Path) + log.Err("FileCache::downloadFile : Failed to delete old file %s", handle.Path) } } else { // Create the file if if doesn't already exist. err := os.MkdirAll(filepath.Dir(localPath), fc.defaultPermission) if err != nil { - log.Err("FileCache::download : error creating directory structure for file %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : error creating directory structure for file %s [%s]", handle.Path, err.Error()) return err } } @@ -754,7 +754,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { // Open the file in write mode. f, err = common.OpenFile(localPath, os.O_CREATE|os.O_RDWR, fMode) if err != nil { - log.Err("FileCache::download : error creating new file %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : error creating new file %s [%s]", handle.Path, err.Error()) return err } @@ -766,10 +766,10 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { if fc.diskHighWaterMark != 0 { currSize, err := common.GetUsage(fc.tmpPath) if err != nil { - log.Err("FileCache::download : error getting current usage of cache [%s]", err.Error()) + log.Err("FileCache::downloadFile : error getting current usage of cache [%s]", err.Error()) } else { if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::download : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, handle.Path) + log.Err("FileCache::downloadFile : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, handle.Path) return syscall.ENOSPC } } @@ -786,7 +786,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { }) if err != nil { // File was created locally and now download has failed so we need to delete it back from local cache - log.Err("FileCache::download : error downloading file from storage %s [%s]", handle.Path, err.Error()) + log.Err("FileCache::downloadFile : error downloading file from storage %s [%s]", handle.Path, err.Error()) _ = f.Close() _ = os.Remove(localPath) return err @@ -796,7 +796,7 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { // Update the last download time of this file flock.SetDownloadTime() - log.Debug("FileCache::download : Download of %s is complete", handle.Path) + log.Debug("FileCache::downloadFile : Download of %s is complete", handle.Path) f.Close() // After downloading the file, update the modified times and mode of the file. From b00dce9a8abbbbc8fc68b711fd558f188dbee4c8 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:39:22 -0600 Subject: [PATCH 76/85] removed "for' from error message in ReadInBuffer --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 570108fbf..7cedb6960 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -960,7 +960,7 @@ func (fc *FileCache) ReadInBuffer(options internal.ReadInBufferOptions) (int, er err := fc.downloadFile(options.Handle) if err != nil { - return 0, fmt.Errorf("error downloading file for %s [%s]", options.Handle.Path, err) + return 0, fmt.Errorf("error downloading file %s [%s]", options.Handle.Path, err) } f := options.Handle.GetFileObject() From c9f18461ede070bd2c55cd2d756b2116eea908c3 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 13:43:21 -0600 Subject: [PATCH 77/85] adjusted Error log to reflect function called --- component/file_cache/file_cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 7cedb6960..1bedcf515 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -1321,7 +1321,7 @@ func (fc *FileCache) TruncateFile(options internal.TruncateFileOptions) error { err = fc.downloadFile(h) if err != nil { - log.Err("FileCache::TruncateFile : Error opening file %s [%s]", options.Name, err.Error()) + log.Err("FileCache::TruncateFile : Error calling downloadFile with %s [%s]", options.Name, err.Error()) return err } } From 1bef0510a3fccf7cf115d4037c5151fb953b6591 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 17:09:39 -0600 Subject: [PATCH 78/85] put the TestFileCacheTestSuite back in file_cache_test.go --- component/file_cache/file_cache_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 0ac18b64b..3fd6ab043 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -35,6 +35,7 @@ import ( "strconv" "strings" "syscall" + "testing" "time" "github.com/Seagate/cloudfuse/common" @@ -1677,3 +1678,6 @@ func (suite *fileCacheTestSuite) TestHandleDataChange() { // In order for 'go test' to run this suite, we need to create // a normal test function and pass our suite to suite.Run +func TestFileCacheTestSuite(t *testing.T) { + suite.Run(t, new(fileCacheTestSuite)) +} From e927ccd0a2fb316cf8be2ff21de753c0bc835be7 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Tue, 11 Jun 2024 18:31:21 -0600 Subject: [PATCH 79/85] put ReadFile() call back in place for TestReadFile() --- component/file_cache/file_cache_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 3fd6ab043..324496176 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -896,10 +896,11 @@ func (suite *fileCacheTestSuite) TestReadFile() { suite.fileCache.FlushFile(internal.FlushFileOptions{Handle: handle}) handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - - d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) - suite.assert.EqualValues(9, d) + suite.assert.EqualValues(9, n) + suite.assert.EqualValues(data, d) } func (suite *fileCacheTestSuite) TestReadFileNoFlush() { From 6b07bbb54837355d9b90b7117b37378a6d48e668 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 13:18:03 -0600 Subject: [PATCH 80/85] adjusted assertions for handle not being nil in TestHardLimitOnSize --- component/file_cache/file_cache_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 324496176..090cfdc8f 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1613,7 +1613,8 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { // try opening bigger file which shall fail due to hardlimit err = suite.fileCache.downloadFile(bigHandle) suite.assert.Error(err) - suite.assert.Nil(bigHandle) + suite.assert.NotNil(bigHandle.GetValue("openFileOptions")) + suite.assert.Nil(bigHandle.GetFileObject()) suite.assert.Equal(syscall.ENOSPC, err) // try writing a small file From 00dcbb970dacad49ae1b8593c2a6dcb42351b1fd Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 14:09:08 -0600 Subject: [PATCH 81/85] removed comments in test code --- component/file_cache/file_cache_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 090cfdc8f..ea875fb97 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -667,7 +667,7 @@ func (suite *fileCacheTestSuite) TestSyncFile() { handle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: path, Flags: os.O_RDWR, Mode: 0777}) handlemap.Add(handle) suite.assert.NoError(err) - err = suite.fileCache.SyncFile(internal.SyncFileOptions{Handle: handle}) //sync flag set here gets wiped in WriteFile + err = suite.fileCache.SyncFile(internal.SyncFileOptions{Handle: handle}) suite.assert.NoError(err) testData := "test data" data := []byte(testData) @@ -676,7 +676,7 @@ func (suite *fileCacheTestSuite) TestSyncFile() { handle, loaded := handlemap.Load(handle.ID) suite.assert.True(loaded) suite.fileCache.FlushFile(internal.FlushFileOptions{Handle: handle}) - suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle}) //file isn't getting deleted Handle.Fsynced() is false + suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: handle}) // Path should not be in file cache _, err = os.Stat(common.JoinUnixFilepath(suite.cache_path, path)) From d047eeba9eb28f23a014d784ea26fb5f52b89732 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 15:57:58 -0600 Subject: [PATCH 82/85] Moved hard limit check from downloadFile to OpenFile --- component/file_cache/file_cache.go | 33 +++++++++++++++---------- component/file_cache/file_cache_test.go | 7 ++---- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 1bedcf515..08baea38d 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -763,18 +763,6 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { } if fileSize > 0 { - if fc.diskHighWaterMark != 0 { - currSize, err := common.GetUsage(fc.tmpPath) - if err != nil { - log.Err("FileCache::downloadFile : error getting current usage of cache [%s]", err.Error()) - } else { - if (currSize + float64(fileSize)) > fc.diskHighWaterMark { - log.Err("FileCache::downloadFile : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, handle.Path) - return syscall.ENOSPC - } - } - - } // Download/Copy the file from storage to the local file. // We pass a count of 0 to get the entire object err = fc.NextComponent().CopyToFile( @@ -855,13 +843,32 @@ func (fc *FileCache) downloadFile(handle *handlemap.Handle) error { func (fc *FileCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Handle, error) { log.Trace("FileCache::OpenFile : name=%s, flags=%d, mode=%s", options.Name, options.Flags, options.Mode) - _, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name}) + attr, err := fc.NextComponent().GetAttr(internal.GetAttrOptions{Name: options.Name}) // return err in case of authorization permission mismatch if err != nil && err == syscall.EACCES { return nil, err } + fileSize := int64(0) + if attr != nil { + fileSize = int64(attr.Size) + } + + if fileSize > 0 { + if fc.diskHighWaterMark != 0 { + currSize, err := common.GetUsage(fc.tmpPath) + if err != nil { + log.Err("FileCache::OpenFile : error getting current usage of cache [%s]", err.Error()) + } else { + if (currSize + float64(fileSize)) > fc.diskHighWaterMark { + log.Err("FileCache::OpenFile : cache size limit reached [%f] failed to open %s", fc.maxCacheSize, options.Name) + return nil, syscall.ENOSPC + } + } + } + } + // create handle and set value handle := handlemap.NewHandle(options.Name) handle.SetValue("openFileOptions", openFileOptions{flags: options.Flags, fMode: options.Mode}) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index ea875fb97..33a19b2b6 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -1608,13 +1608,10 @@ func (suite *fileCacheTestSuite) TestHardLimitOnSize() { err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: smallHandle}) suite.assert.NoError(err) - bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) - suite.assert.NoError(err) // try opening bigger file which shall fail due to hardlimit - err = suite.fileCache.downloadFile(bigHandle) + bigHandle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: pathbig, Flags: os.O_RDONLY, Mode: suite.fileCache.defaultPermission}) suite.assert.Error(err) - suite.assert.NotNil(bigHandle.GetValue("openFileOptions")) - suite.assert.Nil(bigHandle.GetFileObject()) + suite.assert.Nil(bigHandle) suite.assert.Equal(syscall.ENOSPC, err) // try writing a small file From fa6dce72eba806c88bfe0c564db4dd56ec91a731 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 16:19:50 -0600 Subject: [PATCH 83/85] Added ReadFile calls and respecitve asserts in tests. --- component/file_cache/file_cache_test.go | 32 +++++++++++++++++++------ 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index 33a19b2b6..b833bc935 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -914,9 +914,10 @@ func (suite *fileCacheTestSuite) TestReadFileNoFlush() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + err := suite.fileCache.downloadFile(handle) + d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) - suite.assert.EqualValues(9, d) + suite.assert.EqualValues(data, d) } func (suite *fileCacheTestSuite) TestReadFileErrorBadFd() { @@ -1484,9 +1485,13 @@ func (suite *fileCacheTestSuite) TestCachePathSymlink() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - d, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + suite.assert.NoError(err) + suite.assert.EqualValues(9, n) + + d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) - suite.assert.EqualValues(9, d) + suite.assert.EqualValues(data, d) } func (suite *fileCacheTestSuite) TestZZOffloadIO() { @@ -1540,10 +1545,12 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { suite.setupTestHelper(config) // setup a new file cache with a custom config (teardown will occur after the test as usual) path := "file42" - err := os.WriteFile(suite.fake_storage_path+"/"+path, []byte("test data"), 0777) + byteArr := []byte("test data") + err := os.WriteFile(suite.fake_storage_path+"/"+path, byteArr, 0777) suite.assert.NoError(err) data := make([]byte, 20) + options := internal.OpenFileOptions{Name: path, Mode: 0777} // Read file once and we shall get the same data @@ -1553,11 +1560,15 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(9, n) + d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) + suite.assert.Equal(d, byteArr) + suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) - // Modify the fil ein background but we shall still get the old data - err = os.WriteFile(suite.fake_storage_path+"/"+path, []byte("test data1"), 0777) + // Modify the file in background but we shall still get the old data + byteArr = []byte("test data1") + err = os.WriteFile(suite.fake_storage_path+"/"+path, byteArr, 0777) suite.assert.NoError(err) f, err = suite.fileCache.OpenFile(options) suite.assert.NoError(err) @@ -1565,10 +1576,14 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(9, n) + d, err = suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) + suite.assert.NotEqual(d, byteArr) + suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) // Now wait for 5 seconds and we shall get the updated content on next read + byteArr = []byte("test data123456") err = os.WriteFile(suite.fake_storage_path+"/"+path, []byte("test data123456"), 0777) suite.assert.NoError(err) time.Sleep(12 * time.Second) @@ -1578,6 +1593,9 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(15, n) + d, err = suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) + suite.assert.Equal(d, byteArr) + suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) } From 7bb87b396736b3a11e70185ad34dff5b69807742 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 16:41:02 -0600 Subject: [PATCH 84/85] removed extra ReadFile and download calls in tests and added download to ReadFile function --- component/file_cache/file_cache.go | 2 ++ component/file_cache/file_cache_test.go | 21 +++++---------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index 08baea38d..c9bb008c3 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -936,6 +936,8 @@ func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) localPath := common.JoinUnixFilepath(fc.tmpPath, options.Handle.Path) fc.policy.CacheValid(localPath) + err := fc.downloadFile(options.Handle) + f := options.Handle.GetFileObject() if f == nil { log.Err("FileCache::ReadFile : error [couldn't find fd in handle] %s", options.Handle.Path) diff --git a/component/file_cache/file_cache_test.go b/component/file_cache/file_cache_test.go index b833bc935..7e8d1fdd2 100644 --- a/component/file_cache/file_cache_test.go +++ b/component/file_cache/file_cache_test.go @@ -895,11 +895,10 @@ func (suite *fileCacheTestSuite) TestReadFile() { suite.fileCache.WriteFile(internal.WriteFileOptions{Handle: handle, Offset: 0, Data: data}) suite.fileCache.FlushFile(internal.FlushFileOptions{Handle: handle}) - handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + handle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) + suite.assert.NoError(err) d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) - suite.assert.EqualValues(9, n) suite.assert.EqualValues(data, d) } @@ -912,9 +911,9 @@ func (suite *fileCacheTestSuite) TestReadFileNoFlush() { data := []byte(testData) suite.fileCache.WriteFile(internal.WriteFileOptions{Handle: handle, Offset: 0, Data: data}) - handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) + handle, err := suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) + suite.assert.NoError(err) - err := suite.fileCache.downloadFile(handle) d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) suite.assert.EqualValues(data, d) @@ -1485,9 +1484,8 @@ func (suite *fileCacheTestSuite) TestCachePathSymlink() { handle, _ = suite.fileCache.OpenFile(internal.OpenFileOptions{Name: file, Mode: 0777}) - n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: handle, Offset: 0, Data: data}) + err = suite.fileCache.downloadFile(handle) suite.assert.NoError(err) - suite.assert.EqualValues(9, n) d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: handle}) suite.assert.NoError(err) @@ -1560,9 +1558,6 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err := suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(9, n) - d, err := suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) - suite.assert.Equal(d, byteArr) - suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) @@ -1576,9 +1571,6 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(9, n) - d, err = suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) - suite.assert.NotEqual(d, byteArr) - suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) @@ -1593,9 +1585,6 @@ func (suite *fileCacheTestSuite) TestReadFileWithRefresh() { n, err = suite.fileCache.ReadInBuffer(internal.ReadInBufferOptions{Handle: f, Offset: 0, Data: data}) suite.assert.NoError(err) suite.assert.Equal(15, n) - d, err = suite.fileCache.ReadFile(internal.ReadFileOptions{Handle: f}) - suite.assert.Equal(d, byteArr) - suite.assert.NoError(err) err = suite.fileCache.CloseFile(internal.CloseFileOptions{Handle: f}) suite.assert.NoError(err) } From a1253aaed8ad18c63d7ff74dc2b39dabbc592811 Mon Sep 17 00:00:00 2001 From: David Habinsky Date: Wed, 12 Jun 2024 16:46:54 -0600 Subject: [PATCH 85/85] added error check for ReadFile --- component/file_cache/file_cache.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/component/file_cache/file_cache.go b/component/file_cache/file_cache.go index c9bb008c3..b74b0e733 100644 --- a/component/file_cache/file_cache.go +++ b/component/file_cache/file_cache.go @@ -937,6 +937,10 @@ func (fc *FileCache) ReadFile(options internal.ReadFileOptions) ([]byte, error) fc.policy.CacheValid(localPath) err := fc.downloadFile(options.Handle) + if err != nil { + log.Err("FileCache::ReadFile : error from calling downloadFile for %s", options.Handle.Path) + return nil, err + } f := options.Handle.GetFileObject() if f == nil {