Skip to content

Commit 504ab31

Browse files
authored
ESQL: Fix a breaker bug (#136105) (#136293)
Fixes a case where we release memory we never acquired, which breaker the circuit breaker accounting. Closes #135224 Closes #135260
1 parent bbc1639 commit 504ab31

File tree

3 files changed

+14
-9
lines changed

3 files changed

+14
-9
lines changed

docs/changelog/136105.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 136105
2+
summary: Fix a breaker bug
3+
area: ES|QL
4+
type: bug
5+
issues:
6+
- 135224
7+
- 135260

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -546,12 +546,6 @@ tests:
546546
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
547547
method: test {p0=analytics/nested_top_metrics_sort/terms order by top metrics numeric not null integer values}
548548
issue: https://github.com/elastic/elasticsearch/issues/135162
549-
- class: org.elasticsearch.compute.operator.topn.TopNOperatorTests
550-
method: testSimpleWithCranky
551-
issue: https://github.com/elastic/elasticsearch/issues/135224
552-
- class: org.elasticsearch.compute.operator.topn.TopNOperatorTests
553-
method: testSimpleCircuitBreaking
554-
issue: https://github.com/elastic/elasticsearch/issues/135260
555549
- class: org.elasticsearch.xpack.esql.qa.single_node.PushQueriesIT
556550
method: testEqualityAndOther {SEMANTIC_TEXT_WITH_KEYWORD}
557551
issue: https://github.com/elastic/elasticsearch/issues/135333

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/topn/TopNOperator.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,10 @@ static final class Row implements Accountable, Releasable {
8484
RefCounted shardRefCounter;
8585

8686
Row(CircuitBreaker breaker, List<SortOrder> sortOrders, int preAllocatedKeysSize, int preAllocatedValueSize) {
87+
breaker.addEstimateBytesAndMaybeBreak(SHALLOW_SIZE, "topn");
8788
this.breaker = breaker;
8889
boolean success = false;
8990
try {
90-
breaker.addEstimateBytesAndMaybeBreak(SHALLOW_SIZE, "topn");
9191
keys = new BreakingBytesRefBuilder(breaker, "topn", preAllocatedKeysSize);
9292
values = new BreakingBytesRefBuilder(breaker, "topn", preAllocatedValueSize);
9393
bytesOrder = new BytesOrder(sortOrders, breaker, "topn");
@@ -679,8 +679,12 @@ public long ramBytesUsed() {
679679

680680
@Override
681681
public void close() {
682-
Releasables.close(Releasables.wrap(this), () -> breaker.addWithoutBreaking(-Queue.sizeOf(topCount)));
683-
682+
Releasables.close(
683+
// Release all entries in the topn
684+
Releasables.wrap(this),
685+
// Release the array itself
686+
() -> breaker.addWithoutBreaking(-Queue.sizeOf(topCount))
687+
);
684688
}
685689

686690
public static long sizeOf(int topCount) {

0 commit comments

Comments
 (0)