bug in block cache open call #1580
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…cate the file to zero. This is incorrect behaviour. We don't see it in the normal scenario as write-back cache is on by default. Hence all the open calls with O_WRONLY will be redirected O_RDWR. To simulate this turn of the write-back cache and then open file in O_WRONLY.
syeleti-msft
requested review from
vibhansa-msft,
souravgupta-msft,
ashruti-msft and
jainakanksha-msft
as code owners
| @@ -397,7 +397,7 @@ func (bc *BlockCache) OpenFile(options internal.OpenFileOptions) (*handlemap.Han | |||
| log.Debug("BlockCache::OpenFile : Size of file handle.Size %v", handle.Size) | |||
| bc.prepareHandleForBlockCache(handle) | |||
|
|
|||
| if options.Flags&os.O_TRUNC != 0 || (options.Flags&os.O_WRONLY != 0 && options.Flags&os.O_APPEND == 0) { | |||
| if options.Flags&os.O_TRUNC != 0 { | |||
There was a problem hiding this comment.
We shall add a UT case around this validating that truncate clears the file contents while wronly does not.
There was a problem hiding this comment.
Added, Please review it
|
@syeleti-msft Can you please add the steps to validate this flow? |
|
@syeleti-msft Also tag the related issue with this PR. |
syeleti-msft
force-pushed
the
syeleti/bc-fix
branch
from
7e2b180 to
3c5317e
Compare
Added in the description part of the PR |
jainakanksha-msft
approved these changes
Dec 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
✅ What
Current implementation of open file when opened in O_WRONLY mode truncates the file to zero.
While this mode - means we should have opened the file in read write mode, not truncate it.
So, This is incorrect behaviour.
👩🔬 How to validate
We don't see it in the normal scenario as write-back cache is on by default. All the open calls with O_WRONLY will be redirected to O_RDWR mode.
So, To simulate this issue, turn off the write-back cache while mounting with --disable-writeback-cache=true and then open file in O_WRONLY mode, you see that the file is truncated to size zero.