[Bug 470696] Review Request: rubygem-passenger - Passenger Ruby on Rails deployment system

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=470696

--- Comment #64 from Mohammed Morsi <mmorsi@xxxxxxxxxx> 2010-09-14 12:53:29 EDT ---
(In reply to comment #63)
> Passenger 3's Boost is actually based on 1.42.0. A lot of API breakages have
> happened between Boost 1.35 and 1.42. I believe it's impossible to support
> multiple Boost versions without making our code a mess with #ifdefs everywhere.
> We only support one specific version of Boost for the time being: the one we
> use during development.

Can you please elaborate on this. Looking at the passenger repository, it seems
boost is still marked as being on version 1.35

http://github.com/FooBarWidget/passenger/blob/master/ext/boost/VERSION.TXT

and the last commit I see relating to 'updating boost' is the update to 1.35

http://github.com/FooBarWidget/passenger/commits/master/ext/boost

At what point did you update to 1.42? In any case for this submission, boost is
still at 1.35 which I hope isn't so incompatible with 1.44 (shipping on F14)
that it won't compile against it.


> 
> The condition_variable and mutex error checks do not retry on all errors: they
> only retry on EINTR. BOOST_VERIFY aborts even on EINTR but EINTR is usually not
> fatal, it just means you need to try later. This issue bit us on one of our
> customers' production systems.

This is good to know that this was to solve an edge case on a production
system. Which OS was this customer running btw? If it something other than
Linux (and maybe even if it's another distro) we might not run into this onto
Fedora and thus this can be omitted.


> 
> Exception hierarchy change: yes they're informational. If a thread fails to
> spawn (i.e. thread_resource_error is thrown) then we'll want to show a
> backtrace. This is not possible without patching Boost and changing the ABI.
> Please do not remove TRACE_POINTs. They're an essential part of our system
> inspection features. Right now it's possible to query the backtrace of all
> threads in Phusion Passenger during runtime thanks to the TRACE_POINTs. If you
> get rid of that then our users will lose their ability to see what's going on
> and will introduce a lot of headaches when things go wrong.

I would like to include this (and everything else), but it looks like there is
no way to include passenger in Fedora without this being removed (eg passenger
needs to work against the stock boost). So its either remove this debugging
stuff or not ship passenger at all. Like you said in a previous comment, if
anyone wants any of these features I'm removing they can simply get passenger
via gem.


> 
> Thread constructor: it is not unnecessary, we do this intentionally to reduce
> the VM size. We create a lot of threads. The default thread stack size on Linux
> is 8 MB, so unless we reduce the stack sizes we can't create many threads and
> the VM size will grow very large, giving users the impression that we're memory
> hungry.

Have you guys ever considered using thread pools? Launching many threads
indiscriminately is often considered bad practice, and you may get a
performance boost by making use of a pool. Regardless its good to know that
this is simply to make the memory footprint smaller, and it should work without
this. Just curious is there any way to reduce the default thread stack size,
systemwide lets say, without modifying/recompiling every program?


I'm starting to hack at this to see if I can get it working. Based on your
response the only thing that makes me not sure if this will work is the
discrepancy between boost versions, but we'll just see if that poses and issue.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]