-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Chunked encoding for tasks APIs #91935
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 tasks APIs #91935
Conversation
This response can reach many MiB in size in a large and busy cluster, let's use chunking here. Relates elastic#89838
Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just one spot to fix I think :)
@SafeVarargs | ||
@SuppressWarnings("varargs") | ||
public static <T> Stream<T> concat(Stream<T>... streams) { | ||
if (streams.length == 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just return Arrays.stream(streams).flatMap(Function.identity());
? We might even get into stack depth troubles if we use the concat like we do here?
The docs for concat state:
Use caution when constructing streams from repeated concatenation. Accessing an element of a deeply concatenated stream can result in deep call chains, or even StackOverflowError.
Subsequent changes to the sequential/parallel execution mode of the returned stream are not guaranteed to be propagated to the input streams.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might even get into stack depth troubles
Touché 😁
Yeah you're right .flatMap(Function.identity())
is better here, no need for a utility to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
This response can reach many MiB in size in a large and busy cluster, let's use chunking here.
Relates #89838