Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions docs/changelog/122417.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 122417
summary: Fix listener leak in exchange service
area: ES|QL
type: bug
issues:
- 122271
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public EsqlRefCountingListener(ActionListener<Void> delegate) {
}

public ActionListener<Void> acquire() {
return refs.acquireListener().delegateResponse((l, e) -> {
var listener = ActionListener.assertAtLeastOnce(refs.acquireListener());
return listener.delegateResponse((l, e) -> {
failureCollector.unwrapAndCollect(e);
l.onFailure(e);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.elasticsearch.action.ActionListenerResponseHandler;
import org.elasticsearch.action.support.ChannelActionListener;
import org.elasticsearch.action.support.SubscribableListener;
import org.elasticsearch.common.breaker.CircuitBreakingException;
import org.elasticsearch.common.component.AbstractLifecycleComponent;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -366,7 +367,13 @@ private void doFetchPageAsync(boolean allSourcesFinished, ActionListener<Exchang
final long reservedBytes = allSourcesFinished ? 0 : estimatedPageSizeInBytes.get();
if (reservedBytes > 0) {
// This doesn't fully protect ESQL from OOM, but reduces the likelihood.
blockFactory.breaker().addEstimateBytesAndMaybeBreak(reservedBytes, "fetch page");
try {
blockFactory.breaker().addEstimateBytesAndMaybeBreak(reservedBytes, "fetch page");
} catch (Exception e) {
assert e instanceof CircuitBreakingException : new AssertionError(e);
listener.onFailure(e);
return;
}
listener = ActionListener.runAfter(listener, () -> blockFactory.breaker().addWithoutBreaking(-reservedBytes));
}
transportService.sendChildRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ final class ComputeListener implements Releasable {
* Acquires a new listener that doesn't collect result
*/
ActionListener<Void> acquireAvoid() {
return refs.acquire().delegateResponse((l, e) -> {
var listener = ActionListener.assertAtLeastOnce(refs.acquire());
return listener.delegateResponse((l, e) -> {
try {
runOnFailure.run();
} finally {
Expand Down