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

Chunked encoding for cluster reroute API #92615

Merged

Conversation

DaveCTurner
Copy link
Contributor

The cluster reroute API (optionally) returns the cluster state in its response, which can therefore be rather large. #92285 enables a chunked encoding of the cluster state, and this commit adjusts the reroute API to make use of this encoding too.

The cluster reroute API (optionally) returns the cluster state in its
response, which can therefore be rather large. elastic#92285 enables a chunked
encoding of the cluster state, and this commit adjusts the reroute API
to make use of this encoding too.
@DaveCTurner DaveCTurner added >non-issue :Distributed Coordination/Network Http and internode communication implementations v8.7.0 labels Dec 31, 2022
@DaveCTurner DaveCTurner marked this pull request as ready for review December 31, 2022 10:31
@elasticsearchmachine elasticsearchmachine added the Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. label Dec 31, 2022
@elasticsearchmachine
Copy link
Collaborator

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

Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

Comment on lines 34 to 35
* Similar to {@link #toXContentChunked} but for the {@link RestApiVersion#V_7} API. Note that chunked response bodies cannot send
* deprecation warning headers once transmission has started, so implementations must check for deprecated feature use before returning.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the deprecation warning warning should go into the class javadocs instead. The usage added in this PR emits a warning in toXContentChunked, not the v7 one, and I think it applies equally to both.

}
int actualChunks = 0;
final var iterator = response.toXContentChunked(params);
while (iterator.hasNext()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we assert that only toXContentChunked outputs deprecations warnings? We may have to stop filtering warnings, but that seems desirable anyway.

@DaveCTurner DaveCTurner added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 24, 2023
@elasticsearchmachine elasticsearchmachine merged commit 87f2221 into elastic:main Jan 24, 2023
@DaveCTurner DaveCTurner deleted the 2022-12-31-chunked-reroute-api branch January 24, 2023 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :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.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants