Hi Chris & Mike,

I submitted a patch on Sunday.  I just wanted to check if it got caught by a
spam filter or if you guys didn't get a chance to look at it yet.

Since then, I thought of one more idea to improve it and wanted to get your
feedback.  The way the patch works now is that a reference count is keep at
the FastCGIProcessGroup level and when each fcgi subprocess in spawned and
reaped, the increments or decrements the reference count.  When the count
hits zero, the socket is closed and when it goes from zero to one, it gets
created again.

The downside to this scheme is that if you restart an fcgi process group,
there will be a period of time where the socket is down, which can cause
client to to get errors during that time.  The idea I had is that we really
only need to tear down the socket when the group config is changed during a
reload.  I believe that this always happens though a called to the
stopProcessGroup() method in the RPC interface.  If so, we can have that
method trigger a callback on the process group (something like
ProcessGroup.stop_requested()).  The base class does not need to do anything
in that method but the FastCGIProcessGroup can set a flag that shutdown has
been signaled and will close the fcgi socket when that flag is set and the
reference count hits zero.

If you agree with that concept, that should be a pretty simple change and I
can submit a new patch.  Thanks,

Roger

On Sun, Aug 16, 2009 at 10:26 PM, Roger Hoover <[email protected]>wrote:

> Thanks, Mike.  I have a patch (see attached) that addresses the following
> issues:
>
> 1) Reloading the config for an fcgi process group did not close the fcgi
> socket - although the fcgi socket is a shared resource at the fcgi process
> group level, it can no longer be managed in the FastCGIProcessGroup class
> because that class has no way to get an event when all it's processes have
> been reaped (which is when the socket needs to be closed).  So, I had to
> implement a reference counting scheme and override the Subprocess.spawn()
> and Subprocess.finish() methods.
> 2) Rereading the config did not pick up changes to the socket parameter in
> a fcgi-program section - this was a simple fix requiring a little
> customization of the __eq__() method
>
> I wrote unit tests for the new code and also tested the reread/reload
> functionality with an integration test.  Let me know if you find any issues.
>
> Roger
>
>
> On Sun, Aug 16, 2009 at 1:02 PM, Mike Naberezny <[email protected]>wrote:
>
>> Hi Roger,
>>
>> Roger Hoover wrote:
>>
>>> Chris or Mike, after briefly looking over the code, it looks like the
>>> place to do this is to override the finish() method like this.
>>>
>>> class FastCGISubprocess(Subprocess):
>>>
>>>    def finish(self, pid, sts):
>>>        #do FCGI Socket clean up here
>>>         ...
>>>        #do parent finish()
>>>        Subprocess.finish(self, pid, sts)
>>>
>>> Does this seem like the right approach?
>>>
>>
>> This looks good to me.
>>
>> Thanks,
>> Mike
>>
>> --
>> Mike Naberezny
>> Maintainable Software
>> http://maintainable.com
>>
>
>
_______________________________________________
Supervisor-users mailing list
[email protected]
http://lists.supervisord.org/mailman/listinfo/supervisor-users

Reply via email to