Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ActionListener#notifyOnce can release its delegate listener once completed #92451

Closed
DaveCTurner opened this issue Dec 19, 2022 · 1 comment · Fixed by #92452
Closed

ActionListener#notifyOnce can release its delegate listener once completed #92451

DaveCTurner opened this issue Dec 19, 2022 · 1 comment · Fixed by #92452
Labels
:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team

Comments

@DaveCTurner
Copy link
Contributor

elasticsearch/server/src/main/java/org/elasticsearch/action/ActionListener.java

Lines 420 to 432 in 099fb84

static <Response> ActionListener<Response> notifyOnce(ActionListener<Response> delegate) {
return new NotifyOnceListener<Response>() {
@Override
protected void innerOnResponse(Response response) {
delegate.onResponse(response);
}
@Override
protected void innerOnFailure(Exception e) {
delegate.onFailure(e);
}
};
}

Rather than keeping hold of the delegate and using an AtomicBoolean to track the completion, we could use an AtomicReference and drop the whole listener on completion, which might free up heap in some fail-fast cases.
@DaveCTurner DaveCTurner added >enhancement :Core/Infra/Core Core issues without another label labels Dec 19, 2022
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Dec 19, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@DaveCTurner DaveCTurner mentioned this issue Dec 19, 2022
Merged
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this issue Dec 19, 2022
@DaveCTurner
ActionListener#notifyOnce should release delegate on completion
a7342ae 
There's no need to keep hold of the delegate after completing it, and in
some cases this might hold on to excessive heap. With this commit we
drop the reference to the delegate when it's complete.

Closes elastic#92451
@DaveCTurner DaveCTurner mentioned this issue Dec 19, 2022
Merged
elasticsearchmachine pushed a commit that referenced this issue Dec 19, 2022
@DaveCTurner
ActionListener#notifyOnce should release delegate on completion (#92452)
661ea5f 
There's no need to keep hold of the delegate after completing it, and in
some cases this might hold on to excessive heap. With this commit we
drop the reference to the delegate when it's complete.

Closes #92451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Core/Infra/Core Core issues without another label >enhancement Team:Core/Infra Meta label for core/infra team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ActionListener#notifyOnce should release delegate on completion DaveCTurner/elasticsearch
2 participants
@DaveCTurner @elasticsearchmachine