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

ide: mark PRD as exhausted even if on buffer boundary #633

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

eric135
Copy link
Contributor

@eric135 eric135 commented Jan 8, 2025

The PRD table can become exhausted early without the exhaustion flag being set because the buffer is not fully exhausted, leaving the disk waiting for remaining data that will never come. Additionally, the exhaustion logic for reads needs to also be copied for writes.

@mattkur
Copy link
Contributor

mattkur commented Jan 8, 2025

Thanks for submitting this. How did you find this? (was this a fuzzer-found bug)?

@eric135 eric135 marked this pull request as ready for review January 8, 2025 23:01
@eric135 eric135 requested review from a team as code owners January 8, 2025 23:01
@eric135
Copy link
Contributor Author

eric135 commented Jan 8, 2025

Thanks for submitting this. How did you find this? (was this a fuzzer-found bug)?

This is a resubmit on a fix attempted on the old repo months ago. There were still some testing issues with that fix, but with recent changes to vmm-tests, I am re-running to see if they are now resolved.

@mattkur mattkur changed the title ide: mark PRD as exhausted eeven if on buffer boundary ide: mark PRD as exhausted even if on buffer boundary Jan 8, 2025
@mattkur
Copy link
Contributor

mattkur commented Jan 8, 2025

Thanks for submitting this. How did you find this? (was this a fuzzer-found bug)?

This is a resubmit on a fix attempted on the old repo months ago. There were still some testing issues with that fix, but with recent changes to vmm-tests, I am re-running to see if they are now resolved.

Got it. Can you do two things?

  1. please add a new test case for this (or let us know why you think that's not viable / not worth it), and
  2. elaborate on the problem & how this fixes it in the description

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants