Last Comment Bug 1219750 - Allow Apache2::SizeLimit to be configured via params
: Allow Apache2::SizeLimit to be configured via params
Status: RESOLVED FIXED
:
Product: bugzilla.mozilla.org
Classification: Other
Component: General (show other bugs)
: Production
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: Dylan Hardison [:dylan]
:
:
Mentors:
Depends on:
Blocks: 1227031 1283664
  Show dependency treegraph
 
Reported: 2015-10-29 07:21 PDT by Philippe M. Chiasson (:gozer)
Modified: 2016-06-30 16:14 PDT (History)
3 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
[POC] use configuration parameter to set Apache::SizeLimit2 treshold (803 bytes, patch)
2015-10-29 07:21 PDT, Philippe M. Chiasson (:gozer)
dylan: feedback+
Details | Diff | Splinter Review
1219750_1.patch (2.19 KB, patch)
2015-10-29 10:46 PDT, Dylan Hardison [:dylan]
dkl: review-
Details | Diff | Splinter Review
1219750_2.patch (2.63 KB, patch)
2015-11-02 06:49 PST, Dylan Hardison [:dylan]
dkl: review+
Details | Diff | Splinter Review

Description User image Philippe M. Chiasson (:gozer) 2015-10-29 07:21:30 PDT
Created attachment 8680632 [details] [diff] [review]
[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 User image Byron Jones ‹:glob› 2015-10-29 07:36:58 PDT
Comment on attachment 8680632 [details] [diff] [review]
[POC] use configuration parameter to set Apache::SizeLimit2 treshold

POCs don't need review.
Comment 2 User image Dylan Hardison [:dylan] 2015-10-29 07:51:49 PDT
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 User image Dylan Hardison [:dylan] 2015-10-29 08:32:48 PDT
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 User image Dylan Hardison [:dylan] 2015-10-29 10:46:11 PDT
Created attachment 8680743 [details] [diff] [review]
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 User image David Lawrence [:dkl] 2015-10-30 14:36:16 PDT
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 User image Dylan Hardison [:dylan] 2015-11-02 06:49:05 PST
Created attachment 8681967 [details] [diff] [review]
1219750_2.patch
Comment 7 User image David Lawrence [:dkl] 2015-11-03 14:30:40 PST
Comment on attachment 8681967 [details] [diff] [review]
1219750_2.patch

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

Works good. r=dkl
Comment 8 User image Dylan Hardison [:dylan] 2015-11-03 19:09:51 PST
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   b5271ca..338bad4  master -> master
Comment 9 User image Dylan Hardison [:dylan] 2015-11-03 19:12:25 PST
Woops. Note: always re-apply the patch.

To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   338bad4..2776e92  master -> master

Note You need to log in before you can comment on or make changes to this bug.