Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
c96f5b5
ESQL: Split large pages on load sometimes
nik9000 Jul 9, 2025
50f632f
Update docs/changelog/131053.yaml
nik9000 Jul 10, 2025
f24df78
[CI] Auto commit changes from spotless
Jul 10, 2025
735ea24
Iypdate
nik9000 Jul 11, 2025
b1aebd5
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 11, 2025
fcfb421
update
nik9000 Jul 11, 2025
b58082a
Fix size and build once
nik9000 Jul 11, 2025
5186b88
Merge remote-tracking branch 'nik9000/esql_danger_zone_2' into esql_d…
nik9000 Jul 11, 2025
60b3314
move check
nik9000 Jul 14, 2025
ae089f8
Rename
nik9000 Jul 14, 2025
c82eaa0
Better tests and real offset
nik9000 Jul 15, 2025
c2f656d
Rework page_size
nik9000 Jul 15, 2025
00e6c3b
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 15, 2025
dd3e9c4
[CI] Auto commit changes from spotless
Jul 15, 2025
eaabdee
Merge cleanup
nik9000 Jul 15, 2025
4fed32c
Merge remote-tracking branch 'nik9000/esql_danger_zone_2' into esql_d…
nik9000 Jul 15, 2025
9641518
Explain
nik9000 Jul 15, 2025
36c35a4
Fix
nik9000 Jul 15, 2025
74114a8
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 16, 2025
68d45d9
Check build
nik9000 Jul 16, 2025
53b4bdc
Fix test
nik9000 Jul 16, 2025
fb0249a
Wrong spot
nik9000 Jul 16, 2025
1dcb020
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 16, 2025
a81d427
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 17, 2025
77ff429
Fixup serverless
nik9000 Jul 17, 2025
9a7613f
Merge remote-tracking branch 'nik9000/esql_danger_zone_2' into esql_d…
nik9000 Jul 17, 2025
aa7c608
Merge branch 'main' into esql_danger_zone_2
nik9000 Jul 17, 2025
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
5 changes: 2 additions & 3 deletions benchmarks/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,11 +152,10 @@ exit
Grab the async profiler from https://github.com/jvm-profiling-tools/async-profiler
and run `prof async` like so:
```
gradlew -p benchmarks/ run --args 'LongKeyedBucketOrdsBenchmark.multiBucket -prof "async:libPath=/home/nik9000/Downloads/async-profiler-3.0-29ee888-linux-x64/lib/libasyncProfiler.so;dir=/tmp/prof;output=flamegraph"'
gradlew -p benchmarks/ run --args 'LongKeyedBucketOrdsBenchmark.multiBucket -prof "async:libPath=/home/nik9000/Downloads/async-profiler-4.0-linux-x64/lib/libasyncProfiler.so;dir=/tmp/prof;output=flamegraph"'
```

Note: As of January 2025 the latest release of async profiler doesn't work
with our JDK but the nightly is fine.
Note: As of July 2025 the 4.0 release of the async profiler works well.

If you are on Mac, this'll warn you that you downloaded the shared library from
the internet. You'll need to go to settings and allow it to run.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.NumericUtils;
import org.elasticsearch.common.breaker.NoopCircuitBreaker;
import org.elasticsearch.common.logging.LogConfigurator;
import org.elasticsearch.common.lucene.Lucene;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
Expand Down Expand Up @@ -85,10 +86,13 @@
@State(Scope.Thread)
@Fork(1)
public class ValuesSourceReaderBenchmark {
static {
LogConfigurator.configureESLogging();
}

private static final int BLOCK_LENGTH = 16 * 1024;
private static final int INDEX_SIZE = 10 * BLOCK_LENGTH;
private static final int COMMIT_INTERVAL = 500;
private static final BigArrays BIG_ARRAYS = BigArrays.NON_RECYCLING_INSTANCE;
private static final BlockFactory blockFactory = BlockFactory.getInstance(
new NoopCircuitBreaker("noop"),
BigArrays.NON_RECYCLING_INSTANCE
Expand Down
5 changes: 5 additions & 0 deletions docs/changelog/131053.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 131053
summary: Split large pages on load sometimes
area: ES|QL
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ protected void writeExtent(BlockLoader.IntBuilder builder, Extent extent) {
public BlockLoader.AllReader reader(LeafReaderContext context) throws IOException {
return new BlockLoader.AllReader() {
@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
var binaryDocValues = context.reader().getBinaryDocValues(fieldName);
var reader = new GeometryDocValueReader();
try (var builder = factory.ints(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(binaryDocValues, docs.get(i), reader, builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,10 +124,10 @@ private static class SingletonLongs extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.LongBuilder builder = factory.longsFromDocValues(docs.count())) {
int lastDoc = -1;
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < lastDoc) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -173,9 +173,9 @@ private static class Longs extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.LongBuilder builder = factory.longsFromDocValues(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < this.docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -259,10 +259,10 @@ private static class SingletonInts extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.IntBuilder builder = factory.intsFromDocValues(docs.count())) {
int lastDoc = -1;
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < lastDoc) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -308,9 +308,9 @@ private static class Ints extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.IntBuilder builder = factory.intsFromDocValues(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < this.docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -408,10 +408,10 @@ private static class SingletonDoubles extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count())) {
int lastDoc = -1;
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < lastDoc) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -461,9 +461,9 @@ private static class Doubles extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < this.docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -544,10 +544,10 @@ private static class DenseVectorValuesBlockReader extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
// Doubles from doc values ensures that the values are in order
try (BlockLoader.FloatBuilder builder = factory.denseVectors(docs.count(), dimensions)) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we also minus the offset in the previous line?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah. Let me figure out why that didn't get caught by the tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

There isn't a BlockLoaderTestCase for dense vectors. I think that's a "for later" problem.

int doc = docs.get(i);
if (doc < iterator.docID()) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -652,12 +652,12 @@ private BlockLoader.Block readSingleDoc(BlockFactory factory, int docId) throws
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
if (docs.count() == 1) {
return readSingleDoc(factory, docs.get(0));
}
try (BlockLoader.SingletonOrdinalsBuilder builder = factory.singletonOrdinalsBuilder(ordinals, docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < ordinals.docID()) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -700,9 +700,9 @@ private static class Ordinals extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BytesRefBuilder builder = factory.bytesRefsFromDocValues(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < ordinals.docID()) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -780,9 +780,9 @@ private static class BytesRefsFromBinary extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.BytesRefBuilder builder = factory.bytesRefs(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -879,9 +879,9 @@ private static class DenseVectorFromBinary extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.FloatBuilder builder = factory.denseVectors(docs.count(), dimensions)) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -963,10 +963,10 @@ private static class SingletonBooleans extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.BooleanBuilder builder = factory.booleansFromDocValues(docs.count())) {
int lastDoc = -1;
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < lastDoc) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down Expand Up @@ -1012,9 +1012,9 @@ private static class Booleans extends BlockDocValuesReader {
}

@Override
public BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException {
public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
try (BlockLoader.BooleanBuilder builder = factory.booleansFromDocValues(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
int doc = docs.get(i);
if (doc < this.docID) {
throw new IllegalStateException("docs within same block must be in order");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ interface ColumnAtATimeReader extends Reader {
/**
* Reads the values of all documents in {@code docs}.
*/
BlockLoader.Block read(BlockFactory factory, Docs docs) throws IOException;
BlockLoader.Block read(BlockFactory factory, Docs docs, int offset) throws IOException;
}

interface RowStrideReader extends Reader {
Expand Down Expand Up @@ -149,7 +149,7 @@ public String toString() {
*/
class ConstantNullsReader implements AllReader {
@Override
public Block read(BlockFactory factory, Docs docs) throws IOException {
public Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
return factory.constantNulls();
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those constantX() methods ignoring the offset? Is it right?

Copy link
Member Author

Choose a reason for hiding this comment

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

They are ignoring it and it's right. Constants always have the same value.

Copy link
Contributor

Choose a reason for hiding this comment

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

The block has to have a specific position count right? And given the other methods, the count is docs.getCount() - offset. As here offset is being ignored, the total position count could be wrong (?).

I'm not sure what that factory does exactly btw, maybe it already takes the offset into account?

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, I see it was changed already

}

Expand Down Expand Up @@ -183,7 +183,7 @@ public Builder builder(BlockFactory factory, int expectedCount) {
public ColumnAtATimeReader columnAtATimeReader(LeafReaderContext context) {
return new ColumnAtATimeReader() {
@Override
public Block read(BlockFactory factory, Docs docs) {
public Block read(BlockFactory factory, Docs docs, int offset) {
return factory.constantBytes(value);
}

Expand Down Expand Up @@ -261,8 +261,8 @@ public ColumnAtATimeReader columnAtATimeReader(LeafReaderContext context) throws
}
return new ColumnAtATimeReader() {
@Override
public Block read(BlockFactory factory, Docs docs) throws IOException {
return reader.read(factory, docs);
public Block read(BlockFactory factory, Docs docs, int offset) throws IOException {
return reader.read(factory, docs, offset);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't emit falses before trues so we conform to the doc values contract and can use booleansFromDocValues
try (BlockLoader.BooleanBuilder builder = factory.booleans(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't sort the values sort, so we can't use factory.longsFromDocValues
try (BlockLoader.LongBuilder builder = factory.longs(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't sort the values sort, so we can't use factory.doublesFromDocValues
try (BlockLoader.DoubleBuilder builder = factory.doubles(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't pre-sort our output so we can't use bytesRefsFromDocValues
try (BlockLoader.BytesRefBuilder builder = factory.bytesRefs(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't pre-sort our output so we can't use bytesRefsFromDocValues
try (BlockLoader.BytesRefBuilder builder = factory.bytesRefs(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public int docId() {
}

@Override
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs) throws IOException {
public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset) throws IOException {
// Note that we don't pre-sort our output so we can't use longsFromDocValues
try (BlockLoader.LongBuilder builder = factory.longs(docs.count())) {
for (int i = 0; i < docs.count(); i++) {
for (int i = offset; i < docs.count(); i++) {
read(docs.get(i), builder);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ static ElasticsearchCluster buildCluster() {
.setting("xpack.security.enabled", "false")
.setting("xpack.license.self_generated.type", "trial")
.setting("esql.query.allow_partial_results", "false")
.setting("logger.org.elasticsearch.compute.lucene.read", "DEBUG")
.jvmArg("-Xmx512m");
String javaVersion = JvmInfo.jvmInfo().version();
if (javaVersion.equals("20") || javaVersion.equals("21")) {
Expand Down
Loading
Loading