1219750 - Allow Apache2::SizeLimit to be configured via params

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: General, defect)

Description

[Philippe M. Chiasson (:gozer)]

Attachment: [POC] use configuration parameter to set Apache::SizeLimit2 treshold

Right now, Apache2::SizeLimit->set_max_unshared_size is called in mod_perl.pl with a conditional based on urlbase.

Wouldn't it make more sense to turn that value into a configurable parameter in data/params, for instance?

Attached patch just illustrates what I mean.

Comment 1

[:glob ✱]

Comment on attachment 8680632 [details] [diff] [review]
[POC] use configuration parameter to set Apache::SizeLimit2 treshold

POCs don't need review.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8680632 [details] [diff] [review]
[POC] use configuration parameter to set Apache::SizeLimit2 treshold

Review of attachment 8680632 [details] [diff] [review]:
-----------------------------------------------------------------

f+ to the idea. I think this may belong in localconfig, with a sensible default value. 
(it seems similar to $su_exec or $webservergroup).

Comment 3

[Dylan Hardison [:dylan] (he/him)]

addressing bug 731589 -- I disagree with that decision. Putting in the database so it can be "scaled" is silly.
If Bugzilla didn't have so many memory leaks its memory usage would be flat and SizeLimit would not be needed,
or at least would not need to be highly dynamic. Working on a patch for this now.

Comment 4

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1219750_1.patch

We'll want to discuss deployment of this. What's required to make sure localconfig gets the right value for this on prod?

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 8680743 [details] [diff] [review]
1219750_1.patch

Review of attachment 8680743 [details] [diff] [review]:
-----------------------------------------------------------------

Other files look good to me.

::: mod_perl.pl
@@ +61,4 @@
>  # This means that every httpd child will die after processing a request if it
>  # is taking up more than 700MB of RAM all by itself, not counting RAM it is
>  # sharing with the other httpd processes.
> +Apache2::SizeLimit->set_max_unshared_size(Bugzilla->localconfig->{apache_size_limit});

So worry about SCL3, lets leave the old code in place and use it if the localconfig value does not exist:

my $apache_size_limit = '250_000';
if (Bugzilla->localconfig->{apache_size_limit}) {
    $apache_size_limit = Bugzilla->localconfig->{apache_size_limit};
}
elsif (Bugzilla->params->{'urlbase'} eq 'https://bugzilla.mozilla.org/') {
    $apache_size_limit = '700_000';
}
Apache2::SizeLimit->set_max_unshared_size($apache_size_limit);

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Attachment: 1219750_2.patch

Comment 7

[David Lawrence [:dkl]]

Comment on attachment 8681967 [details] [diff] [review]
1219750_2.patch

Review of attachment 8681967 [details] [diff] [review]:
-----------------------------------------------------------------

Works good. r=dkl

Comment 8

[Dylan Hardison [:dylan] (he/him)]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   b5271ca..338bad4  master -> master

Comment 9

[Dylan Hardison [:dylan] (he/him)]

Woops. Note: always re-apply the patch.

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   338bad4..2776e92  master -> master