Use chunked encoding for RestGetSettingsAction #90326
Conversation
This one needs chunked encoding as well, the response easily grows to O(10M) for large deployments with many indices. Especially when settings are numerous or have large values like e.g. settings from the Beats templates.
original-brownbear
added
>non-issue
:Distributed Coordination/Network
elasticsearchmachine
added
the
Team:Distributed (Obsolete)
|
Pinging @elastic/es-distributed (Team:Distributed) |
There was a problem hiding this comment.
LGTM.
I'd sort of prefer that we ensured not to implement ToXContent now, rather than later. This PR would (I think) need additional changes to accomodate that change. And as such we are building up debt that we'd need to tackle in the future rather than in individual PRs like this one.
| @@ -154,38 +144,44 @@ public String toString() { | |||
| try { | |||
| ByteArrayOutputStream baos = new ByteArrayOutputStream(); | |||
| XContentBuilder builder = new XContentBuilder(JsonXContent.jsonXContent, baos); | |||
| toXContent(builder, ToXContent.EMPTY_PARAMS, false); | |||
| var iterator = toXContentChunked(false); | |||
There was a problem hiding this comment.
If we think this can grow to 10M+, should we then not avoid assembling this fully here, i.e., limit the output to say 100 entries?
There was a problem hiding this comment.
Fair point, not entirely trivial to do that, would have to add more logic to achieve that. I wonder if it matters all that much, we have the same problem for the other large responses as well.
I'll try to address this in the next PR on this while dropping the toXContent from these objects also. Incoming :)
|
Thanks Henning! It's hard to drop |
This one needs chunked encoding as well, the response easily grows to O(10M) for large deployments with many indices. Especially when settings are numerous or have large values like e.g. settings from the Beats templates.
relates #89838