diff --git a/internal/storage/caching/fast_stat_bucket.go b/internal/storage/caching/fast_stat_bucket.go index 37ce1d2d49..1d18d1247d 100644 --- a/internal/storage/caching/fast_stat_bucket.go +++ b/internal/storage/caching/fast_stat_bucket.go @@ -367,18 +367,25 @@ func (b *fastStatBucket) UpdateObject( func (b *fastStatBucket) DeleteObject( ctx context.Context, req *gcs.DeleteObjectRequest) (err error) { - b.invalidate(req.Name) err = b.wrapped.DeleteObject(ctx, req) + if err != nil { + b.invalidate(req.Name) + } else { + b.addNegativeEntry(req.Name) + } return } func (b *fastStatBucket) DeleteFolder(ctx context.Context, folderName string) error { err := b.wrapped.DeleteFolder(ctx, folderName) + // In case of an error; invalidate the cached entry. This will make sure that + // gcsfuse is not caching possibly erroneous status of the folder and next + // call will hit GCS backend to probe the latest status. if err != nil { - return err + b.invalidate(folderName) + } else { + b.addNegativeEntryForFolder(folderName) } - // TODO: Caching negative entries for both objects and folders will be implemented together due to test failures. - b.invalidate(folderName) return err } diff --git a/internal/storage/caching/fast_stat_bucket_test.go b/internal/storage/caching/fast_stat_bucket_test.go index 6f0f057116..56e570c096 100644 --- a/internal/storage/caching/fast_stat_bucket_test.go +++ b/internal/storage/caching/fast_stat_bucket_test.go @@ -906,8 +906,8 @@ func (t *DeleteObjectTest) WrappedSucceeds() { const name = "" var err error - // Erase - ExpectCall(t.cache, "Erase")(Any()) + // AddNegativeEntry + ExpectCall(t.cache, "AddNegativeEntry")(Any(), Any()) // Wrapped ExpectCall(t.wrapped, "DeleteObject")(Any(), Any()). @@ -1003,9 +1003,10 @@ func init() { RegisterTestSuite(&DeleteFolderTest{}) } func (t *DeleteFolderTest) Test_DeleteFolder_Success() { const name = "some-name" + ExpectCall(t.cache, "AddNegativeEntryForFolder")(name, Any()). + WillOnce(Return()) ExpectCall(t.wrapped, "DeleteFolder")(Any(), name). WillOnce(Return(nil)) - ExpectCall(t.cache, "Erase")(name).WillOnce(Return()) err := t.bucket.DeleteFolder(context.TODO(), name) @@ -1014,6 +1015,8 @@ func (t *DeleteFolderTest) Test_DeleteFolder_Success() { func (t *DeleteFolderTest) Test_DeleteFolder_Failure() { const name = "some-name" + // Erase + ExpectCall(t.cache, "Erase")(Any()) ExpectCall(t.wrapped, "DeleteFolder")(Any(), name). WillOnce(Return(fmt.Errorf("mock error"))) diff --git a/tools/integration_tests/implicit_dir/local_file_test.go b/tools/integration_tests/implicit_dir/local_file_test.go index 728221fad0..34bb8448c7 100644 --- a/tools/integration_tests/implicit_dir/local_file_test.go +++ b/tools/integration_tests/implicit_dir/local_file_test.go @@ -37,21 +37,23 @@ var ( // ////////////////////////////////////////////////////////////////////// func TestNewFileUnderImplicitDirectoryShouldNotGetSyncedToGCSTillClose(t *testing.T) { - testDirPath = setup.SetupTestDirectory(testDirName) - CreateImplicitDir(ctx, storageClient, testDirName, t) + testBaseDirName := path.Join(testDirName, operations.GetRandomName(t)) + testDirPath = setup.SetupTestDirectoryRecursive(testBaseDirName) + CreateImplicitDir(ctx, storageClient, testBaseDirName, t) fileName := path.Join(ImplicitDirName, FileName1) _, fh := CreateLocalFileInTestDir(ctx, storageClient, testDirPath, fileName, t) operations.WriteWithoutClose(fh, FileContents, t) - ValidateObjectNotFoundErrOnGCS(ctx, storageClient, testDirName, fileName, t) + ValidateObjectNotFoundErrOnGCS(ctx, storageClient, testBaseDirName, fileName, t) // Validate. - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh, testDirName, fileName, FileContents, t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh, testBaseDirName, fileName, FileContents, t) } func TestReadDirForImplicitDirWithLocalFile(t *testing.T) { - testDirPath = setup.SetupTestDirectory(testDirName) - CreateImplicitDir(ctx, storageClient, testDirName, t) + testBaseDirName := path.Join(testDirName, operations.GetRandomName(t)) + testDirPath = setup.SetupTestDirectoryRecursive(testBaseDirName) + CreateImplicitDir(ctx, storageClient, testBaseDirName, t) fileName1 := path.Join(ImplicitDirName, FileName1) fileName2 := path.Join(ImplicitDirName, FileName2) _, fh1 := CreateLocalFileInTestDir(ctx, storageClient, testDirPath, fileName1, t) @@ -66,8 +68,8 @@ func TestReadDirForImplicitDirWithLocalFile(t *testing.T) { operations.VerifyFileEntry(entries[1], FileName2, 0, t) operations.VerifyFileEntry(entries[2], ImplicitFileName1, GCSFileSize, t) // Close the local files. - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh1, testDirName, fileName1, "", t) - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh2, testDirName, fileName2, "", t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh1, testBaseDirName, fileName1, "", t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh2, testBaseDirName, fileName2, "", t) } func TestRecursiveListingWithLocalFiles(t *testing.T) { @@ -80,7 +82,8 @@ func TestRecursiveListingWithLocalFiles(t *testing.T) { // mntDir/implicit/foo2 --- file // mntDir/implicit/implicitFile1 --- file - testDirPath = setup.SetupTestDirectory(testDirName) + testBaseDirName := path.Join(testDirName, operations.GetRandomName(t)) + testDirPath = setup.SetupTestDirectoryRecursive(testBaseDirName) fileName2 := path.Join(ExplicitDirName, ExplicitFileName1) fileName3 := path.Join(ImplicitDirName, FileName2) // Create local file in mnt/ dir. @@ -89,7 +92,7 @@ func TestRecursiveListingWithLocalFiles(t *testing.T) { operations.CreateDirectory(path.Join(testDirPath, ExplicitDirName), t) _, fh2 := CreateLocalFileInTestDir(ctx, storageClient, testDirPath, fileName2, t) // Create implicit dir with 1 local file1 and 1 synced file. - CreateImplicitDir(ctx, storageClient, testDirName, t) + CreateImplicitDir(ctx, storageClient, testBaseDirName, t) _, fh3 := CreateLocalFileInTestDir(ctx, storageClient, testDirPath, fileName3, t) // Recursively list mntDir/ directory. @@ -135,7 +138,7 @@ func TestRecursiveListingWithLocalFiles(t *testing.T) { if err != nil { t.Errorf("filepath.WalkDir() err: %v", err) } - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh1, testDirName, FileName1, "", t) - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh2, testDirName, fileName2, "", t) - CloseFileAndValidateContentFromGCS(ctx, storageClient, fh3, testDirName, fileName3, "", t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh1, testBaseDirName, FileName1, "", t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh2, testBaseDirName, fileName2, "", t) + CloseFileAndValidateContentFromGCS(ctx, storageClient, fh3, testBaseDirName, fileName3, "", t) } diff --git a/tools/integration_tests/kernel_list_cache/infinite_kernel_list_cache_test.go b/tools/integration_tests/kernel_list_cache/infinite_kernel_list_cache_test.go index 87bcafa8b9..9b9a07c053 100644 --- a/tools/integration_tests/kernel_list_cache/infinite_kernel_list_cache_test.go +++ b/tools/integration_tests/kernel_list_cache/infinite_kernel_list_cache_test.go @@ -526,8 +526,12 @@ func TestInfiniteKernelListCacheTest(t *testing.T) { } // Define flag set to run the tests. + // Note: metadata cache is disabled to avoid cache consistency issue between + // gcsfuse cache and kernel cache. As gcsfuse cache might hold the entry which + // already became stale due to delete operation. + // TODO: Replace metadata-cache-ttl-secs with something better flagsSet := [][]string{ - {"--kernel-list-cache-ttl-secs=-1"}, + {"--kernel-list-cache-ttl-secs=-1", "--metadata-cache-ttl-secs=0"}, } // Run tests. diff --git a/tools/integration_tests/util/operations/string_operations.go b/tools/integration_tests/util/operations/string_operations.go index 316e31ff1f..d0dfca0285 100644 --- a/tools/integration_tests/util/operations/string_operations.go +++ b/tools/integration_tests/util/operations/string_operations.go @@ -18,6 +18,8 @@ package operations import ( "strings" "testing" + + "github.com/google/uuid" ) func VerifyExpectedSubstrings(t *testing.T, input string, expectedSubstrings []string) { @@ -35,3 +37,11 @@ func VerifyUnexpectedSubstrings(t *testing.T, input string, unexpectedSubstrings } } } + +func GetRandomName(t *testing.T) string { + id, err := uuid.NewRandom() + if err != nil { + t.Errorf("Error while generating random string, err: %v", err) + } + return id.String() +} diff --git a/tools/integration_tests/util/setup/setup.go b/tools/integration_tests/util/setup/setup.go index 66c919ce87..9d8922e9b2 100644 --- a/tools/integration_tests/util/setup/setup.go +++ b/tools/integration_tests/util/setup/setup.go @@ -382,6 +382,18 @@ func SetupTestDirectory(testDirName string) string { return testDirPath } +// SetupTestDirectoryRecursive recursively creates a testDirectory in the mounted directory and cleans up +// any content present in it. +func SetupTestDirectoryRecursive(testDirName string) string { + testDirPath := path.Join(MntDir(), testDirName) + err := os.MkdirAll(testDirPath, DirPermission_0755) + if err != nil && !strings.Contains(err.Error(), "file exists") { + log.Printf("Error while setting up directory %s for testing: %v", testDirPath, err) + } + CleanUpDir(testDirPath) + return testDirPath +} + // CleanupDirectoryOnGCS cleans up the object/directory path passed in parameter. func CleanupDirectoryOnGCS(ctx context.Context, client *storage.Client, directoryPathOnGCS string) { bucket, dirPath := GetBucketAndObjectBasedOnTypeOfMount(directoryPathOnGCS)