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.

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

Make ChunkedRestResponseBody extend Releasable #99871

Conversation

DaveCTurner
Copy link
Contributor

If an unchunked body implements Releasable then it is released once
the response is sent. We have some need for the same behaviour with
chunked bodies, except that in this case there's no need for an
instanceof check since we control the body type directly. This commit
makes ChunkedRestResponseBody extend Releasable and adds support for
closing it when the response is sent.

If an unchunked body implements `Releasable` then it is released once
the response is sent. We have some need for the same behaviour with
chunked bodies, except that in this case there's no need for an
`instanceof` check since we control the body type directly. This commit
makes `ChunkedRestResponseBody extend Releasable` and adds support for
closing it when the response is sent.
@DaveCTurner DaveCTurner added >non-issue :Distributed Coordination/Network Http and internode communication implementations v8.11.0 labels Sep 25, 2023
@DaveCTurner DaveCTurner requested a review from nik9000 September 25, 2023 12:35
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Sep 25, 2023
@nik9000
Copy link
Member

nik9000 commented Sep 25, 2023

It does exactly what I need. Wonderful1

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @DaveCTurner. LGTM.

}
assertTrue(isClosed.get());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

This is almost exactly what I have in my branch.

@DaveCTurner DaveCTurner merged commit 2458118 into elastic:main Sep 25, 2023
@DaveCTurner DaveCTurner deleted the 2023/09/25/ChunkedRestResponseBody-extends-Releasable branch September 25, 2023 13:40
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Sep 25, 2023
Follow-up to elastic#99871 to inline and remove the deprecated overrides which
do not take a `Releasable` parameter.
elasticsearchmachine pushed a commit that referenced this pull request Sep 25, 2023
Follow-up to #99871 to inline and remove the deprecated overrides which
do not take a `Releasable` parameter.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 25, 2024
On reflection is was probably a mistake to give each
`ChunkedRestResponseBody` a nontrivial lifecycle in elastic#99871. The
lifecycle really belongs to the whole containing `RestResponse`. This
commit moves it there.
DaveCTurner added a commit that referenced this pull request Jan 25, 2024
On reflection is was probably a mistake to give each
`ChunkedRestResponseBody` a nontrivial lifecycle in #99871. The
lifecycle really belongs to the whole containing `RestResponse`. This
commit moves it there.
@DaveCTurner DaveCTurner restored the 2023/09/25/ChunkedRestResponseBody-extends-Releasable branch June 17, 2024 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Network Http and internode communication implementations >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants