Closed Bug 532318 Opened 10 years ago Closed 10 years ago

[Windows] Apache2::SizeLimit (for mod_perl) doesn't work on Windows

Categories

(Bugzilla :: Bugzilla-General, defect)

3.5.2
defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 3.6

People

(Reporter: mockodin, Assigned: mockodin)

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v1 (obsolete) — Splinter Review
Syntax error on line 81 of C:/Apache2.2/conf/httpd.conf:
Apache2::SizeLimit at the moment works only with non-threaded MPMs at C:/usr/site/lib/Apache2/SizeLimit.pm line 51.
BEGIN failed--compilation aborted at C:/usr/site/lib/Apache2/SizeLimit.pm line 98.
Compilation failed in require at D:/WebRoot/Bugzilla/mod_perl.pl line 34.
BEGIN failed--compilation aborted at D:/WebRoot/Bugzilla/mod_perl.pl line 34.
Compilation failed in require at (eval 4) line 1.


PerlDoc of Apache2::SizeLimit
At this time, Apache2::SizeLimit does not support use under threaded
MPMs.  This is because there is no efficient way to get the memory
usage of a thread, or make a thread exit cleanly.

This should not mean be taken to mean that mod_perl will not function on windows. Rather that extra care must be taken when setting up apache config settings. The main consideration id noted in bug 370370 comment 20 and 21

The consideration is the proper setting for MaxRequestsPerChild. Except on heavily used servers this setting can probably be left as default. Reducing the max requests will result in apache killing and recreating child threads more often. Given enough memory and Server/MaxClient ratio the impact should be relatively low perhaps not noticeable in the right environment/configuration. This is not a constant however and *is* always subject to hardware and environment considerations.

The attached patch checks for a multi-threaded environment in the same manner as Apache2::SizeLimit. If a threaded apache environment is detected then Apache2::SizeLimit and its related code will not be loaded by mod_perl.pl
Attachment #415559 - Attachment is patch: true
Attachment #415559 - Attachment mime type: application/octet-stream → text/plain
Attachment #415559 - Flags: review?(mkanat)
Assignee: general → mockodin
Status: NEW → ASSIGNED
Comment on attachment 415559 [details] [diff] [review]
v1

  This is great, it was a really good design decision to check it on Apache2::MPM. My only concern is that people will start using Worker on Linux, which will cause total and strange failure on Bugzilla's part, and is likely to cause a very difficult and confusing support burden. So we might want to go back to checking ON_WINDOWS instead, for that reason. (You can just load Bugzilla::Constants first.)

> use Apache2::ServerUtil;
>-use Apache2::SizeLimit;
>+use Apache2::MPM;
>+use if !Apache2::MPM->is_threaded() , 'Apache2::SizeLimit';
> use ModPerl::RegistryLoader ();
> use CGI ();
> CGI->compile(qw(:cgi -no_xhtml -oldstyle_urls :private_tempfiles
>@@ -49,12 +50,13 @@
> 
> # This means that every httpd child will die after processing
> # a CGI if it is taking up more than 70MB of RAM all by itself.
>-$Apache2::SizeLimit::MAX_UNSHARED_SIZE = 70000;
>+$Apache2::SizeLimit::MAX_UNSHARED_SIZE = 70000 if !Apache2::MPM->is_threaded();

  Let's put everything that has to do with Apache2::SizeLimit together in a BEGIN block with an "if" clause--does that sound workable?

>+my $size_limit = ( !Apache2::MPM->is_threaded() ? "PerlCleanupHandler  Apache2::SizeLimit" : '');

  If we don't have a $size_limit, we need to set MaxRequestsPerChild, because we can't guarantee how the user's Apache is set up. I think 25 or 50 would be a reasonable choice for the sort of memory problems we're trying to avoid. Another option is to use Apache2::Resource (does that exist for mod_perl2, and does it work on Windows?) on Windows to hard-limit the RAM usage of a Bugzilla process.

  r- just to think about these things.
Attachment #415559 - Flags: review?(mkanat) → review-
OS: Windows 7 → All
Hardware: x86 → All
Summary: [Windows] Skip loading Apache::SizeLimit when Apache2::MPM->is_threaded() returns true → [Windows] Apache2::SizeLimit (for mod_perl) doesn't work on Windows
Target Milestone: --- → Bugzilla 3.6
(In reply to comment #1)
> using Worker on Linux, which will cause total and strange failure on Bugzilla's part

I'm interested to know how worker under linux is different than under windows? I haven't seen any problems under a windows model? The only issue off the top of my head should be common to both which is non-threadsafe perl modules.

> If we don't have a $size_limit, we need to set MaxRequestsPerChild, because
we can't guarantee how the user's Apache is set up

Not sure we can do this since, can we test if they have already set a limit? Else we may override theirs. The default is 10000 requests, except for mpm_netware and mpm_winnt which is 0 aka infinite.

> Another option is to use Apache2::Resource
Appears Not:

PS D:\WebRoot\bugzilla> ppm install Apache2::Resource
No missing packages to install

PS C:\Apache2.2\bin> .\httpd.exe -t
Syntax error on line 81 of C:/Apache2.2/conf/httpd.conf:
Can't locate BSD/Resource.pm in @INC (@INC contains: D:/WebRoot/Bugzilla D:/WebRoot/Bugzilla/lib C:/usr/site/lib C:/usr/
lib C:/Apache2.2) at C:/usr/site/lib/Apache2/Resource.pm line 26.
BEGIN
failed--compilation aborted at C:/usr/site/lib/Apache2/Resource.pm line 26.
Compilation failed in require at D:/WebRoot/Bugzilla/mod_perl.pl line 34.
BEGIN failed--compilation aborted at D:/WebRoot/Bugzilla/mod_perl.pl line 34.
Compilation failed in require at (eval 4) line 1.
PS D:\WebRoot\bugzilla> ppm install BSD::Resource
ppm.bat install failed: Can't find any package that provides BSD::Resource
PS D:\WebRoot\bugzilla> ppm repo
┌────┬───────┬────────────────────────────────┐
│ id │ pkgs  │ name                           │
├────┼───────┼────────────────────────────────┤
│  1 │ 11731 │ ActiveState Package Repository │
│  2 │   294 │ theory58S                      │
└────┴───────┴────────────────────────────────┘
 (2 enabled repositories)

> I think 25 or 50 would be a reasonable choice for the sort of memory problems we're trying to avoid

My concern will be that it may be too aggressive as we are then constantly regenerating killing and regenerating threads. Also I imagine many installations share the Apache instance with other web apps that *may* have more then 25-50 elements to a page. The limit needs to be set to allow a single thread to handle all of one clients request. But I still go back to what if the user already has one set? And again under mpm_netware and mpm_winnt they do not expire unless overridden by design, be that good or bad.

I think at some point we are forced to advise the user but not enforce.
(In reply to comment #2)
> I'm interested to know how worker under linux is different than under windows?

  In the Windows MPM, mod_perl fakes a whole Perl interpreter environment for each thread, so thread safety isn't an issue. Worker in Linux is a real threaded environment, requires a threaded Perl (AFAIK), etc.

> I haven't seen any problems under a windows model? The only issue off the top
> of my head should be common to both which is non-threadsafe perl modules.

  Bugzilla itself is not thread-safe.

> Not sure we can do this since, can we test if they have already set a limit?
> Else we may override theirs. The default is 10000 requests, except for
> mpm_netware and mpm_winnt which is 0 aka infinite.

  Well, those are certainly bad defaults for us, though fine for serving static files.

  I know that you have a lot of Windows and Perl expertise where you work, but the vast majority of users installing Bugzilla on Windows have little to no idea what they're doing, and so it's safer to just always set a sensible default that works for us. People who know what they're doing can edit mod_perl.pl, and we can mention that in the documentation as a side note or something.

> > I think 25 or 50 would be a reasonable choice for the sort of memory problems we're trying to avoid
> 
> My concern will be that it may be too aggressive as we are then constantly
> regenerating killing and regenerating threads. 

  That's fine, since a thread is very easy to regenerate. The way that Bugzilla currently works on Linux, it actually ends up killing the server after every single mod_perl request (and thus spawning a whole new *process*), and it can still survive a slashdotting on bugzilla.mozilla.org.

> Also I imagine many
> installations share the Apache instance with other web apps that *may* have
> more then 25-50 elements to a page.

  Though we recommend that KeepAlive be turned off, so those would be going to another thread anyway, if they happen simultaneously.

  Also, most Bugzilla installations on Windows don't share Apache with anything, in my long consulting experience.

> I think at some point we are forced to advise the user but not enforce.

  If Bugzilla was super-easy to install and we had highly educated users, we could require them to do one tiny additional thing to get a system that wouldn't crash them. As it is, Bugzilla is extremely difficult for the average Windows admin to install and we can't ship a system that will crash them *by default*. So we need to ship secure, and then they can make modifications if they need to. As I said, we can include notes in the documentation that if they want to fix MaxRequestsPerChild, they can do it immediately after PerlConfigRequire, but that we don't recommend it unless they really know what they're doing.
Fair enough.

I'll see what I can come up with.
Attached patch v2 (obsolete) — Splinter Review
Sets MaxChildRequests = 25 IF ON_WINDOWS

Might be an issue if we still support Apache 1.3 which had a non-threaded version for windows as I recall.
Attachment #415559 - Attachment is obsolete: true
Attachment #420854 - Flags: review?(mkanat)
(In reply to comment #5)
> Might be an issue if we still support Apache 1.3 which had a non-threaded
> version for windows as I recall.

  We don't; we only support mod_perl2.
(In reply to comment #6)
>   We don't; we only support mod_perl2.

Excellent, this bug *should* be good to go then.
Attachment #420854 - Flags: review?(mkanat) → review-
Comment on attachment 420854 [details] [diff] [review]
v2

>+use constant ON_WINDOWS => ($^O =~ /MSWin32/i);

  You don't need to do that. Just use Bugzilla::Constants first.

>+my $maxrequests = ( ON_WINDOWS ? "MaxRequestsPerChild 25" : '');;

  Nit: double-semicolons. Also, you don't need the parens in this statement or the one immediately above it.

  Otherwise this seems to look OK.
Attached patch v3Splinter Review
Moved Apache2::SizeLimit later in the script, rather than move Bugzilla::Constants
This groups most of the Apache2::SizeLimit references into a single block, and leaves the Bugzilla modules grouped as well.

This of course can be reversed if necessary.
Attachment #420854 - Attachment is obsolete: true
Attachment #421115 - Flags: review?(mkanat)
Attachment #421115 - Flags: review?(mkanat) → review+
Comment on attachment 421115 [details] [diff] [review]
v3

>+use if !Bugzilla::Constants->ON_WINDOWS

  Should be Bugzilla::Constants::ON_WINDOWS instead.

>+my $sizelimit = ( !Bugzilla::Constants->ON_WINDOWS ? "PerlCleanupHandler  Apache2::SizeLimit" : '');

  The parens are unnecessary. (Also, I suspect that these lines are longer than 80 chars.)

  Everything else looks fine, so I'll fix all this stuff on checkin.
Flags: approval+
Okay, did a bit of cleanup on checkin.

Checking in mod_perl.pl;
/cvsroot/mozilla/webtools/bugzilla/mod_perl.pl,v  <--  mod_perl.pl
new revision: 1.15; previous revision: 1.14
done
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Keywords: relnote
Added to the release notes in bug 547466.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.