Skip to content

Commit e881c28

Browse files
committed
ESQL: Fix a breaker bug
Fixes a case where we release memory we never acquired, which breaker the circuit breaker accounting. Closes #135224 Closes #135260
1 parent 7cd76e1 commit e881c28

File tree

2 files changed

+7
-9
lines changed

2 files changed

+7
-9
lines changed

muted-tests.yml

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -534,12 +534,6 @@ tests:
534534
- class: org.elasticsearch.xpack.test.rest.XPackRestIT
535535
method: test {p0=analytics/nested_top_metrics_sort/terms order by top metrics numeric not null integer values}
536536
issue: https://github.com/elastic/elasticsearch/issues/135162
537-
- class: org.elasticsearch.compute.operator.topn.TopNOperatorTests
538-
method: testSimpleWithCranky
539-
issue: https://github.com/elastic/elasticsearch/issues/135224
540-
- class: org.elasticsearch.compute.operator.topn.TopNOperatorTests
541-
method: testSimpleCircuitBreaking
542-
issue: https://github.com/elastic/elasticsearch/issues/135260
543537
- class: org.elasticsearch.xpack.esql.qa.single_node.PushQueriesIT
544538
method: testEqualityAndOther {SEMANTIC_TEXT_WITH_KEYWORD}
545539
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");
@@ -684,8 +684,12 @@ public long ramBytesUsed() {
684684

685685
@Override
686686
public void close() {
687-
Releasables.close(Releasables.wrap(this), () -> breaker.addWithoutBreaking(-Queue.sizeOf(topCount)));
688-
687+
Releasables.close(
688+
// Release all entries in the topn
689+
Releasables.wrap(this),
690+
// Release the array itself
691+
() -> breaker.addWithoutBreaking(-Queue.sizeOf(topCount))
692+
);
689693
}
690694

691695
public static long sizeOf(int topCount) {

0 commit comments

Comments
 (0)