zacharymorn commented on a change in pull request #2258:
URL: https://github.com/apache/lucene-solr/pull/2258#discussion_r568294041
##########
File path:
lucene/misc/src/java/org/apache/lucene/misc/store/DirectIODirectory.java
##########
@@ -381,17 +377,18 @@ public long length() {
@Override
public byte readByte() throws IOException {
if (!buffer.hasRemaining()) {
- refill();
+ refill(1);
}
+
return buffer.get();
}
- private void refill() throws IOException {
+ private void refill(int byteToRead) throws IOException {
filePos += buffer.capacity();
// BaseDirectoryTestCase#testSeekPastEOF test for consecutive read past
EOF,
// hence throwing EOFException early to maintain buffer state (position
in particular)
- if (filePos > channel.size()) {
+ if (filePos > channel.size() || (channel.size() - filePos < byteToRead))
{
Review comment:
If I understand your comment correctly, your concern is about the
consistency of directory's internal state after EOF is raised right? I think
DirectIODirectory already handles that actually (by manipulating `filePos`, but
not `channel.position` per se), and I have added some more tests to confirm
that to be the case in the latest commit.
Please note that for the additional tests, I was originally adding them into
`BaseDirectoryTestCase#testSeekPastEOF`, but that would fail some existing
tests for other directory implementations, as read immediately after seek past
EOF doesn't raise EOFException for them:
* TestHardLinkCopyDirectoryWrapper
* TestMmapDirectory
* TestByteBuffersDirectory
* TestMultiMMap
However, according to java doc here
https://github.com/apache/lucene-solr/blob/15aaec60d9bfa96f2837c38b7ca83e2c87c66d8d/lucene/core/src/java/org/apache/lucene/store/IndexInput.java#L66-L73,
this seems to be an unspecified state in general.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
