Fix - OutOfMemory error in StreamHandler #1578
Conversation
|
@Seldaek sorry to bother you. Can you help me with the phpstan errors I get in this PR? |
|
Feel free to ignore this for now, I need to find time to review the rest of the PR first, then will happily help with that. |
Thanks for Your quick response. |
|
@Seldaek your last change fixed the problem I had on this pull request during the automatic checks. Thanks. |
| try { | ||
| stream_set_chunk_size($this->stream, $this->streamChunkSize); | ||
| } catch (\Exception $exception) { | ||
| throw new \RuntimeException('Impossible to set the stream chunk size.' | ||
| .PHP_EOL.'Error: '.$exception->getMessage() | ||
| .PHP_EOL.'Trace: '.$exception->getTraceAsString()); | ||
| } |
There was a problem hiding this comment.
Why rethrowing here and not simply letting the original exception do its thing?
There was a problem hiding this comment.
I'll revert this change for now as I don't understand it and I'd like to get the rest merged, if you think it's important please explain and we can do it in another PR.
| /* | ||
| * At this point, stream_set_chunk_size() failed in the constructor. | ||
| * Probably because not enough memory. | ||
| * The rest of th test depends on the success pf stream_set_chunk_size(), that is why |
There was a problem hiding this comment.
several typos in that sentence
… and use lower amount of chunk size based on memory limit
Seldaek
force-pushed
the
fix-oom-streamhandler
branch
from
b5e0383 to
b2d6af6
Compare
Seldaek
force-pushed
the
fix-oom-streamhandler
branch
from
b2d6af6 to
70fe092
Compare
|
cc @dsch this reduces the chunk size again, but IMO it'll still be large enough in most cases to fit in enough data to make writes atomic Thanks @juan-morales for the PR! I tweaked things a bit to make it all simpler and not dependent on the current memory usage, will let you take a look at my commit. |
Fixes Issue #1577
This change is meant to prevent setting a stream chunk size bigger than the available amount of memory.
The default value is still set, but can get change after calculations based on the php settings and the memory currently used by the php process.
There is a hard limit for an stream size of 2G ... is does not matter if the PHP process gets 1000 G of memory, the stream size cannot be bigger than 2G! and I suspect is because of some C lib used behind the scenes to implement streams in PHP.
Also ... the official PHP documentation states that
stream_set_chunk_size()can hold a value of PHP_INT_MAX , but this is not completely true, because no matter what, it cannot be bigger than 2G! (because of the reason I already mentioned.I suspect that if maybe the libs used in PHP are compiled with a 64 bit version... maybe the size can get bigger than 2G ... but just to be safe ... I would prefer to keep it in 2G.
This topic is quite tricky, please, someone elses opinion on this? ... thanks in advance
Ref: PHP official documentation for stream_set_chunk_size()