Skip to content

FPM: Change last_idle_child lookup to fix idle time computation - #4104

Closed
mpdude wants to merge 1 commit into
php:PHP-7.2from
mpdude:find-longest-idle-child
Closed

mpdude wants to merge 1 commit into
php:PHP-7.2from
mpdude:find-longest-idle-child

Conversation

@mpdude

@mpdude mpdude commented May 3, 2019

Copy link
Copy Markdown

Currently, the last_idle_child is determined by looking at the start time. That is, it is the child that has been around for the longest period of time.

A few lines later, for pm = ondemand the idle time is computed for this last_idle_child to decide if it should be terminated.

Thus, I think we need to determine the last_idle_childby looking at the time of the last request served, not the startup time.

Currently, the "last_idle_child" is determined by looking at the
starting time, that is, it is the child that has been around for
the longest time.

A few lines later, for pm = ondemand the idle time is computed for
this last_idle_child to decide if it has been idle too long and
should be terminated.

To make this calculation correct, we need to find the "last_idle_child"
by looking at the time of the last request served, not the startup
time.
@nikic
nikic requested a review from bukka May 7, 2019 08:23
@krakjoe

krakjoe commented May 25, 2019

Copy link
Copy Markdown
Member

@bukka can you take a look at this one please ?

@bukka

bukka commented May 26, 2019

Copy link
Copy Markdown
Member

I think this would make sense if we had FIFO selection as discussed in #4101 . However I'm not sure if it makes much difference for the round robin selection (accept).

@mpdude Have you seen any improvements with this patch when testing?

@cmb69

cmb69 commented Jun 12, 2019

Copy link
Copy Markdown
Member

@mpdude, any news?

@mpdude

mpdude commented Jun 13, 2019

Copy link
Copy Markdown
Author

No, I haven’t done any performance research. I am not even sure how I would do this and what to look out for 🤔

I found this while studying the code and wanted to bring it to the attention of someone who knows better than me 🤷🏻‍♂️

@ramsey

ramsey commented May 27, 2022

Copy link
Copy Markdown
Member

@mpdude Are you interested in pursuing this PR? If not, please feel free to close it. Thanks!

@github-actions

Copy link
Copy Markdown

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants