Hi, On Tue, 12 Aug 2025 at 22:30, Thomas Munro <thomas.mu...@gmail.com> wrote: > > On Tue, Aug 12, 2025 at 11:22 PM Nazir Bilal Yavuz <byavu...@gmail.com> wrote: > > Unfortunately this doesn't work. We need to handle backwards I/O > > combining in the StartReadBuffersImpl() function too as buffer indexes > > won't have correct blocknums. Also, I think buffer forwarding of split > > backwards I/O should be handled in a couple of places. > > Perhaps there could be a flag pending_read_backwards that can only > become set with pending_read_nblocks goes from 1 to 2, and then a new > flag stream->ios[x].backwards (in struct InProgressIO) that is set in > read_stream_start_pending_read(). Then immediately after > WaitReadBuffers(), we reverse the buffers it returned in place if that > flag was set. Oh, I see, you were imagining a flag > READ_BUFFERS_REVERSE that tells WaitReadBuffers() to do that > internally. Hmm. Either way I don't think you need to consider the > forwarded buffers because they will be reversed during a later call > that includes them in *nblocks (output value), no?
I think the problem is that we are not sure whether we will run WaitReadBuffers() or not. Let's say that we will process blocknums 25, 24, 23, 22, 21 and 20 so we combined these IOs. We set the pending_read_backwards flag and sent this IO operation to the StartReadBuffers(). Let's consider that 22 and 20 are cache hits and the rest are cache misses. In that case, starting processing buffers (inside StartReadBuffers()) from 20 will fail because we will try to return that immediately since this is a first buffer and it is cache hit. I think something like this, we will pass the pending_read_backwards to the StartReadBuffers() and it will start to process blocknums from backwards because of the pending_read_backwards being true. So, buffer[0] -> 25 ... buffer[2] -> 23 and we will stop there because 22 is a cache hit. Now, we will reverse these buffers so that buffer[0] -> 23 ... buffer[2] -> 25, and then send this IO operation to the WaitReadBuffers() and reverse these buffers again after WaitReadBuffers(). The problem with that approach is that we need to forward 22, 21 and 20 and pending_read_blocknum shouldn't change because we are still at 20, processed buffers don't affect pending_read_blocknum. And we need to preserve pending_read_backwards until we process all forwarded buffers, otherwise we may try to combine forward (pending_read_blocknum is 20 and the let's say next blocknum from read_stream_get_block() is 21, we shouldn't do IO combining in that case). -- Regards, Nazir Bilal Yavuz Microsoft
