Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed

Conversation

miguilimzero
Copy link
Contributor

I would like to do a follow-up on #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 #648.

The #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: dd3263b.

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
Copy link
Member

Just use the configuration option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants