Fix RecyclerBytesStreamOutput allocating unlimited heap for some capacities#90632
Fix RecyclerBytesStreamOutput allocating unlimited heap for some capacities#90632original-brownbear merged 2 commits intoelastic:mainfrom original-brownbear:gigantic-recyler-unlucky
Conversation
…cities We'd allocate infinite memory here because we'd overflow to negative capacities for newPosition close to `Integer.MAX_VALUE`.
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @original-brownbear, I've created a changelog YAML for you. |
| // than Integer.MAX_VALUE | ||
| if (newPosition > Integer.MAX_VALUE - (Integer.MAX_VALUE % pageSize)) { | ||
| throw new IllegalArgumentException(getClass().getSimpleName() + " cannot hold more than 2GB of data"); | ||
| } |
There was a problem hiding this comment.
Should we convert org.elasticsearch.common.io.stream.RecyclerBytesStreamOutput#currentCapacity to long to prevent this overflow? It looks like it is not used anywhere else. Such change would also allow us to make above limit configurable when needed.
idegtiarenko
left a comment
There was a problem hiding this comment.
Left a suggestion, otherwise LGTM 👍
|
Thanks Ievgen, as discussed will address the suggestion in a follow-up. |
💔 Backport failed
You can use sqren/backport to manually backport by running |
|
I think this deserves a test and it doesn't look too hard to do so without actually allocating GBs of heap - see #90638. |
|
@original-brownbear this is labelled 7.17 but I don't think |
We'd allocate infinite memory here because we'd overflow to negative capacities for newPosition close to
Integer.MAX_VALUE.