lidavidm commented on pull request #11486: URL: https://github.com/apache/arrow/pull/11486#issuecomment-954218790
Broadly I'm in agreement. I like the approach here but it does seem we will want to 'fuse' some of the layers to get the best implementation. The duplication with the asynchronous path is one such issue. I agree we will want a unit test to ensure the bytes read is as expected. Additionally, another candidate for a follow-up item is to include the I/O coalescer so that we don't suffer on remote filesystems. (This could also be folded into ARROW-14429; another thing we could fold into there is to make ReadRangeCache not hold on to memory for the use case of linearly scanning a file.) Also, I think IoRecordedRandomAccessFile should be moved into reader.cc - it has fairly specific use and I don't see a reason to expose it more broadly, especially since we aren't subjecting it to unit tests separately. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
