Skip to content

Update listen backlog on reload and stop enforcing default backlog #12977

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Update listen backlog on reload and stop enforcing default backlog #12977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1851e77
Add support for adjusting the listen backlog when reloading php-fpm and
vnsavage Dec 19, 2023
8fabfe8
Update the messaging about the backlog being changed
vnsavage Dec 19, 2023
c203b8f
Fix typo with wp->config- >name
vnsavage Dec 20, 2023
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions sapi/fpm/fpm/fpm_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -915,8 +915,7 @@ static int fpm_conf_process_all_pools(void)
}

if (config->listen_backlog < FPM_BACKLOG_DEFAULT) {
zlog(ZLOG_WARNING, "[pool %s] listen.backlog(%d) was too low for the ondemand process manager. I updated it for you to %d.", wp->config->name, config->listen_backlog, FPM_BACKLOG_DEFAULT);
config->listen_backlog = FPM_BACKLOG_DEFAULT;
zlog(ZLOG_WARNING, "[pool %s] listen.backlog(%d) is too low for the ondemand process manager. I suggest updating it to at least %d.", wp->config->name, config->listen_backlog, FPM_BACKLOG_DEFAULT);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably verify why this is an issue for ondemand mode and test it with lower listen backlog. This was added together with ondemand mode from checking the changes so there might be a reason for adding it. There should be also a test added for setting it low and using ondemand. I would prefer to separate it to a new PR as it's unrelated to other change.
}

/* certainly useless but proper */
Expand Down
1 change: 1 addition & 0 deletions sapi/fpm/fpm/fpm_sockets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ static int fpm_sockets_get_listening_socket(struct fpm_worker_pool_s *wp, struct

sock = fpm_sockets_hash_op(0, sa, 0, wp->listen_address_domain, FPM_GET_USE_SOCKET);
if (sock >= 0) {
listen(sock, wp->config->listen_backlog); /* change backlog via listen() */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For correctness we should check the return value of listen even though it is unlikely to fail. This is actually good to verify the we got a correct socket.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be also good to have a test that does reload and changes the configuration to make that covered.
return sock;
}

Expand Down