Skip to content

Commit c990377

Browse files
authored
ESQL: Limit memory usage of fold (#118602)
`fold` can be surprisingly heavy! The maximally efficient/paranoid thing would be to fold each expression one time, in the constant folding rule, and then store the result as a `Literal`. But this PR doesn't do that because it's a big change. Instead, it creates the infrastructure for tracking memory usage for folding as plugs it into as many places as possible. That's not perfect, but it's better. This infrastructure limit the allocations of fold similar to the `CircuitBreaker` infrastructure we use for values, but it's different in a critical way: you don't manually free any of the values. This is important because the plan itself isn't `Releasable`, which is required when using a real CircuitBreaker. We could have tried to make the plan releasable, but that'd be a huge change. Right now there's a single limit of 5% of heap per query. We create the limit at the start of query planning and use it throughout planning. There are about 40 places that don't yet use it. We should get them plugged in as quick as we can manage. After that, we should look to the maximally efficient/paranoid thing that I mentioned about waiting for constant folding. That's an even bigger change, one I'm not equipped to make on my own.
1 parent 67e1bd4 commit c990377

File tree

170 files changed

+1858
-972
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

170 files changed

+1858
-972
lines changed

benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/EvalBenchmark.java

+15-10
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.core.TimeValue;
2828
import org.elasticsearch.xpack.esql.core.expression.Expression;
2929
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
30+
import org.elasticsearch.xpack.esql.core.expression.FoldContext;
3031
import org.elasticsearch.xpack.esql.core.expression.Literal;
3132
import org.elasticsearch.xpack.esql.core.expression.predicate.regex.RLikePattern;
3233
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -71,12 +72,11 @@ public class EvalBenchmark {
7172
BigArrays.NON_RECYCLING_INSTANCE
7273
);
7374

75+
private static final FoldContext FOLD_CONTEXT = FoldContext.small();
76+
7477
private static final int BLOCK_LENGTH = 8 * 1024;
7578

76-
static final DriverContext driverContext = new DriverContext(
77-
BigArrays.NON_RECYCLING_INSTANCE,
78-
BlockFactory.getInstance(new NoopCircuitBreaker("noop"), BigArrays.NON_RECYCLING_INSTANCE)
79-
);
79+
static final DriverContext driverContext = new DriverContext(BigArrays.NON_RECYCLING_INSTANCE, blockFactory);
8080

8181
static {
8282
// Smoke test all the expected values and force loading subclasses more like prod
@@ -114,18 +114,20 @@ private static EvalOperator.ExpressionEvaluator evaluator(String operation) {
114114
return switch (operation) {
115115
case "abs" -> {
116116
FieldAttribute longField = longField();
117-
yield EvalMapper.toEvaluator(new Abs(Source.EMPTY, longField), layout(longField)).get(driverContext);
117+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new Abs(Source.EMPTY, longField), layout(longField)).get(driverContext);
118118
}
119119
case "add" -> {
120120
FieldAttribute longField = longField();
121121
yield EvalMapper.toEvaluator(
122+
FOLD_CONTEXT,
122123
new Add(Source.EMPTY, longField, new Literal(Source.EMPTY, 1L, DataType.LONG)),
123124
layout(longField)
124125
).get(driverContext);
125126
}
126127
case "add_double" -> {
127128
FieldAttribute doubleField = doubleField();
128129
yield EvalMapper.toEvaluator(
130+
FOLD_CONTEXT,
129131
new Add(Source.EMPTY, doubleField, new Literal(Source.EMPTY, 1D, DataType.DOUBLE)),
130132
layout(doubleField)
131133
).get(driverContext);
@@ -140,7 +142,8 @@ private static EvalOperator.ExpressionEvaluator evaluator(String operation) {
140142
lhs = new Add(Source.EMPTY, lhs, new Literal(Source.EMPTY, 1L, DataType.LONG));
141143
rhs = new Add(Source.EMPTY, rhs, new Literal(Source.EMPTY, 1L, DataType.LONG));
142144
}
143-
yield EvalMapper.toEvaluator(new Case(Source.EMPTY, condition, List.of(lhs, rhs)), layout(f1, f2)).get(driverContext);
145+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new Case(Source.EMPTY, condition, List.of(lhs, rhs)), layout(f1, f2))
146+
.get(driverContext);
144147
}
145148
case "date_trunc" -> {
146149
FieldAttribute timestamp = new FieldAttribute(
@@ -149,35 +152,37 @@ private static EvalOperator.ExpressionEvaluator evaluator(String operation) {
149152
new EsField("timestamp", DataType.DATETIME, Map.of(), true)
150153
);
151154
yield EvalMapper.toEvaluator(
155+
FOLD_CONTEXT,
152156
new DateTrunc(Source.EMPTY, new Literal(Source.EMPTY, Duration.ofHours(24), DataType.TIME_DURATION), timestamp),
153157
layout(timestamp)
154158
).get(driverContext);
155159
}
156160
case "equal_to_const" -> {
157161
FieldAttribute longField = longField();
158162
yield EvalMapper.toEvaluator(
163+
FOLD_CONTEXT,
159164
new Equals(Source.EMPTY, longField, new Literal(Source.EMPTY, 100_000L, DataType.LONG)),
160165
layout(longField)
161166
).get(driverContext);
162167
}
163168
case "long_equal_to_long" -> {
164169
FieldAttribute lhs = longField();
165170
FieldAttribute rhs = longField();
166-
yield EvalMapper.toEvaluator(new Equals(Source.EMPTY, lhs, rhs), layout(lhs, rhs)).get(driverContext);
171+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new Equals(Source.EMPTY, lhs, rhs), layout(lhs, rhs)).get(driverContext);
167172
}
168173
case "long_equal_to_int" -> {
169174
FieldAttribute lhs = longField();
170175
FieldAttribute rhs = intField();
171-
yield EvalMapper.toEvaluator(new Equals(Source.EMPTY, lhs, rhs), layout(lhs, rhs)).get(driverContext);
176+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new Equals(Source.EMPTY, lhs, rhs), layout(lhs, rhs)).get(driverContext);
172177
}
173178
case "mv_min", "mv_min_ascending" -> {
174179
FieldAttribute longField = longField();
175-
yield EvalMapper.toEvaluator(new MvMin(Source.EMPTY, longField), layout(longField)).get(driverContext);
180+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, new MvMin(Source.EMPTY, longField), layout(longField)).get(driverContext);
176181
}
177182
case "rlike" -> {
178183
FieldAttribute keywordField = keywordField();
179184
RLike rlike = new RLike(Source.EMPTY, keywordField, new RLikePattern(".ar"));
180-
yield EvalMapper.toEvaluator(rlike, layout(keywordField)).get(driverContext);
185+
yield EvalMapper.toEvaluator(FOLD_CONTEXT, rlike, layout(keywordField)).get(driverContext);
181186
}
182187
default -> throw new UnsupportedOperationException();
183188
};

docs/changelog/118602.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 118602
2+
summary: Limit memory usage of `fold`
3+
area: ES|QL
4+
type: bug
5+
issues: []

docs/reference/esql/functions/kibana/definition/bucket.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/reference/esql/functions/kibana/definition/match_operator.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/reference/esql/functions/kibana/docs/match_operator.md

+8-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

server/src/main/java/org/elasticsearch/common/settings/Setting.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1727,7 +1727,7 @@ public static <T extends Enum<T>> Setting<T> enumSetting(
17271727
*
17281728
* @param key the key for the setting
17291729
* @param defaultValue the default value for this setting
1730-
* @param properties properties properties for this setting like scope, filtering...
1730+
* @param properties properties for this setting like scope, filtering...
17311731
* @return the setting object
17321732
*/
17331733
public static Setting<ByteSizeValue> memorySizeSetting(String key, ByteSizeValue defaultValue, Property... properties) {

test/external-modules/esql-heap-attack/src/javaRestTest/java/org/elasticsearch/xpack/esql/heap_attack/HeapAttackIT.java

+32-3
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ private void assertCircuitBreaks(ThrowingRunnable r) throws IOException {
194194
);
195195
}
196196

197+
private void assertFoldCircuitBreaks(ThrowingRunnable r) throws IOException {
198+
ResponseException e = expectThrows(ResponseException.class, r);
199+
Map<?, ?> map = responseAsMap(e.getResponse());
200+
logger.info("expected fold circuit breaking {}", map);
201+
assertMap(
202+
map,
203+
matchesMap().entry("status", 400).entry("error", matchesMap().extraOk().entry("type", "fold_too_much_memory_exception"))
204+
);
205+
}
206+
197207
private void assertParseFailure(ThrowingRunnable r) throws IOException {
198208
ResponseException e = expectThrows(ResponseException.class, r);
199209
Map<?, ?> map = responseAsMap(e.getResponse());
@@ -325,11 +335,23 @@ public void testManyConcatFromRow() throws IOException {
325335
assertManyStrings(resp, strings);
326336
}
327337

338+
/**
339+
* Hits a circuit breaker by building many moderately long strings.
340+
*/
341+
public void testHugeManyConcatFromRow() throws IOException {
342+
assertFoldCircuitBreaks(
343+
() -> manyConcat(
344+
"ROW a=9999999999999, b=99999999999999999, c=99999999999999999, d=99999999999999999, e=99999999999999999",
345+
5000
346+
)
347+
);
348+
}
349+
328350
/**
329351
* Fails to parse a huge huge query.
330352
*/
331353
public void testHugeHugeManyConcatFromRow() throws IOException {
332-
assertParseFailure(() -> manyConcat("ROW a=9999, b=9999, c=9999, d=9999, e=9999", 50000));
354+
assertParseFailure(() -> manyConcat("ROW a=9999, b=9999, c=9999, d=9999, e=9999", 6000));
333355
}
334356

335357
/**
@@ -387,13 +409,20 @@ public void testHugeManyRepeat() throws IOException {
387409
* Returns many moderately long strings.
388410
*/
389411
public void testManyRepeatFromRow() throws IOException {
390-
int strings = 10000;
412+
int strings = 300;
391413
Response resp = manyRepeat("ROW a = 99", strings);
392414
assertManyStrings(resp, strings);
393415
}
394416

395417
/**
396-
* Fails to parse a huge huge query.
418+
* Hits a circuit breaker by building many moderately long strings.
419+
*/
420+
public void testHugeManyRepeatFromRow() throws IOException {
421+
assertFoldCircuitBreaks(() -> manyRepeat("ROW a = 99", 400));
422+
}
423+
424+
/**
425+
* Fails to parse a huge, huge query.
397426
*/
398427
public void testHugeHugeManyRepeatFromRow() throws IOException {
399428
assertParseFailure(() -> manyRepeat("ROW a = 99", 100000));

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expression.java

+10-2
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,20 @@ public Expression(Source source, List<Expression> children) {
7878
super(source, children);
7979
}
8080

81-
// whether the expression can be evaluated statically (folded) or not
81+
/**
82+
* Whether the expression can be evaluated statically, aka "folded", or not.
83+
*/
8284
public boolean foldable() {
8385
return false;
8486
}
8587

86-
public Object fold() {
88+
/**
89+
* Evaluate this expression statically to a constant. It is an error to call
90+
* this if {@link #foldable} returns false.
91+
*/
92+
public Object fold(FoldContext ctx) {
93+
// TODO After removing FoldContext.unbounded from non-test code examine all calls
94+
// for places we should use instanceof Literal instead
8795
throw new QlIllegalArgumentException("Should not fold expression");
8896
}
8997

x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ public static boolean foldable(List<? extends Expression> exps) {
107107
return true;
108108
}
109109

110-
public static List<Object> fold(List<? extends Expression> exps) {
110+
public static List<Object> fold(FoldContext ctx, List<? extends Expression> exps) {
111111
List<Object> folded = new ArrayList<>(exps.size());
112112
for (Expression exp : exps) {
113-
folded.add(exp.fold());
113+
folded.add(exp.fold(ctx));
114114
}
115115

116116
return folded;
@@ -135,7 +135,7 @@ public static String name(Expression e) {
135135
/**
136136
* Is this {@linkplain Expression} <strong>guaranteed</strong> to have
137137
* only the {@code null} value. {@linkplain Expression}s that
138-
* {@link Expression#fold()} to {@code null} <strong>may</strong>
138+
* {@link Expression#fold} to {@code null} <strong>may</strong>
139139
* return {@code false} here, but should <strong>eventually</strong> be folded
140140
* into a {@link Literal} containing {@code null} which will return
141141
* {@code true} from here.

0 commit comments

Comments
 (0)