Reduce the number of rest calls while doing Rename Operation #1614
Conversation
syeleti-msft
requested review from
vibhansa-msft,
souravgupta-msft,
ashruti-msft and
jainakanksha-msft
as code owners
| err = suite.attrCache.RenameFile(options) | ||
| suite.assert.Nil(err) | ||
| assertDeleted(suite, src) | ||
| assertUntouched(suite, dst) |
There was a problem hiding this comment.
Shouldn't destination attribute definition be updated by source attribute information?
Same for L.No 700.
Untouched is more like nothing is updated in cache corresponding to the object.
There was a problem hiding this comment.
Shouldn't destination attribute definition be updated by source attribute information?
We are updating the src attribute
There was a problem hiding this comment.
In attr_cache, we make sure things are copied to destination, so destination attribute should be updated right?
component/attr_cache/attr_cache.go
Outdated
| // Dst blob may not exist before | ||
| dstCacheEntry.attrFlag.Set(AttrFlagExists) | ||
| // Refresh the cache | ||
| dstCacheEntry.cachedAt = time.Now() |
There was a problem hiding this comment.
Like invalidatePath you can also create a function in attribute cache to updatePath and do this assignment there.
I understand this might be too many parameters but can be reused operation by multiple other objects.
component/azstorage/block_blob.go
Outdated
| @@ -340,6 +344,8 @@ func (bb *BlockBlob) RenameFile(source string, target string) error { | |||
| return err | |||
| } | |||
|
|
|||
| var dstLMT *time.Time = startCopy.LastModified | |||
There was a problem hiding this comment.
Why we are picking up LMT from startCopy.
It can be from srcAttr also if needed.
But bigger question is why did we need it at this point.
Only after copying the blob we would need to update the LMT over destination object right?
There was a problem hiding this comment.
Why we are picking up LMT from startCopy.
we picking it up from the startCopy because many times the Copyblob API copies the file immediately(i.e., you get the x-ms-copystatus=success in the response). in this case we can use LMT that was returned by the copyBlob to update our attribute
Only after copying the blob we would need to update the LMT over destination object right?
I think the variable name is culprit here for misunderstanding, I will update it
✅ What
Currently we invalidate the cache item of destination (to remove the older reference of detination in our cache) after the rename operation completes[, this can be identified in the final call of the following flow:
Rename on FNS:
API calls: There are 5 REST api calls assosiated with this operation.
Rename on HNS:
API Calls: There are 4 REST api calls assosiated with this operation.
goal is to serve the last call(invalidatePath) for FNS&HNS in the above REST calls from the attribute cache.
🤔 Why
The idea is to simply copy source Attributes in the destination file object when the rename is success.
The only thing that modified in the destination was the last modified time, it will be modified according to the response of REST call for rename in AzStorage Component.
👩🔬 How to validate if applicable
Do rename operation with "mv" and see that the Final GetAttr on Dst after rename is success will be served from the cache.