Introduce StreamingXContentResponse
#111933
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Similar to `ChunkedZipResponse` (elastic#109820) this utility allows Elasticsearch to send an `XContent`-based response constructed out of a sequence of `ChunkedToXContent` fragments, provided in a streaming and asynchronous fashion. This will enable elastic#93735 to proceed without needing to create a temporary index to hold the intermediate results.
DaveCTurner
added
>non-issue
:Distributed Coordination/Network
elasticsearchmachine
added
the
Team:Distributed
|
Pinging @elastic/es-distributed (Team:Distributed) |
Abhiram98
reviewed
Aug 16, 2024
server/src/main/java/org/elasticsearch/rest/StreamingXContentResponse.java
Show resolved
Hide resolved
Abhiram98
reviewed
Aug 16, 2024
server/src/main/java/org/elasticsearch/rest/StreamingXContentResponse.java
Show resolved
Hide resolved
|
Hi! I added a couple of comments about refactoring the StreamingXContentResponse class. I am happy to send you a small patch to change the code. Let me know what you think? |
nicktindall
reviewed
Aug 19, 2024
| releasables.add(fragment.releasable()); | ||
| } | ||
| assert fragmentQueue.isEmpty() : fragmentQueue.size(); // no concurrent adds | ||
| assert releasables.size() == taskCount - 1 || releasables.size() == taskCount - 2 : taskCount + " vs " + releasables.size(); |
There was a problem hiding this comment.
I'm missing something here, why would releaseables.size() == taskCount - 2, I get that queueLength is an upper bound, but drainQueue only happens after the enqueues have stopped (thus making queueLength accurate), correct?
There was a problem hiding this comment.
We pop one item from the queue before we start to process it, but we decrement queueLength when its processing is complete. So mostly queueLength will be counting this one extra item that isn't really in the queue any more.
nicktindall
approved these changes
Aug 19, 2024
weizijun
added a commit
to weizijun/elasticsearch
that referenced
this pull request
Aug 19, 2024
* upstream/main: Fail `indexDocs()` on rejection (elastic#111962) Move repo analyzer to its own package (elastic#111963) Add generated evaluators for DateNanos conversion functions (elastic#111961) Clean the last traces from global retention in templates (elastic#111669) Fix known issue docs for elastic#111866 (elastic#111956) x-pack/plugin/otel: introduce x-pack-otel plugin (elastic#111091) Improve reaction to blob store corruptions (elastic#111954) Introduce `StreamingXContentResponse` (elastic#111933) Revert "Add 8.15.0 known issue for memory locking in Windows (elastic#111949)" Test get-snapshots API with missing details (elastic#111903) Add 8.15.0 known issue for memory locking in Windows (elastic#111949) # Conflicts: # server/src/main/java/org/elasticsearch/TransportVersions.java
cbuescher
pushed a commit
to cbuescher/elasticsearch
that referenced
this pull request
Sep 4, 2024
Similar to `ChunkedZipResponse` (elastic#109820) this utility allows Elasticsearch to send an `XContent`-based response constructed out of a sequence of `ChunkedToXContent` fragments, provided in a streaming and asynchronous fashion. This will enable elastic#93735 to proceed without needing to create a temporary index to hold the intermediate results.
davidkyle
pushed a commit
to davidkyle/elasticsearch
that referenced
this pull request
Sep 5, 2024
Similar to `ChunkedZipResponse` (elastic#109820) this utility allows Elasticsearch to send an `XContent`-based response constructed out of a sequence of `ChunkedToXContent` fragments, provided in a streaming and asynchronous fashion. This will enable elastic#93735 to proceed without needing to create a temporary index to hold the intermediate results.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to
ChunkedZipResponse(#109820) this utility allowsElasticsearch to send an
XContent-based response constructed out of asequence of
ChunkedToXContentfragments, provided in a streaming andasynchronous fashion.
This will enable #93735 to proceed without needing to create a temporary
index to hold the intermediate results.