io: add pager for append-only file abstraction #17084
Conversation
|
Force push to add missing header https://github.com/redpanda-data/redpanda/compare/01618eccda20f3ab86c46de5c627a13333940a28..3a4a0092702d1032887bfa3c2ef53ce2f0982fc6 |
|
ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46152#018e3af6-0292-4b5a-963c-df7cf4651e70 ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46902#018e8115-18bc-4a5e-a666-84e734b6968e ducktape was retried in https://buildkite.com/redpanda/redpanda/builds/46902#018e8115-18b9-418b-b263-d165016684ba |
src/v/io/pager.cc
Outdated
| page.signal_waiters(); | ||
| } else if (page.test_flag(page::flags::dirty)) { | ||
| vassert(page.test_flag(page::flags::write), "Expected write page"); | ||
| if (!page.test_flag(page::flags::queued)) { |
There was a problem hiding this comment.
What does it mean for handle_completion to be called but queued is not set? A write failure?
There was a problem hiding this comment.
the io-queue clears the queued flag before write-back happens. normally the write finishes and the completion handler is called and the dirty flag is cleared.
but while the write was happening, the page may have been re-queued because new data arrived and was written into the page, in which case the queued flag will be set. this is also the case here in which we don't want to incorrectly clear the dirty flag.
now that i'm thinking about it, it might be simpler to only invoke the completion handler for writes when the page wasn't re-queued.
There was a problem hiding this comment.
Thanks for explaining, that's really helpful. I agree, it may make for easier reasoning to have the io-queue only call the completion callback when the page isn't requeued. Does this result in a meaningful behavioral change for users? Is it that writers now have to wait longer in the face of concurrent writers, before getting a page as we're appending? (I think that makes sense, if so)
There was a problem hiding this comment.
Does this result in a meaningful behavioral change for users? Is it that writers now have to wait longer in the face of concurrent writers, before getting a page as we're appending? (I think that makes sense, if so)
For writers, there should be no change: new writes can always land into a dirty page as appends. That's true even if the page is queued for write-back.
For readers, there should be no change: dirty data in the cache is always readable.
There was a problem hiding this comment.
Ah makes sense. I saw us calling get_page() on the write path and assumed that that could lead us to waiting on the write path. But it makes sense that we could return a dirty page, in which case we aren't faulting (and not waiting!)
There was a problem hiding this comment.
yep. i think that there is a good argument for having a fast path for get_page that doesn't use co_await which would make it more clear.
Avoids terminal spam. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
A page is faulting when a read is occuring in response to a cache miss. A page is dirty when it contains data not written to persistent storage. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Will be used by the page cache to clear release memory. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
These routines are marked as noexcept, and each of the bitset operations throw if an invalid bit position is passed in. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Dirty and faulting are mutually exclusive. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
To avoid the pager needing to have knowledge about the internals of the io queue to clear the dirty bit, the io queue now only delivers write compeltions for writes that aren't requeued. Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
Signed-off-by: Noah Watkins <noahwatkins@gmail.com>
|
Force pushed https://github.com/redpanda-data/redpanda/compare/f308e8a02052f3625a4bfd81ba9871515128b3d0..ae5efe90381fb29d1c0ad284f174ac805db8c228 to fix Git commit messages and add a note about the pager into the io/README |
|
new failures in https://buildkite.com/redpanda/redpanda/builds/46902#018e8115-18bc-4a5e-a666-84e734b6968e: |
|
Failures unit test #16561 |
This PR doesn't touch any code outside |
Add io::pager which provides an append-only abstraction on top of the files using io::scheduler for the write-back process.
Backports Required
Release Notes