Use truncate and remove function while eviction from job during CRC c…

…heck (#2015)

internal/cache/file/cache_handler.go

@@ -87,24 +87,13 @@ func (chr *CacheHandler) cleanUpEvictedFile(fileInfo *data.FileInfo) error {
 	chr.jobManager.InvalidateAndRemoveJob(key.ObjectName, key.BucketName)
 
 	localFilePath := util.GetDownloadPath(chr.cacheDir, util.GetObjectPath(key.BucketName, key.ObjectName))
-	// Truncate the file to 0 size, so that even if there are open file handles
-	// and linux doesn't delete the file, the file will not take space.
-	err = os.Truncate(localFilePath, 0)
+	err = util.TruncateAndRemoveFile(localFilePath)
 	if err != nil {
 		if os.IsNotExist(err) {
-			logger.Warnf("cleanUpEvictedFile: file was not present at the time of truncating: %v", err)
+			logger.Warnf("cleanUpEvictedFile: file was not present at the time of clean up: %v", err)
 			return nil
-		} else {
-			return fmt.Errorf("cleanUpEvictedFile: while truncating file: %s, error: %w", localFilePath, err)
-		}
-	}
-	err = os.Remove(localFilePath)
-	if err != nil {
-		if os.IsNotExist(err) {
-			logger.Warnf("cleanUpEvictedFile: file was not present at the time of deleting: %v", err)
-		} else {
-			return fmt.Errorf("cleanUpEvictedFile: while deleting file: %s, error: %w", localFilePath, err)
 		}
+		return fmt.Errorf("cleanUpEvictedFile: error while cleaning up file: %s, error: %w", localFilePath, err)
 	}
 
 	return nil

internal/cache/file/downloader/job.go

@@ -507,8 +507,8 @@ func (job *Job) validateCRC() (err error) {
 	}
 
 	job.fileInfoCache.Erase(fileInfoKeyName)
-	removeErr := os.Remove(job.fileSpec.Path)
-	if removeErr != nil {
+	removeErr := cacheutil.TruncateAndRemoveFile(job.fileSpec.Path)
+	if removeErr != nil && !os.IsNotExist(removeErr) {
 		err = errors.Join(err, removeErr)
 	}
 

internal/cache/util/util.go

@@ -165,3 +165,19 @@ func CalculateFileCRC32(ctx context.Context, filePath string) (uint32, error) {
 
 	return calculateCRC32(ctx, file)
 }
+
+// TruncateAndRemoveFile first truncates the file to 0 and then remove (delete)
+// the file at given path.
+func TruncateAndRemoveFile(filePath string) error {
+	// Truncate the file to 0 size, so that even if there are open file handles
+	// and linux doesn't delete the file, the file will not take space.
+	err := os.Truncate(filePath, 0)
+	if err != nil {
+		return err
+	}
+	err = os.Remove(filePath)
+	if err != nil {
+		return err
+	}
+	return nil
+}

internal/cache/util/util_test.go

@@ -278,6 +278,64 @@ func (ut *utilTest) Test_CalculateFileCRC32_ShouldReturnErrorWhenContextIsCancel
 	ExpectEq(0, crc)
 }
 
+func (ut *utilTest) Test_TruncateAndRemoveFile_FileExists() {
+	// Create a file to be deleted.
+	fileName := "temp.txt"
+	file, err := os.Create(fileName)
+	AssertEq(nil, err)
+	_, err = file.WriteString("Writing some data")
+	AssertEq(nil, err)
+	err = file.Close()
+	AssertEq(nil, err)
+
+	err = TruncateAndRemoveFile(fileName)
+
+	ExpectEq(nil, err)
+	// Check the file is deleted.
+	_, err = os.Stat(fileName)
+	ExpectTrue(os.IsNotExist(err), fmt.Sprintf("expected not exist error but got error: %v", err))
+}
+
+func (ut *utilTest) Test_TruncateAndRemoveFile_FileDoesNotExist() {
+	// Create a file to be deleted.
+	fileName := "temp.txt"
+
+	err := TruncateAndRemoveFile(fileName)
+
+	ExpectTrue(os.IsNotExist(err), fmt.Sprintf("expected not exist error but got error: %v", err))
+}
+
+func (ut *utilTest) Test_TruncateAndRemoveFile_OpenedFileDeleted() {
+	// Create a file to be deleted.
+	fileName := "temp.txt"
+	file, err := os.Create(fileName)
+	AssertEq(nil, err)
+	fileString := "Writing some data"
+	_, err = file.WriteString(fileString)
+	AssertEq(nil, err)
+	// Close the file to get the contents synced.
+	err = file.Close()
+	AssertEq(nil, err)
+	// Open the file again
+	file, err = os.Open(fileName)
+	defer func() {
+		_ = file.Close()
+	}()
+	AssertEq(nil, err)
+	fileInfo, err := file.Stat()
+	AssertEq(nil, err)
+	AssertEq(len(fileString), fileInfo.Size())
+
+	// File is not closed and call TruncateAndRemoveFile
+	err = TruncateAndRemoveFile(fileName)
+
+	ExpectEq(nil, err)
+	// The size of open file should be 0.
+	fileInfo, err = file.Stat()
+	ExpectEq(nil, err)
+	ExpectEq(0, fileInfo.Size())
+}
+
 func Test_CreateCacheDirectoryIfNotPresentAt_ShouldNotReturnAnyErrorWhenDirectoryExists(t *testing.T) {
 	base := path.Join("./", string(testutil.GenerateRandomBytes(4)))
 	dirPath := path.Join(base, "/", "path/cachedir")