From ebb8421e734b855e18e5c82f9de6d3a8d8556a6f Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Mon, 17 Nov 2025 07:05:54 -0800 Subject: [PATCH] Fix integer overflow in block memory estimation (#138132) When constructing a very large block, the memory estimate (multiplying two int values) can overflow, yielding a negative value instead of several GB. The negative estimate bypasses the circuit breaker, so the allocation proceeds and may lead to OOM. This change fixes the overflow by casting to long before multiplication so the breaker can trigger correctly. We should avoid allocating a gigantic contiguous primitive array; once size crosses a threshold we should switch to BigArray. I will address it in a follow-up. --- docs/changelog/138132.yaml | 5 ++ .../compute/data/BooleanBlockBuilder.java | 2 +- .../data/BooleanVectorFixedBuilder.java | 2 +- .../compute/data/DoubleBlockBuilder.java | 2 +- .../data/DoubleVectorFixedBuilder.java | 2 +- .../compute/data/FloatBlockBuilder.java | 2 +- .../compute/data/FloatVectorFixedBuilder.java | 2 +- .../compute/data/IntBlockBuilder.java | 2 +- .../compute/data/IntVectorFixedBuilder.java | 2 +- .../compute/data/LongBlockBuilder.java | 2 +- .../compute/data/LongVectorFixedBuilder.java | 2 +- .../compute/data/AbstractBlockBuilder.java | 4 +- .../compute/data/X-BlockBuilder.java.st | 2 +- .../compute/data/X-VectorFixedBuilder.java.st | 2 +- .../compute/data/BlockAccountingTests.java | 63 +++++++++++++++++++ 15 files changed, 82 insertions(+), 14 deletions(-) create mode 100644 docs/changelog/138132.yaml diff --git a/docs/changelog/138132.yaml b/docs/changelog/138132.yaml new file mode 100644 index 0000000000000..3235ba43a9921 --- /dev/null +++ b/docs/changelog/138132.yaml @@ -0,0 +1,5 @@ +pr: 138132 +summary: Fix integer overflow in block memory estimation +area: ES|QL +type: bug +issues: [] diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBlockBuilder.java index 1f7c59bbea153..b2686761c33a6 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanBlockBuilder.java @@ -29,7 +29,7 @@ final class BooleanBlockBuilder extends AbstractBlockBuilder implements BooleanB BooleanBlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new boolean[initialSize]; } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanVectorFixedBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanVectorFixedBuilder.java index 21835281393a5..ae190c26c30a5 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanVectorFixedBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/BooleanVectorFixedBuilder.java @@ -50,7 +50,7 @@ private static long ramBytesUsed(int size) { return size == 1 ? ConstantBooleanVector.RAM_BYTES_USED : BooleanArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * Byte.BYTES + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * Byte.BYTES ); } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBlockBuilder.java index 7a69367d85db6..4b0af02b87887 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleBlockBuilder.java @@ -29,7 +29,7 @@ final class DoubleBlockBuilder extends AbstractBlockBuilder implements DoubleBlo DoubleBlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new double[initialSize]; } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleVectorFixedBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleVectorFixedBuilder.java index 2ce356220f257..b64722bee9da5 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleVectorFixedBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/DoubleVectorFixedBuilder.java @@ -50,7 +50,7 @@ private static long ramBytesUsed(int size) { return size == 1 ? ConstantDoubleVector.RAM_BYTES_USED : DoubleArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * Double.BYTES + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * Double.BYTES ); } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatBlockBuilder.java index e0101df2b42de..d99dbb80871a6 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatBlockBuilder.java @@ -29,7 +29,7 @@ final class FloatBlockBuilder extends AbstractBlockBuilder implements FloatBlock FloatBlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new float[initialSize]; } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatVectorFixedBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatVectorFixedBuilder.java index d18d24809301f..c35c43f649211 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatVectorFixedBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/FloatVectorFixedBuilder.java @@ -50,7 +50,7 @@ private static long ramBytesUsed(int size) { return size == 1 ? ConstantFloatVector.RAM_BYTES_USED : FloatArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * Float.BYTES + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * Float.BYTES ); } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBlockBuilder.java index d957993ce10a2..bf395be9ca5a3 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntBlockBuilder.java @@ -29,7 +29,7 @@ final class IntBlockBuilder extends AbstractBlockBuilder implements IntBlock.Bui IntBlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new int[initialSize]; } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntVectorFixedBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntVectorFixedBuilder.java index 56f92f9d0eb6e..968f1944df90c 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntVectorFixedBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/IntVectorFixedBuilder.java @@ -50,7 +50,7 @@ private static long ramBytesUsed(int size) { return size == 1 ? ConstantIntVector.RAM_BYTES_USED : IntArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * Integer.BYTES + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * Integer.BYTES ); } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBlockBuilder.java index cab565d47f58a..d0ac11bcf11fb 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongBlockBuilder.java @@ -29,7 +29,7 @@ final class LongBlockBuilder extends AbstractBlockBuilder implements LongBlock.B LongBlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new long[initialSize]; } diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongVectorFixedBuilder.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongVectorFixedBuilder.java index 2fe289de7fd77..f4dac7403cb1e 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongVectorFixedBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/LongVectorFixedBuilder.java @@ -50,7 +50,7 @@ private static long ramBytesUsed(int size) { return size == 1 ? ConstantLongVector.RAM_BYTES_USED : LongArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * Long.BYTES + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * Long.BYTES ); } diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlockBuilder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlockBuilder.java index 4755b1d609cfb..1382257104b79 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlockBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractBlockBuilder.java @@ -147,9 +147,9 @@ protected final void ensureCapacity() { return; } int newSize = ArrayUtil.oversize(valueCount, elementSize()); - adjustBreaker(newSize * elementSize()); + adjustBreaker((long) newSize * elementSize()); growValuesArray(newSize); - adjustBreaker(-valuesLength * elementSize()); + adjustBreaker(-(long) valuesLength * elementSize()); } @Override diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st index 97adf2871909b..ae00202981d9a 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-BlockBuilder.java.st @@ -38,7 +38,7 @@ $else$ $Type$BlockBuilder(int estimatedSize, BlockFactory blockFactory) { super(blockFactory); int initialSize = Math.max(estimatedSize, 2); - adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + initialSize * elementSize()); + adjustBreaker(RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) initialSize * elementSize()); values = new $type$[initialSize]; } $endif$ diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-VectorFixedBuilder.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-VectorFixedBuilder.java.st index 8bfc48972f995..3328243a316a5 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-VectorFixedBuilder.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-VectorFixedBuilder.java.st @@ -50,7 +50,7 @@ public final class $Type$VectorFixedBuilder implements $Type$Vector.FixedBuilder return size == 1 ? Constant$Type$Vector.RAM_BYTES_USED : $Type$ArrayVector.BASE_RAM_BYTES_USED + RamUsageEstimator.alignObjectSize( - (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + size * $BYTES$ + (long) RamUsageEstimator.NUM_BYTES_ARRAY_HEADER + (long) size * $BYTES$ ); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockAccountingTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockAccountingTests.java index 248dc23c09269..630d3f054cb38 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockAccountingTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockAccountingTests.java @@ -9,8 +9,11 @@ import org.apache.lucene.tests.util.RamUsageTester; import org.apache.lucene.tests.util.RamUsageTester.Accumulator; +import org.apache.lucene.util.ArrayUtil; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.RamUsageEstimator; +import org.elasticsearch.common.breaker.CircuitBreakingException; +import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.util.BigArray; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.BytesRefArray; @@ -26,6 +29,7 @@ import static org.apache.lucene.util.RamUsageEstimator.alignObjectSize; import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.lessThan; @@ -349,6 +353,65 @@ public void testDoubleBlockWithNullFirstValues() { assertThat(empty.ramBytesUsed(), is(expectedEmptyUsed)); } + /** + * Ideally we would test a real Block builder, but that would require a large heap which is not possible in unit tests. + * Instead, a simulated builder tracks memory usage without allocating the backing array. + */ + public void testHugeBlockBuilder() { + class NoopBlockBuilder extends AbstractBlockBuilder { + private int size = 0; + + NoopBlockBuilder(BlockFactory blockFactory) { + super(blockFactory); + } + + @Override + protected int valuesLength() { + return size; + } + + @Override + protected void growValuesArray(int newSize) { + size = newSize; + } + + @Override + protected int elementSize() { + return Long.BYTES; + } + + @Override + public Block.Builder copyFrom(Block block, int beginInclusive, int endExclusive) { + throw new UnsupportedOperationException(); + } + + @Override + public Block.Builder mvOrdering(Block.MvOrdering mvOrdering) { + throw new UnsupportedOperationException(); + } + + @Override + public Block build() { + throw new UnsupportedOperationException(); + } + + void appendUntilBreaking() { + int maxArrayLength = ArrayUtil.MAX_ARRAY_LENGTH; + for (long i = 0; i < maxArrayLength; i++) { + ensureCapacity(); + valueCount++; + } + } + } + ByteSizeValue largeHeap = ByteSizeValue.ofMb(between(4 * 1024, 8 * 1024)); + BlockFactory blockFactory = blockFactory(largeHeap); + try (var builder = new NoopBlockBuilder(blockFactory)) { + expectThrows(CircuitBreakingException.class, builder::appendUntilBreaking); + } finally { + assertThat(blockFactory.breaker().getUsed(), equalTo(0L)); + } + } + static Matcher between(long minInclusive, long maxInclusive) { return allOf(greaterThanOrEqualTo(minInclusive), lessThanOrEqualTo(maxInclusive)); }