Re: [phusion-passenger] Re: how to correctly gc between requests?

[Hongli Lai]

Hi Paul.

Yeah what you've found is basically right. GC between requests - or
Out Of Band GC as Unicorn calls it - can be implemented this way. But
there are a few problems:

- Pinging the process after every request is expensive.
- It only works correctly for single-threaded processes. Phusion
Passenger Enterprise 4 is going to support multithreading, and calling
the GC pauses the entire process.
- Your code is for Phusion Passenger 3, but Phusion Passenger 4 is
coming and the entire request handling core has been rewritten to be
evented.

I'm thinking about an alternative approach. Phusion Passenger 4's
ApplicationPool supports the notion of disabling processes, meaning
that a process is temporarily made ineligible for processing requests.
Thus out of band GC can be implemented as follows:
1. Return an "X-Passenger-Request-OOB-Work: true" header in the response.
2. Upon detecting this header, the HelperAgent should disable the process.
3. When the process's session count has dropped to 0 (meaning it's no
longer doing any more work), send an "OOB work" request to the
process. The process can then run the GC or doing whatever it wants to
do.
4. Upon completion of this request, Phusion Passenger should re-enable
the process.

What do you think?

On Fri, Sep 21, 2012 at 9:03 PM, Paul <pkm...@gmail.com> wrote:
> I dug into Passenger code further ... more specifically HelperAgent's
> handleRequest. Basically there are two sockets involved: clientFd and
> connection to the ruby process called "session". The method ends up reading
> the response from the "session" and writing it to the clientFd. It stops
> reading once the "session" is closed and it then closes the clientFd. As
> soon as "session" is closed, that ruby process is returned to the pool of
> available workers.
>
> I coded up a quick proof of concept ... the basic idea is to close the
> clientFd and then ping the ruby process before return that process back to
> the pool (https://gist.github.com/3762996). The ping is already something
> that's implemented by passenger.
>
> How does this general approach look? The extra ping is obviously not free
> but is preferable (at least in my case) to the user waiting for a gc.
>
> I'm going to clean this up and perhaps make a pull request.
>
> Any thoughts Hongli?
>
>
> On Thursday, September 20, 2012 10:43:39 AM UTC-7, Paul wrote:
>>
>> The idea is to avoid running GC during requests and instead run GC after
>> the request has finished (and before the next request starts). This
>> effectively hides the GC from end-users at the expense of more workers.
>>
>> There has been some discussion (see below) on how to achieve this and the
>> approaches fall into basic categories:
>>
>> Insert GC.start at the very end of
>> PhusionPassenger::AbstractRequestHandler.accept_and_process_next_request
>> after the connection is closed.
>> Insert GC.start in
>> PhusionPassenger::AbstractRequestHandler.accept_and_process_next_request
>> after process_request but before the connection is closed.
>>
>> However, there are problems with both approaches (you can substitute
>> GC.start with sleep 10 to accentuate the problems):
>>
>> The client sees the request finish normally and is not slowed down by
>> GC. However, it appears that once the connection is closed, passenger
>> considers that worker available and forwards it the next request. The worker
>> can't handle the request right away as it is busy doing GC from the last
>> request. People say this shows up as "queuing time".
>> The client sees the GC as part of the request and will not proceed to
>> the next request until the connection is closed.
>>
>> Is there any other way to get this implemented correctly? Can something
>> like this be worked into official passenger?
>>
>> http://pulse.sportngin.com/news_article/show/156863
>>
>> http://stackoverflow.com/questions/6221942/is-there-an-easy-way-to-run-garbage-collection-outside-of-the-request-cycle-in-p
>> https://groups.google.com/d/topic/phusion-passenger/5-geZAGcLKY/discussion
>>
> --
> You received this message because you are subscribed to the Google Groups
> "Phusion Passenger Discussions" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/phusion-passenger/-/8DSZB2nlA9YJ.
>
> To post to this group, send email to phusion-...@googlegroups.com.
> To unsubscribe from this group, send email to
> phusion-passen...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/phusion-passenger?hl=en.



--
Phusion | Ruby & Rails deployment, scaling and tuning solutions

Web: http://www.phusion.nl/
E-mail: in...@phusion.nl
Chamber of commerce no: 08173483 (The Netherlands)

[Hongli Lai]

On Tue, Sep 25, 2012 at 7:38 PM, Paul <pkm...@gmail.com> wrote:
> The general approach is much better.
>
> How do you imagine the app side working? Middleware (something like
> https://gist.github.com/3783149)? some sort of interface inside of passenger
> that can be implemented by the app (where passenger can ask oob_work_needed?
> and then invoke do_oob_work)? People would probably want to monitor their
> oob work (how many times invoked, how long it takes) ... so middleware would
> make that easy (i.e. newrelic middleware could be in front of the oob
> middleware and presumably something similar is possible wwth union station).
> At the same time, I wouldn't want my oob endpoint to be invokable by just
> anyone (i.e. my firewall is configured to disallow external ips from hitting
> anything with /private prefix, so perhaps the oob work url should be
> configurable).

I'm thinking about building upon the existing event API. You could then do

PhusionPassenger.on_event(:oob_work) do
...
end

The OOB endpoint would not be invokable by anyone, just by Phusion
Passenger. Application processes listen on a password-protected Unix
domain socket so these sockets are 1) only accessible from the local
machine and 2) only accessible by Phusion Passenger because only it
knows the password.

> My GC issue is somewhat urgent, so I'm gonna use that ping approach for now.
> However, I can re-work / backport the 4.0 style oob work functionality to 3
> (if you think that would be useful).
>
> Thanks for your thoughts.

Sure. By the way, we require contributors to sign our contributor's
agreement before we can merge their patches:
http://www.phusion.nl/forms/contributor_agreement

[Hongli Lai]

On Tue, Sep 25, 2012 at 8:20 PM, Paul <pkm...@gmail.com> wrote:
> Make sense.
>
> Yeah, I've signed that in the past. I can do it again though.

Ah I see. If you've already signed it then it's fine. :)

[Hongli Lai]

On Wed, Sep 26, 2012 at 8:19 AM, Paul <pkm...@gmail.com> wrote:
> This is what I have so far,
>
> https://github.com/pkmiec/passenger/commit/756b32331a93893ad2833495e189c3845655d19e
>
> To keep things simple, the X-Passenger-Request-OOB-Work only works correctly
> when using global queue. To make it work in the general case it seems you'd
> need all that stuff with disabling the process and waiting for session count
> to drop to zero.

Sure, an interim solution is fine.

> I'd like to really remove the X-Passenger-Request-OOB-Work header from being
> sent back to the client. I could find an example of that being done, so I do
> not know if you have a preference on how to do that. My idea was to just
> compute start of header, end of header (after \r\n), memmove header.data,
> and adjust header.size.

Yes, that's how you remove a header.

[Paul]

Just closing the loop on this thread.

With help from Hongli, I implemented a patch for Passenger 3 and Passenger 4. You can read more about it at http://engineering.appfolio.com/2012/11/19/dont-make-your-users-wait-for-gc.