thecoop commented on code in PR #16619:
URL: https://github.com/apache/lucene/pull/16619#discussion_r3914231987
##########
lucene/misc/src/test/org/apache/lucene/misc/store/TestDirectIODirectory.java:
##########
@@ -218,6 +219,245 @@ public void testUseDirectIODefaults() throws Exception {
}
}
+ private static byte[] writeRandomFile(Directory dir, String name, int size)
throws IOException {
+ byte[] bytes = new byte[size];
+ random().nextBytes(bytes);
+ try (IndexOutput o = dir.createOutput(name, IOContext.DEFAULT)) {
+ o.writeBytes(bytes, 0, size);
+ }
+ return bytes;
+ }
+
+ public void testSliceDefersIOAtEveryOffset() throws Exception {
+ Path path = createTempDir("testSliceDefersIOAtEveryOffset");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = 4 * blockSize;
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ final long[] offsets = {
+ 0, 1, 7, 96, blockSize, blockSize + 188, 2L * blockSize, 3L *
blockSize - 4
+ };
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ for (long offset : offsets) {
+ IndexInput slice = in.slice("slice@" + offset, offset, fileSize -
offset);
+ DirectIODirectory.DirectIOIndexInput directSlice =
+ (DirectIODirectory.DirectIOIndexInput) slice;
+ assertTrue(
+ "slice at offset " + offset + " did not defer its first fill",
+ directSlice.isDeferred());
+ assertEquals(0L, slice.getFilePointer());
+ assertEquals(
+ "first byte of slice at offset " + offset, bytes[(int) offset],
slice.readByte());
+ assertFalse(directSlice.isDeferred());
+ }
+ }
+ }
+ }
+
+ public void testSliceCorrectnessMatrix() throws Exception {
+ Path path = createTempDir("testSliceCorrectnessMatrix");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = 4 * blockSize;
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ final long[] offsets = {
+ 0, 1, 7, 96, blockSize, blockSize + 188, 2L * blockSize, 3L *
blockSize - 4
+ };
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ for (long offset : offsets) {
+ final long[] lengths = {1, 4, blockSize - 7, blockSize, blockSize +
3, fileSize - offset};
+ for (long length : lengths) {
+ if (length <= 0 || length > fileSize - offset) {
+ continue;
+ }
+ final int len = (int) length;
+ // full read-through of the slice equals the same range of the
parent
+ IndexInput slice = in.slice("s", offset, length);
+ byte[] actual = new byte[len];
+ slice.readBytes(actual, 0, len);
+ assertArrayEquals(
+ "offset " + offset + " length " + length,
+ ArrayUtil.copyOfSubArray(bytes, (int) offset, (int) offset +
len),
+ actual);
+ if (len >= 2) {
+ // slice-of-slice at a non-zero inner offset
+ IndexInput inner = in.slice("s", offset, length).slice("ss", 1,
length - 1);
+ byte[] innerBytes = new byte[len - 1];
+ inner.readBytes(innerBytes, 0, len - 1);
+ assertArrayEquals(
+ "inner slice at offset " + offset + " length " + length,
+ ArrayUtil.copyOfSubArray(bytes, (int) offset + 1, (int)
offset + len),
+ innerBytes);
+ // clone-after-partial-read starts at the parent's position
+ IndexInput partial = in.slice("s", offset, length);
+ partial.readBytes(new byte[len / 2], 0, len / 2);
+ IndexInput clone = partial.clone();
+ assertEquals(len / 2, clone.getFilePointer());
+ byte[] rest = new byte[len - len / 2];
+ clone.readBytes(rest, 0, rest.length);
+ assertArrayEquals(
+ "clone at offset " + offset + " length " + length,
+ ArrayUtil.copyOfSubArray(bytes, (int) offset + len / 2,
(int) offset + len),
+ rest);
+ }
+ }
+ }
+ }
+ }
+ }
+
+ public void testSeekResolvesFreshParkedSlice() throws Exception {
+ Path path = createTempDir("testSeekResolvesFreshParkedSlice");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = DirectIODirectory.DEFAULT_MERGE_BUFFER_SIZE + 3 *
blockSize;
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ final long[] offsets = {1, 96, blockSize + 188};
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ for (long offset : offsets) {
+ final long length =
+ Math.min(DirectIODirectory.DEFAULT_MERGE_BUFFER_SIZE, fileSize -
offset);
+ IndexInput slice = in.slice("s", offset, length);
+ // seek() on a slice that has not been read yet must position it
correctly: the byte
+ // read is the slice's own last byte, not one relative to the wrong
base.
+ slice.seek(length - 1);
+ assertEquals("offset " + offset, bytes[(int) (offset + length - 1)],
slice.readByte());
+ assertEquals(length, slice.getFilePointer());
+ }
+ }
+ }
+ }
+
+ public void testCloneOfPartiallyReadUnalignedSliceResolvesToItsPosition()
throws Exception {
+ // clone() positions a fresh clone at the parent's file pointer through
seekInternal, and is
+ // the only caller that reaches it with a pending start. If seekInternal
did not drop that
+ // pending start first, a clone taken after a partial read of a short
unaligned slice would
+ // read the wrong bytes or throw EOFException.
+ Path path = createTempDir("testCloneOfPartiallyReadSlice");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = 4 * blockSize;
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ final long[] offsets = {1, 96, blockSize + 188, 3L * blockSize - 4};
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ for (long offset : offsets) {
+ final long length = Math.min(blockSize - 7, fileSize - offset);
+ IndexInput partial = in.slice("s", offset, length);
+ final int half = (int) length / 2;
+ partial.readBytes(new byte[half], 0, half);
+ IndexInput clone = partial.clone();
+ assertEquals("offset " + offset, half, clone.getFilePointer());
+ byte[] rest = new byte[(int) length - half];
+ clone.readBytes(rest, 0, rest.length);
+ assertArrayEquals(
+ "offset " + offset,
+ ArrayUtil.copyOfSubArray(bytes, (int) offset + half, (int)
offset + (int) length),
+ rest);
+ }
+ }
+ }
+ }
+
+ public void testFirstFillEofSemanticsMatchEagerFill() throws Exception {
+ Path path = createTempDir("testFirstFillEofSemantics");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = 4 * blockSize;
+ // the oracle below relies on the whole file fitting in one buffer window
+ assumeTrue(
+ "file must fit in one buffer window",
+ fileSize <= DirectIODirectory.DEFAULT_MERGE_BUFFER_SIZE);
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ final long[] offsets = {0, 96, blockSize, fileSize - 5};
+ final long[] lengths = {0, 1, 4, blockSize};
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ for (long offset : offsets) {
+ for (long l : lengths) {
+ final long length = Math.min(l, fileSize - offset);
+ final int[] readLens = {1, 4, (int) length + 2, blockSize + 32,
fileSize};
+ for (int readLen : readLens) {
+ IndexInput slice = in.slice("s", offset, length);
+ final String cell = "offset=" + offset + " length=" + length + "
readLen=" + readLen;
+ // Pins today's behaviour, quirk included: bytes inside the
buffered window are
+ // served even past the slice's own length, and only a read that
runs off the end
+ // of the window throws, as EOFException. If the first fill ever
checked EOF against
+ // the caller's read length instead, the in-window cells below
would fail.
+ if (readLen <= fileSize - offset) {
+ byte[] dst = new byte[readLen];
+ slice.readBytes(dst, 0, readLen);
+ assertArrayEquals(
+ cell,
+ ArrayUtil.copyOfSubArray(bytes, (int) offset, (int) offset
+ readLen),
+ dst);
+ } else {
+ expectThrows(
+ EOFException.class, cell, () -> slice.readBytes(new
byte[readLen], 0, readLen));
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ public void testZeroLengthSliceAtFileEndThrowsEOF() throws Exception {
+ Path path = createTempDir("testZeroLengthSliceAtFileEnd");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ // one file whose length is a multiple of the block size (pending offset
0) and one whose
+ // length is not
+ final int[] fileSizes = {2 * blockSize, 2 * blockSize + 300};
+ try (Directory dir = getDirectory(path)) {
+ for (int fileSize : fileSizes) {
+ String name = "out" + fileSize;
+ writeRandomFile(dir, name, fileSize);
+ try (IndexInput in = dir.openInput(name, IOContext.DEFAULT)) {
+ IndexInput slice = in.slice("end", in.length(), 0);
+ // the exception type matters: a first fill that finds nothing to
read must fall
+ // through to the caller's own read and throw EOFException, not a
+ // BufferUnderflowException from an empty buffer
+ expectThrows(EOFException.class, "file size " + fileSize,
slice::readByte);
+ }
+ }
+ }
+ }
+
+ public void testSeekBeyondSliceLengthMatchesEagerBehaviour() throws
Exception {
+ Path path = createTempDir("testSeekBeyondSliceLength");
+ final int blockSize =
Math.toIntExact(Files.getFileStore(path).getBlockSize());
+ final int fileSize = 4 * blockSize;
+ assumeTrue(
+ "file must fit in one buffer window",
+ fileSize <= DirectIODirectory.DEFAULT_MERGE_BUFFER_SIZE);
+ try (Directory dir = getDirectory(path)) {
+ byte[] bytes = writeRandomFile(dir, "out", fileSize);
+ try (IndexInput in = dir.openInput("out", IOContext.DEFAULT)) {
+ // seek targets inside the block the eager code buffered at
construction: today the
+ // seek silently repositions and the read serves the byte beyond the
slice's end. A
+ // fresh lazy slice must do the same rather than throw EOFException,
and
+ // getFilePointer() must return normally afterwards (assertions are
enabled)
+ final long[] offsets = {blockSize, 700};
+ final long[] seekTargets = {10, blockSize + 50L};
+ for (long offset : offsets) {
+ for (long target : seekTargets) {
+ IndexInput slice = in.slice("s", offset, 4);
+ slice.seek(target);
+ assertEquals(
+ "offset " + offset + " target " + target,
+ bytes[(int) (offset + target)],
+ slice.readByte());
+ assertEquals(target + 1, slice.getFilePointer());
+ }
+ }
+ // a target outside the buffered window: the refill checks EOF against
the slice's own
+ // end and throws, as today; getFilePointer() must still return
normally afterwards
+ IndexInput far = in.slice("s", 700, 4);
+ expectThrows(
+ EOFException.class, () ->
far.seek(DirectIODirectory.DEFAULT_MERGE_BUFFER_SIZE + 100L));
+ assertTrue(far.getFilePointer() >= 0);
Review Comment:
Using hamcrest - `assertThat(far.getFilePointer(), greaterThanOrEqualTo(0))`
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
