Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better paging when random reads go backwards #12357

Merged
merged 8 commits into from
Jun 9, 2023
Merged

Better paging when random reads go backwards #12357

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3b13621
Better paging when random reads go backwards
romseygeek Jun 8, 2023
7e8f54a
Extend to int, short, long
romseygeek Jun 8, 2023
1365b68
fix comment
romseygeek Jun 8, 2023
ee40aed
deef
romseygeek Jun 9, 2023
be2341d
changes
romseygeek Jun 9, 2023
8d196a0
rename
romseygeek Jun 9, 2023
66811c8
Merge remote-tracking branch 'origin/main' into backwards-random-reads
romseygeek Jun 9, 2023
a6942fe
deef
romseygeek Jun 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions lucene/core/src/java/org/apache/lucene/store/BufferedIndexInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public abstract class BufferedIndexInput extends IndexInput implements RandomAcc
/** A buffer size for merges set to {@value #MERGE_BUFFER_SIZE}. */
public static final int MERGE_BUFFER_SIZE = 4096;

private int bufferSize = BUFFER_SIZE;
private final int bufferSize;
romseygeek marked this conversation as resolved.
Show resolved Hide resolved

private ByteBuffer buffer = EMPTY_BYTEBUFFER;

Expand Down Expand Up @@ -163,11 +163,11 @@ public final long readLong() throws IOException {
public final byte readByte(long pos) throws IOException {
long index = pos - bufferStart;
if (index < 0 || index >= buffer.limit()) {
bufferStart = pos;
bufferStart = index < 0 ? Math.min(pos, Math.max(0, bufferStart - bufferSize)) : pos;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment explaining that this is trying to avoid backward reading being an adversarial case?

buffer.limit(0); // trigger refill() on read
seekInternal(pos);
seekInternal(bufferStart);
refill();
index = 0;
index = pos - bufferStart;
}
return buffer.get((int) index);
}
Expand Down
18 changes: 15 additions & 3 deletions lucene/core/src/test/org/apache/lucene/store/TestBufferedIndexInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,16 @@ public void testEOF() throws Exception {
});
}

// Test that when reading backwards, we page backwards rather than refilling
// on every call
public void testBackwardsReads() throws IOException {
MyBufferedIndexInput input = new MyBufferedIndexInput(1024 * 8);
for (int i = 2048; i > 0; i -= random().nextInt(16)) {
assertEquals(byten(i), input.readByte(i));
}
assertEquals(3, input.readCount);
}

// byten emulates a file - byten(n) returns the n'th byte in that file.
// MyBufferedIndexInput reads this "file".
private static byte byten(long n) {
Expand All @@ -150,7 +160,8 @@ private static byte byten(long n) {

private static class MyBufferedIndexInput extends BufferedIndexInput {
private long pos;
private long len;
private final long len;
private long readCount = 0;

public MyBufferedIndexInput(long len) {
super("MyBufferedIndexInput(len=" + len + ")", BufferedIndexInput.BUFFER_SIZE);
Expand All @@ -164,14 +175,15 @@ public MyBufferedIndexInput() {
}

@Override
protected void readInternal(ByteBuffer b) throws IOException {
protected void readInternal(ByteBuffer b) {
readCount++;
while (b.hasRemaining()) {
b.put(byten(pos++));
}
}

@Override
protected void seekInternal(long pos) throws IOException {
protected void seekInternal(long pos) {
this.pos = pos;
}

Expand Down