Skip to content

Commit

Permalink
Optimize Rename function logic to reduce the number of REST API calls. (
Browse files Browse the repository at this point in the history
#1459)

* remove extra AREST API call in renameFile
  • Loading branch information
syeleti-msft authored Nov 5, 2024
1 parent 0fb27e4 commit 696ef55
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 23 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ blobfuse2
adlsgen1fuse
venv/
*.backup
__debug_bin
__debug_bin*
.env
*.prof
cpplite/
Expand All @@ -21,3 +21,4 @@ lint.log
azure-storage-fuse
bfusemon
test/scripts/dirIterate.go
component/azstorage/logfile.txt
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
## 2.4.0 (Unreleased)
**Bug Fixes**
- [#1426](https://github.com/Azure/azure-storage-fuse/issues/1426) Read panic in block-cache due to boundary conditions.

- Rename file was calling an additional getProperties call.

**Features**
- Added support for custom component via go plugin.

Expand Down
1 change: 0 additions & 1 deletion component/attr_cache/attr_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,6 @@ func (ac *AttrCache) RenameFile(options internal.RenameFileOptions) error {
ac.cacheLock.RLock()
defer ac.cacheLock.RUnlock()

// TODO: Can we just copy over the attributes from the source to the destination so we don't have to invalidate?
ac.deletePath(options.Src, time.Now())
ac.invalidatePath(options.Dst)
}
Expand Down
47 changes: 27 additions & 20 deletions component/azstorage/block_blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,33 +313,27 @@ func (bb *BlockBlob) DeleteDirectory(name string) (err error) {
}

// RenameFile : Rename the file
// Source file must exist in storage account before calling this method.
func (bb *BlockBlob) RenameFile(source string, target string) error {
log.Trace("BlockBlob::RenameFile : %s -> %s", source, target)

blobClient := bb.Container.NewBlockBlobClient(filepath.Join(bb.Config.prefixPath, source))
newBlobClient := bb.Container.NewBlockBlobClient(filepath.Join(bb.Config.prefixPath, target))

_, err := blobClient.GetProperties(context.Background(), &blob.GetPropertiesOptions{
CPKInfo: bb.blobCPKOpt,
})
if err != nil {
serr := storeBlobErrToErr(err)
if serr == ErrFileNotFound {
log.Err("BlockBlob::RenameFile : %s does not exist", source)
return syscall.ENOENT
} else {
log.Err("BlockBlob::RenameFile : Failed to get blob properties for %s [%s]", source, err.Error())
return err
}
}

// not specifying source blob metadata, since passing empty metadata headers copies
// the source blob metadata to destination blob
startCopy, err := newBlobClient.StartCopyFromURL(context.Background(), blobClient.URL(), &blob.StartCopyFromURLOptions{
Tier: bb.Config.defaultTier,
})

if err != nil {
serr := storeBlobErrToErr(err)
if serr == ErrFileNotFound {
//Ideally this case doesn't hit as we are checking for the existence of src
//before making the call for RenameFile
log.Err("BlockBlob::RenameFile : Src Blob doesn't Exist %s [%s]", source, err.Error())
return syscall.ENOENT
}
log.Err("BlockBlob::RenameFile : Failed to start copy of file %s [%s]", source, err.Error())
return err
}
Expand Down Expand Up @@ -404,13 +398,26 @@ func (bb *BlockBlob) RenameDirectory(source string, target string) error {
}
}

err := bb.RenameFile(source, target)
// check if the marker blob for source directory does not exist but
// blobs were present in it, which were renamed earlier
if err == syscall.ENOENT && srcDirPresent {
err = nil
// To rename source marker blob check its properties before calling rename on it.
blobClient := bb.Container.NewBlockBlobClient(filepath.Join(bb.Config.prefixPath, source))
_, err := blobClient.GetProperties(context.Background(), &blob.GetPropertiesOptions{
CPKInfo: bb.blobCPKOpt,
})
if err != nil {
serr := storeBlobErrToErr(err)
if serr == ErrFileNotFound { //marker blob doesn't exist for the directory
if srcDirPresent { //Some files exist inside the directory
return nil
}
log.Err("BlockBlob::RenameDirectory : %s marker blob does not exist and Src Directory doesn't Exist", source)
return syscall.ENOENT
} else {
log.Err("BlockBlob::RenameDirectory : Failed to get source directory marker blob properties for %s [%s]", source, err.Error())
return err
}
}
return err

return bb.RenameFile(source, target)
}

func (bb *BlockBlob) getAttrUsingRest(name string) (attr *internal.ObjAttr, err error) {
Expand Down

0 comments on commit 696ef55

Please sign in to comment.