niyue commented on a change in pull request #11486:
URL: https://github.com/apache/arrow/pull/11486#discussion_r740561031



##########
File path: cpp/src/arrow/filesystem/mockfs.cc
##########
@@ -242,8 +242,32 @@ class MockFSInputStream : public io::BufferReader {
     return metadata_;
   }
 
+  Result<int64_t> DoReadAt(int64_t position, int64_t nbytes, void* out) 
override {
+    RecordReadIo(position, nbytes);
+    return io::BufferReader::DoReadAt(position, nbytes, out);
+  }
+
+  Result<std::shared_ptr<Buffer>> DoReadAt(int64_t position, int64_t nbytes) 
override {
+    RecordReadIo(position, nbytes);
+    return io::BufferReader::DoReadAt(position, nbytes);
+  }

Review comment:
       > While it's not merged yet, perhaps an approach like the one here will 
be easier? I don't think we need a full mock FileSystem, either - especially 
since the filesystem module isn't necessarily enabled. 
https://github.com/apache/arrow/pull/11535/files#diff-900c46995b5706697d6e4b010f610f1a1cf27d4d865afe48de0a800830ac676bL1708
   
   Got it. It seems the simplest way is to modify this 
`TrackedRandomAccessFile` to add not only the number of read IO times but also 
the offset/length, but this PR is not merged yet, do you think if I can copy 
the `TrackedRandomAccessFile` into my PR and add the necessary change, and we 
can do some merge later?




-- 
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]


Reply via email to