Defect in zend_accel_schedule_restart_if_necessary

[Incognito]

Discovered this gem while working on a slow server with @alexlecca .

The server had lots of ram but ran out of keys available.

If the hash table is full ( https://github.com/zendtech/ZendOptimizerPlus/blob/8c3e56f83bf4fda5f6780618638c77ee43867a70/ZendAccelerator.c#L1111 ) then zend_accel_schedule_restart_if_necessary is called, however, it will only restart if out of memory ( https://github.com/zendtech/ZendOptimizerPlus/blob/8c3e56f83bf4fda5f6780618638c77ee43867a70/ZendAccelerator.c#L189 )

[dstogov]

Right. The cache may be populated with valid data. Restart may lead to reloading the same data and restart again. Just increase the number of hash slots through opcache.max_accelerated_files in php.ini

Thanks. Dmitry.

On Thu, Aug 6, 2015 at 9:16 PM, Brian Graham notifications@github.com wrote:

Discovered this gem while working on a slow server with @alexlecca https://github.com/alexlecca .

The server had lots of ram but ran out of keys available.

If the hash table is full ( https://github.com/zendtech/ZendOptimizerPlus/blob/8c3e56f83bf4fda5f6780618638c77ee43867a70/ZendAccelerator.c#L1111 ) then zend_accel_schedule_restart_if_necessary is called, however, it will only restart if out of memory ( https://github.com/zendtech/ZendOptimizerPlus/blob/8c3e56f83bf4fda5f6780618638c77ee43867a70/ZendAccelerator.c#L189 )

— Reply to this email directly or view it on GitHub https://github.com/zendtech/ZendOptimizerPlus/issues/216.

[Incognito]

That makes sense to me. My specific use case made me thing about things differently.

Doesn't the same problem for key limits exist for memory limits?

[alexlecca]

Hi Dmitry, Incognito,

Wouldn't it make more sense to allow for the restart since even the status functionality reports on it? http://php.net/manual/en/function.opcache-get-status.php documenting three types of restarts ["oom_restarts"]=> int(0) ["hash_restarts"]=> int(0) ["manual_restarts"]=>

Also, it would seem that the worst case if the restart is triggered is having it happen often rather than the worst if it doesn't happen is running on completely uncached code.

Regardless of this, increasing the hash pool is obviously the way to go from an implementation point of view and I am not trying to adresse that issue in this way - just what happens when we do put the wrong configuration in.

[Incognito]

I think the semantics of the code are confusing this issue.

We call the restart option when the keys are full, but prevent a restart because we don't care about the keys being full (because they may or may not refill the same way depending on your use case), but that isn't very different from the use case where we restart if the memory is full. At least, I don't really see the difference: the memory can fill back up the same way the keys can after a reboot.

[dstogov]

Lets say we have two hash slots and access 3 files in sequence. If we restart SHM after hash is full, we will reload it on each 3 request. This means we won't benefit from caching at all.

PHP7 got additional (experimental) file cache, that may help in this case.

On Tue, Aug 11, 2015 at 4:56 AM, Brian Graham notifications@github.com wrote:

I think the semantics of the code are confusing this issue.

We call the restart option when the keys are full, but prevent a restart because we don't care about the keys being full (because they may or may not refill the same way depending on your use case), but that isn't very different from the use case where we restart if the memory is full. At least, I don't really see the difference: the memory can fill back up the same way the keys can after a reboot.

— Reply to this email directly or view it on GitHub https://github.com/zendtech/ZendOptimizerPlus/issues/216#issuecomment-129672953 .

[alexlecca]

Shouldn't we in that case remove reporting of restarts due to hash keys?