Allow Apache2::SizeLimit to be configured via params

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
General
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: gozer, Assigned: dylan)

Tracking

Production
Dependency tree / graph

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

2 years ago
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.

Updated

2 years ago
Attachment #8680632 - Flags: review?(glob)
Comment on attachment 8680632 [details] [diff] [review]
[POC] use configuration parameter to set Apache::SizeLimit2 treshold

POCs don't need review.
Attachment #8680632 - Flags: review?(glob)

Updated

2 years ago
Attachment #8680632 - Flags: feedback?(glob)
Attachment #8680632 - Flags: feedback?(glob) → feedback?(dylan)
(Assignee)

Comment 2

2 years ago
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).
Attachment #8680632 - Flags: feedback?(dylan) → feedback+
(Assignee)

Updated

2 years ago
See Also: → bug 731589
(Assignee)

Comment 3

2 years ago
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.
(Assignee)

Updated

2 years ago
Assignee: nobody → dylan
(Assignee)

Comment 4

2 years ago
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?
Attachment #8680632 - Attachment is obsolete: true
Attachment #8680743 - Flags: review?(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);
Attachment #8680743 - Flags: review?(dkl) → review-
(Assignee)

Comment 6

2 years ago
Created attachment 8681967 [details] [diff] [review]
1219750_2.patch
Attachment #8680743 - Attachment is obsolete: true
Attachment #8681967 - Flags: review?(dkl)
Comment on attachment 8681967 [details] [diff] [review]
1219750_2.patch

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

Works good. r=dkl
Attachment #8681967 - Flags: review?(dkl) → review+
(Assignee)

Comment 8

2 years ago
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   b5271ca..338bad4  master -> master
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Comment 9

2 years ago
Woops. Note: always re-apply the patch.

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

Updated

a year ago
Blocks: 1283664
You need to log in before you can comment on or make changes to this bug.