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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 30 additions & 30 deletions internal/fs/inode/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -1092,48 +1092,48 @@ func (d *dirInode) DeleteChildDir(
d.cache.Erase(name)
}

// If the directory is an implicit directory, then no backing object
// exists in the gcs bucket, so returning from here.
// Hierarchical buckets don't have implicit dirs so this will be always false in hierarchical bucket case.
if isImplicitDir {
return nil
}

childName := NewDirName(d.Name(), name)
req := &gcs.DeleteObjectRequest{
Name: childName.GcsObjectName(),
Generation: 0, // Delete the latest version.
}

// Delete the backing object. Unfortunately we have no way to precondition
// this on the directory being empty.
err := d.bucket.DeleteObject(
ctx,
&gcs.DeleteObjectRequest{
Name: childName.GcsObjectName(),
Generation: 0, // Delete the latest version of object named after dir.
})
// Hierarchical Namespace (HNS) Buckets
if d.isBucketHierarchical() {
// Ignoring delete object error here, as in case of hns there is no way of knowing
// if underlying placeholder object exists or not in Hierarchical bucket.
// The DeleteFolder operation handles removing empty folders.
_ = d.bucket.DeleteObject(ctx, req)

if !d.isBucketHierarchical() {
if err != nil {
return fmt.Errorf("DeleteObject: %w", err)
if err := d.bucket.DeleteFolder(ctx, req.Name); err != nil {
return fmt.Errorf("DeleteFolder: %w", err)
}
if !d.IsTypeCacheDeprecated() {
d.cache.Erase(name)

if dirInode != nil {
dirInode.Unlink()
}

return nil
}

// Ignoring delete object error here, as in case of hns there is no way of knowing
// if underlying placeholder object exists or not in Hierarchical bucket.
// The DeleteFolder operation handles removing empty folders.
if err = d.bucket.DeleteFolder(ctx, childName.GcsObjectName()); err != nil {
return fmt.Errorf("DeleteFolder: %w", err)
if isImplicitDir {
if !d.IsTypeCacheDeprecated() {
// If the directory is an implicit directory, then no backing object
// exists in the gcs bucket, so returning from here.
// Hierarchical buckets don't have implicit dirs so this will be always false in hierarchical bucket case.
return nil
}
// Implicit directories do not have a backing object in GCS.
// Set this flag to skip the GCS network call and only invalidate the local cache.
req.OnlyDeleteFromCache = true
}

if d.isBucketHierarchical() {
dirInode.Unlink()
// Delete the backing object. Unfortunately we have no way to precondition
// this on the directory being empty.
if err := d.bucket.DeleteObject(ctx, req); err != nil {
return fmt.Errorf("DeleteObject: %w", err)
}

if !d.IsTypeCacheDeprecated() {
d.cache.Erase(name)
}
return nil
}

Expand Down
57 changes: 57 additions & 0 deletions internal/fs/inode/hns_dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,3 +736,60 @@ func (t *HNSDirTest) TestReadEntriesInHierarchicalBucket() {
}
}
}

func (t *NonHNSDirTest) TestDeleteChildDir_TypeCacheDeprecated() {
testCases := []struct {
name string
isImplicitDir bool
onlyDeleteFromCache bool
}{
{
name: "ImplicitDir",
isImplicitDir: true,
onlyDeleteFromCache: true,
},
{
name: "ExplicitDir",
isImplicitDir: false,
onlyDeleteFromCache: false,
},
}

for _, tc := range testCases {
t.T().Run(tc.name, func(st *testing.T) {
// Enable type cache deprecation
config := &cfg.Config{
EnableTypeCacheDeprecation: true,
}
dirInode := NewDirInode(
dirInodeID,
NewDirName(NewRootName(""), dirInodeName),
fuseops.InodeAttributes{
Uid: uid,
Gid: gid,
Mode: dirMode,
},
true, // implicitDirs
false, // enableNonexistentTypeCache
typeCacheTTL,
&t.bucket,
&t.fixedTime,
&t.fixedTime,
config,
)
dirName := path.Join(dirInodeName, tc.name) + "/"
// Expectation: DeleteObject called with OnlyDeleteFromCache
expectedReq := &gcs.DeleteObjectRequest{
Name: dirName,
Generation: 0,
OnlyDeleteFromCache: tc.onlyDeleteFromCache,
}
t.mockBucket.On("DeleteObject", t.ctx, expectedReq).Return(nil)

err := dirInode.DeleteChildDir(t.ctx, tc.name, tc.isImplicitDir, nil)

assert.NoError(st, err)
t.mockBucket.AssertExpectations(st)
})
}
}
4 changes: 4 additions & 0 deletions internal/storage/caching/fast_stat_bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,10 @@ func (b *fastStatBucket) UpdateObject(
func (b *fastStatBucket) DeleteObject(
ctx context.Context,
req *gcs.DeleteObjectRequest) (err error) {
if req.OnlyDeleteFromCache {
b.addNegativeEntry(req.Name)
return nil
}
err = b.wrapped.DeleteObject(ctx, req)
// In case of successful delete, add a negative entry to the cache.
if err == nil {
Expand Down
16 changes: 16 additions & 0 deletions internal/storage/caching/fast_stat_bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,22 @@ func (t *DeleteObjectTest) WrappedSucceeds_AddsNegativeEntry() {
AssertEq(nil, err)
}

func (t *DeleteObjectTest) OnlyDeleteFromCache() {
const name = "taco"
req := &gcs.DeleteObjectRequest{
Name: name,
OnlyDeleteFromCache: true,
}
// Expect AddNegativeEntry call.
ExpectCall(t.cache, "AddNegativeEntry")(
name,
timeutil.TimeEq(t.clock.Now().Add(negativeCacheTTL)))

err := t.bucket.DeleteObject(context.TODO(), req)

AssertEq(nil, err)
}

func (t *StatObjectTest) TestShouldReturnFromCacheWhenEntryIsPresent() {
const name = "some-name"
folder := &gcs.Folder{
Expand Down
6 changes: 6 additions & 0 deletions internal/storage/gcs/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,12 @@ type DeleteObjectRequest struct {
// with the given name (and optionally generation), and its meta-generation
// is not equal to this value.
MetaGenerationPrecondition *int64

// OnlyDeleteFromCache controls whether the deletion is restricted to the local cache.
//
// If true, it updates the cache with a negative entry and skips the GCS call.
// If false, it proceeds with the standard GCS deletion and updates the cache on success.
OnlyDeleteFromCache bool
}

// MoveObjectRequest represents a request to move or rename an object.
Expand Down
Loading