Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fuse position read bug #17558

Merged
merged 3 commits into from
Jun 19, 2023
Merged

Conversation

bzheng888
Copy link
Contributor

What changes are proposed in this pull request?

The position read will return -1 when position >= filesize, then the fuse will throw an error.
image
/tmp/alluxio-fuse/f1 is a 1mb file.

Why are the changes needed?

Fix the fuse position read bug.

Does this PR introduce any user facing changes?

Please list the user-facing changes introduced by your change, including

  1. change in user-facing APIs
  2. addition or removal of property keys
  3. webui

@bzheng888 bzheng888 closed this Jun 5, 2023
@bzheng888 bzheng888 reopened this Jun 5, 2023
Copy link
Contributor

@dbw9580 dbw9580 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this PR fixes the described issue. Can you describe how you debugged the error?

Comment on lines 168 to 169
if (totalRead == 0) {
return currentRead;
return totalRead;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original logic is correct. We must have reached here by breaking out of the above while loop, due to currentRead < 0. If totalRead == 0, then it means the very first read call return with EOF, therefore we return currentRead here to notify the caller.

@dbw9580
Copy link
Contributor

dbw9580 commented Jun 7, 2023

position read will return -1 when position > filesize

this is the expected behavior: you cannot read beyond the end of the file.

On the other hand, read should return 0 if position == filesize and length == 0

@bzheng888
Copy link
Contributor Author

bzheng888 commented Jun 7, 2023

position read will return -1 when position > filesize

this is the expected behavior: you cannot read beyond the end of the file.

On the other hand, read should return 0 if position == filesize and length == 0

Thanks for reply, this fix may not work, but the position read problem exist any way, can you help to figure out what's going on? I did not follow up this issue in detail.

@alluxio-bot
Copy link
Contributor

Automated checks report:

  • PR title follows the conventions: PASS
  • Commits associated with Github account: FAIL
    • It looks like your commits can't be linked to a valid Github account.
      Your commits are made with the email bingbzheng@tencent.com, which does not allow your contribution to be tracked by Github.
      See this link for possible reasons this might be happening.
      To change the author email address that your most recent commit was made under, you can run:
      git -c user.name="Name" -c user.email="Email" commit --amend --reset-author
      See this answer for more details about how to change commit email addresses.
      Once the author email address has been updated, update the pull request by running:
      git push --force https://github.com/bzheng888/alluxio.git dora-fix-standalon-fuse

Some checks failed. Please fix the reported issues and reply 'alluxio-bot, check this please' to re-run checks.

@bzheng888
Copy link
Contributor Author

The operations:
image
Debug info.
image
The read will exceed the file length actually, @dbw9580 @LuQQiu FYI

@dbw9580 dbw9580 self-requested a review June 9, 2023 04:02
@maobaolong
Copy link
Contributor

@bzheng888 I guess you may use a unknown email for this PR

image

@bzheng888
Copy link
Contributor Author

@dbw9580 @LuQQiu Can you help to look this problem, thanks?

@LuQQiu
Copy link
Contributor

LuQQiu commented Jun 19, 2023

@bzheng888 looks like the PR-EMAIL failed? Email not linked to a valid Github account

@LuQQiu
Copy link
Contributor

LuQQiu commented Jun 19, 2023

@bzheng888 can you help create a github issue and link to this PR?

@LuQQiu
Copy link
Contributor

LuQQiu commented Jun 19, 2023

alluxio-bot, merge this please

@alluxio-bot
Copy link
Contributor

merge failed:
Merge refused because pull request does not have label start with type-

@LuQQiu LuQQiu added the type-bug This issue is about a bug label Jun 19, 2023
@LuQQiu
Copy link
Contributor

LuQQiu commented Jun 19, 2023

alluxio-bot, merge this please

@alluxio-bot alluxio-bot merged commit 54aee26 into Alluxio:main Jun 19, 2023
@bzheng888
Copy link
Contributor Author

@bzheng888 can you help create a github issue and link to this PR?

Ok

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug This issue is about a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants