Skip to content

Commit

Permalink
Use new API to get and set FPM config values
Browse files Browse the repository at this point in the history
  • Loading branch information
jcameron committed Sep 3, 2022
1 parent 69f46ca commit 996cb0e
Showing 1 changed file with 4 additions and 22 deletions.
26 changes: 4 additions & 22 deletions virtual_feature.pl
Original file line number Diff line number Diff line change
Expand Up @@ -1497,15 +1497,8 @@ sub feature_get_web_php_children
# Read from FPM config file
my $conf = &virtual_server::get_php_fpm_config();
return -1 if (!$conf);
my $file = $conf->{'dir'}."/".$d->{'id'}.".conf";
my $lref = &read_file_lines($file, 1);
my $childs = 0;
foreach my $l (@$lref) {
if ($l =~ /pm.max_children\s*=\s*(\d+)/) {
$childs = $1;
}
}
&unflush_file_lines($file);
my $childs = &virtual_server::get_php_fpm_pool_config_value(
$conf, $d->{'id'}, "pm.max_children");
return $childs == $childrenmax ? 0 : $childs;
}
else {
Expand Down Expand Up @@ -1535,20 +1528,9 @@ sub feature_save_web_php_children
# Set in the FPM config
my $conf = &virtual_server::get_php_fpm_config();
return 0 if (!$conf);
my $file = $conf->{'dir'}."/".$d->{'id'}.".conf";
return 0 if (!-r $file);
&lock_file($file);
my $lref = &read_file_lines($file);
$children = $childrenmax if ($children == 0); # Recommended default
foreach my $l (@$lref) {
if ($l =~ /pm.max_children\s*=\s*(\d+)/) {
$l = "pm.max_children = $children";
}
}
&flush_file_lines($file);
&unlock_file($file);
&virtual_server::register_post_action(
\&virtual_server::restart_php_fpm_server);
&save_php_fpm_pool_config_value(
$conf, $d->{'id'}, "pm.max_children", $children);
}
&virtual_server::save_domain($d);
}
Expand Down

4 comments on commit 996cb0e

@iliajie
Copy link
Contributor

@iliajie iliajie commented on 996cb0e Sep 3, 2022

Choose a reason for hiding this comment

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

This is nice that we have API for this!

Later, we could use it to tweak pm.max_spare_servers depending on passed pm.max_children value, to allow user values for pm.max_children lower than 5. Do you think we should it?

@jcameron
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's keep things simple - I suspect the majority of users don't need this level of fine-grained control over PHP subprocesses.

@swelljoe
Copy link
Collaborator

Choose a reason for hiding this comment

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

All I wanted was a reasonable default!

@iliajie
Copy link
Contributor

@iliajie iliajie commented on 996cb0e Sep 3, 2022

Choose a reason for hiding this comment

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

Agreed. Defaults are reasonably good now. Although, if a user starts changing pm.max_children option then pm.max_spare_servers should also be adjusted.

Please sign in to comment.