laravel / octane

Supercharge your Laravel application's performance.
https://laravel.com/docs/octane
MIT License
3.74k stars 291 forks source link

[2.x] Use workers count to timer table size instead of 250 (or param) #824

Closed miguilimzero closed 7 months ago

miguilimzero commented 7 months ago

I would like to do a follow-up on https://github.com/laravel/octane/pull/819. This PR aims to make Octane work out of the box with a high number of workers, usually more than 200 workers.

The timer table is internally used by Octane to check if the worker is processing a request for more time than is allowed. Due to the hard-coded size number of 250, Octane starts to give some errors when there are too many workers processing requests at the same time, as reported at https://github.com/laravel/octane/issues/648.

The https://github.com/laravel/octane/pull/650 PR aimed to fix this by introducing the max_timer_table_size so we can manually control the timer size table on octane.php. Before the pull request was merged, this parameter was removed from the config file and remained un-documented: https://github.com/laravel/octane/pull/650/commits/dd3263b22e9f03d05bf51703d76ebfbc9e46c839.

In the previous PR, a question was raised as to why the parameter could not just be documented. By setting the table size as the number of workers, we can just make it work out of the box in all scenarios. This would make the parameter obsolete, as it is not needed anymore, and we would remove this default limitation of 250 on the timer table size, without doing any breaking change. Being a better solution than just documenting the current parameter.

Alternative

If the max_timer_table_size un-documented parameter is really necessary for any other reason that I'm not aware of. We can still use the workers count to get rid of the hard-coded number of 250. By doing something like:

$serverState['octaneConfig']['max_timer_table_size'] ?? $serverState['workers']
taylorotwell commented 7 months ago

Just use the configuration option.