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

feat: Reduce stat operation if we are reading all #5146

Merged
merged 6 commits into from
Sep 28, 2024
Merged

feat: Reduce stat operation if we are reading all #5146

merged 6 commits into from
Sep 28, 2024

Conversation

Xuanwo
Copy link
Member

@Xuanwo Xuanwo commented Sep 28, 2024

Which issue does this PR close?

None

Rationale for this change

We used to perform a stat operation to make sure we have the read size known. But in the read_all cases, we don't need to care about that.

What changes are included in this PR?

This PR refactor the internal range parse logic so that we can read all without fetching it's metadata first.

Are there any user-facing changes?

@Xuanwo Xuanwo changed the title feat: Reduce stat operation if we are reading to end feat: Reduce stat operation if we are reading all Sep 28, 2024
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
Signed-off-by: Xuanwo <[email protected]>
@Xuanwo Xuanwo requested review from G-XD and dqhl76 September 28, 2024 06:26
Copy link
Member

@dqhl76 dqhl76 left a comment

Choose a reason for hiding this comment

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

Thanks for your work! The code LGTM. However, I am not sure if the failed test related with this PR because it has so many lines of log information, it makes me hard to find the specific failed test 😢.

@Xuanwo
Copy link
Member Author

Xuanwo commented Sep 28, 2024

Thanks a lot for your review!

Thanks for your work! The code LGTM. However, I am not sure if the failed test related with this PR because it has so many lines of log information, it makes me hard to find the specific failed test 😢.

  • Aliyun Drive and Google Drive are unstable; this is a known issue.
  • I will try to make the logs less verbose.

@Xuanwo Xuanwo merged commit 860f5b2 into main Sep 28, 2024
265 of 267 checks passed
@Xuanwo Xuanwo deleted the reduce-head branch September 28, 2024 06:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants