You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Mladen Turk <mt...@apache.org> on 2012/05/07 18:24:24 UTC

Purpose of unlockAccept

Hi

Did some testing and removing unlockAccept works equally
fine (and in some circumstances pause/continue and even shutdown is much faster).
Using a simple socket close on stop/destroy and monitoring
for pause 'after' acceptSocket() by not handling single
connection received after pause eliminates the need for
unlockAccept entirely.
It also fixes various issues with disappearing network
stack, link-local addresses, and various hang-outs on
shutdown/pause.


Any objections that I commit that to trunk?
AFAICT this is not covered by any spec and the only
change is that a single connection that could get accepted
during pause phase can receive ECONNRESET instead ECONNREFUSED
in case pause continues beyond client socket timeout.


Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/08/2012 12:28 PM, Mark Thomas wrote:
> The only downside I can see is if someone wants zero backlog on
> pause. That is no longer going to be possible.

According to javadocs setting socket's backlog to zero means 'OS default'
so this is not reliable anyhow.


Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Mark Thomas <ma...@apache.org>.
On 08/05/2012 21:33, Mladen Turk wrote:
> On 05/08/2012 08:54 PM, Mark Thomas wrote:
>> On 08/05/2012 19:47, Costin Manolache wrote:
>>> You may still want to accept requests for existing sessions.
>>>
>>> Both 'graceful shutdown' and app deploy are important cases - it's just
>>> that current 'pause()' is not that good for either of them, if you
>>> really
>>> want to cleanup...
>>>
>>> Maybe an option to return 503 or custom status/headers - so people can
>>> adjust to various LBs.
>>
>> pause() on the connector means something very different to pause in the
>> reverse proxy. In the connector pause means:
>> - stop processing new connections
>> - close existing connections
> 
> So it closes existing connections or waits for response finish
> and closes the sockets?

If the request line has been processed, a request is allowed to complete
and then the connection is closed. If a connection is in keep-alive, the
connection is closed immediately.

Mark

> I always thought it's similar to graceful shutdown which
> would imply that it waits till all current req/resp
> cycles are finished (probably forcing keepAlive = false)
> 
>> - keep the socket bound
>>
>> It is very similar to stop() if bindOnInit==true
>>
> 
> 
> 
> 
> 
> Regards


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/08/2012 08:54 PM, Mark Thomas wrote:
> On 08/05/2012 19:47, Costin Manolache wrote:
>> You may still want to accept requests for existing sessions.
>>
>> Both 'graceful shutdown' and app deploy are important cases - it's just
>> that current 'pause()' is not that good for either of them, if you really
>> want to cleanup...
>>
>> Maybe an option to return 503 or custom status/headers - so people can
>> adjust to various LBs.
>
> pause() on the connector means something very different to pause in the
> reverse proxy. In the connector pause means:
> - stop processing new connections
> - close existing connections

So it closes existing connections or waits for response finish
and closes the sockets?
I always thought it's similar to graceful shutdown which
would imply that it waits till all current req/resp
cycles are finished (probably forcing keepAlive = false)

> - keep the socket bound
>
> It is very similar to stop() if bindOnInit==true
>





Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Mark Thomas <ma...@apache.org>.
On 08/05/2012 21:09, Costin Manolache wrote:
> On Tue, May 8, 2012 at 11:54 AM, Mark Thomas <ma...@apache.org> wrote:

>> pause() on the connector means something very different to pause in the
>> reverse proxy. In the connector pause means:
>> - stop processing new connections
>> - close existing connections
>> - keep the socket bound
>>
> 
> I know what it does - I don't know why. If you know the use case for
> pause(), it would be great to add it to the javadoc, I haven't found any (
> I hope it wasn't me to add it :-)
> 
> Anyways - I was mostly curious.

I know some folks do use it. I'm not entirely sure of the use case
myself. Very much before my time. If we can drop it entirely I'd be
happy to do that in 8 onwards.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
On Tue, May 8, 2012 at 11:54 AM, Mark Thomas <ma...@apache.org> wrote:

> On 08/05/2012 19:47, Costin Manolache wrote:
> > You may still want to accept requests for existing sessions.
> >
> > Both 'graceful shutdown' and app deploy are important cases - it's just
> > that current 'pause()' is not that good for either of them, if you really
> > want to cleanup...
> >
> > Maybe an option to return 503 or custom status/headers - so people can
> > adjust to various LBs.
>
> pause() on the connector means something very different to pause in the
> reverse proxy. In the connector pause means:
> - stop processing new connections
> - close existing connections
> - keep the socket bound
>

I know what it does - I don't know why. If you know the use case for
pause(), it would be great to add it to the javadoc, I haven't found any (
I hope it wasn't me to add it :-)

Anyways - I was mostly curious.

Costin


>
> It is very similar to stop() if bindOnInit==true
>
> I don't see a requirement for "process existing sessions but reject new
> sessions" and if there were such a requirement I'd implement that in a
> Valve (or maybe a Filter), not in the connector.



>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mark Thomas <ma...@apache.org>.
On 08/05/2012 19:47, Costin Manolache wrote:
> You may still want to accept requests for existing sessions.
> 
> Both 'graceful shutdown' and app deploy are important cases - it's just
> that current 'pause()' is not that good for either of them, if you really
> want to cleanup...
> 
> Maybe an option to return 503 or custom status/headers - so people can
> adjust to various LBs.

pause() on the connector means something very different to pause in the
reverse proxy. In the connector pause means:
- stop processing new connections
- close existing connections
- keep the socket bound

It is very similar to stop() if bindOnInit==true

I don't see a requirement for "process existing sessions but reject new
sessions" and if there were such a requirement I'd implement that in a
Valve (or maybe a Filter), not in the connector.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
On Tue, May 8, 2012 at 11:01 AM, Mladen Turk <mt...@apache.org> wrote:

> On 05/08/2012 07:09 PM, Costin Manolache wrote:
>
>> IMHO neither 'graceful shutdown' nor 'deploy' are best served by not
>> accepting connections:
>> - for 'graceful' - a number of connections will get to backlog and
>> timeout,
>>
>
> Not sure if ServerSocket.close() would cause cascade close of all
> accepted sockets that are still running. If not, even pause could
> just close the socket and then init if needed.
> In that case clients will not fill the backlog.


You may still want to accept requests for existing sessions.

Both 'graceful shutdown' and app deploy are important cases - it's just
that current 'pause()' is not that good for either of them, if you really
want to cleanup...

Maybe an option to return 503 or custom status/headers - so people can
adjust to various LBs.

Costin


>
>
>  which is bad for the user. A better solution would be to support an option
>> to accept and return immediately ( maybe with a way to do this only for
>> requests not matching any existing session ). I never worked with a LB
>> that
>>  detects status based only on not accepting connections and timeout.
>>
>>
> Hmm, right, a valid option if LB won't get confused.
>
>
>
>> Besides that - is there any other use for pause() ? Maybe that's what
>> should be removed/replaced :-). The behavior ( delay/timeout all TCP
>> connections until backlog is full, then reject ) doesn't seem ideal.
>>
>>
> Agreed. A 'pause' or invalidating backed node should be done in LB.
> We are doing that in mod_jk, mod_proxy, mod_cluster, etc.
> If LB wrongly instructs the clients to connect to a node that is going to
> get shutdown we can't do much about it. Client will experience one
> or other type of connection failures.
>




>
>
>
> Regards
> --
> ^TM
>
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.**org<de...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/08/2012 07:09 PM, Costin Manolache wrote:
> IMHO neither 'graceful shutdown' nor 'deploy' are best served by not
> accepting connections:
> - for 'graceful' - a number of connections will get to backlog and timeout,

Not sure if ServerSocket.close() would cause cascade close of all
accepted sockets that are still running. If not, even pause could
just close the socket and then init if needed.
In that case clients will not fill the backlog.

> which is bad for the user. A better solution would be to support an option
> to accept and return immediately ( maybe with a way to do this only for
> requests not matching any existing session ). I never worked with a LB that
>   detects status based only on not accepting connections and timeout.
>

Hmm, right, a valid option if LB won't get confused.

>
> Besides that - is there any other use for pause() ? Maybe that's what
> should be removed/replaced :-). The behavior ( delay/timeout all TCP
> connections until backlog is full, then reject ) doesn't seem ideal.
>

Agreed. A 'pause' or invalidating backed node should be done in LB.
We are doing that in mod_jk, mod_proxy, mod_cluster, etc.
If LB wrongly instructs the clients to connect to a node that is going to
get shutdown we can't do much about it. Client will experience one
or other type of connection failures.



Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
Thanks for looking it up :-)

+1 on removing it anyways - but it would be good to add some javadocs on
the pause() method, I never understood the use case.

IMHO neither 'graceful shutdown' nor 'deploy' are best served by not
accepting connections:
- for 'graceful' - a number of connections will get to backlog and timeout,
which is bad for the user. A better solution would be to support an option
to accept and return immediately ( maybe with a way to do this only for
requests not matching any existing session ). I never worked with a LB that
 detects status based only on not accepting connections and timeout.

- for deploy - I assume that means you pause when an app is updated ? That
doesn't make a lot of sense, you could still serve other apps, and either
queue or redirect requests for the app getting updated. If you are behind a
LB the updated app would still be served, without timeouts or slow requests.

Besides that - is there any other use for pause() ? Maybe that's what
should be removed/replaced :-). The behavior ( delay/timeout all TCP
connections until backlog is full, then reject ) doesn't seem ideal.


Costin


On Tue, May 8, 2012 at 3:28 AM, Mark Thomas <ma...@apache.org> wrote:

> On 08/05/2012 07:41, Mladen Turk wrote:
> > On 05/08/2012 08:34 AM, Costin Manolache wrote:
> >> On Mon, May 7, 2012 at 11:05 PM, Mladen Turk<mt...@apache.org>  wrote:
> >>
> >>>
> >>> For real pause (stop accepting connections and wait till all sessions
> >>> times out) this can be done safely by setting 10 second timeout
> >>> on listening socket. It means that in worse case we would have
> >>> session-timeout + 10s.
> >>
> >>
> >> I see, you want a graceful shutdown, with a loadbalancer that uses the
> >> fact
> >> that the server rejects connections to indicate 'unhealthy' ( instead of
> >> some error code ). What about the listen backlog, even if you don't call
> >> accept it'll still ACK few connections that will timeout.
> >>
> >
> > The current patch I've send doesn't use timeout.
> > If paused a single connection will get accepted but not
> > processed thus behaving exactly like backlog.
> > Later if continued (pause during deploy) it'll be processed or
> > closed (exactly like backlog behaves).
> >
> >
> >> Sorry, never used pause(), the lbs I know use status code from the
> health
> >> check, and the server is supposed to keep accepting connections until
> the
> >> LB figures things out ( and to be really 'graceful' the LB could keep
> >> sending requests for established sessions for a while, but not new
> >> sessions
> >> ).
> >>
> >> Well - +1, as long as you're sure the close() not unblocking accept()
> bug
> >> is no longer there ( may have been 10 years ago in 1.1, can't remember
> >> :-)
> >>
> >
> > Well, the javadocs for ServerSocket.close() says:
> > Closes this socket.
> > Any thread currently blocked in accept() will throw a SocketException.
>
> I did some svn / BZ archaeology.
>
> The unlocking of the acceptor was introduced [1] in response to a bug
> report [2]. It appears that this was indeed working around a long since
> fixed JVM bug [3].
>
> Given this, I have no objections to Mladen's propose changes. I would
> ask that a note is added to the docs and the Javadoc that explains that
> pausing a socket effectively adds one to the current value of the
> backlog. The only downside I can see is if someone wants zero backlog on
> pause. That is no longer going to be possible.
>
> Mark
>
>
> [1] http://svn.apache.org/viewvc?view=revision&revision=283457
> [2] https://issues.apache.org/bugzilla/show_bug.cgi?id=1418
> [3] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4344135
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mark Thomas <ma...@apache.org>.
On 08/05/2012 07:41, Mladen Turk wrote:
> On 05/08/2012 08:34 AM, Costin Manolache wrote:
>> On Mon, May 7, 2012 at 11:05 PM, Mladen Turk<mt...@apache.org>  wrote:
>>
>>>
>>> For real pause (stop accepting connections and wait till all sessions
>>> times out) this can be done safely by setting 10 second timeout
>>> on listening socket. It means that in worse case we would have
>>> session-timeout + 10s.
>>
>>
>> I see, you want a graceful shutdown, with a loadbalancer that uses the
>> fact
>> that the server rejects connections to indicate 'unhealthy' ( instead of
>> some error code ). What about the listen backlog, even if you don't call
>> accept it'll still ACK few connections that will timeout.
>>
> 
> The current patch I've send doesn't use timeout.
> If paused a single connection will get accepted but not
> processed thus behaving exactly like backlog.
> Later if continued (pause during deploy) it'll be processed or
> closed (exactly like backlog behaves).
> 
> 
>> Sorry, never used pause(), the lbs I know use status code from the health
>> check, and the server is supposed to keep accepting connections until the
>> LB figures things out ( and to be really 'graceful' the LB could keep
>> sending requests for established sessions for a while, but not new
>> sessions
>> ).
>>
>> Well - +1, as long as you're sure the close() not unblocking accept() bug
>> is no longer there ( may have been 10 years ago in 1.1, can't remember
>> :-)
>>
> 
> Well, the javadocs for ServerSocket.close() says:
> Closes this socket.
> Any thread currently blocked in accept() will throw a SocketException.

I did some svn / BZ archaeology.

The unlocking of the acceptor was introduced [1] in response to a bug
report [2]. It appears that this was indeed working around a long since
fixed JVM bug [3].

Given this, I have no objections to Mladen's propose changes. I would
ask that a note is added to the docs and the Javadoc that explains that
pausing a socket effectively adds one to the current value of the
backlog. The only downside I can see is if someone wants zero backlog on
pause. That is no longer going to be possible.

Mark


[1] http://svn.apache.org/viewvc?view=revision&revision=283457
[2] https://issues.apache.org/bugzilla/show_bug.cgi?id=1418
[3] http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4344135

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/08/2012 08:34 AM, Costin Manolache wrote:
> On Mon, May 7, 2012 at 11:05 PM, Mladen Turk<mt...@apache.org>  wrote:
>
>>
>> For real pause (stop accepting connections and wait till all sessions
>> times out) this can be done safely by setting 10 second timeout
>> on listening socket. It means that in worse case we would have
>> session-timeout + 10s.
>
>
> I see, you want a graceful shutdown, with a loadbalancer that uses the fact
> that the server rejects connections to indicate 'unhealthy' ( instead of
> some error code ). What about the listen backlog, even if you don't call
> accept it'll still ACK few connections that will timeout.
>

The current patch I've send doesn't use timeout.
If paused a single connection will get accepted but not
processed thus behaving exactly like backlog.
Later if continued (pause during deploy) it'll be processed or
closed (exactly like backlog behaves).


> Sorry, never used pause(), the lbs I know use status code from the health
> check, and the server is supposed to keep accepting connections until the
> LB figures things out ( and to be really 'graceful' the LB could keep
> sending requests for established sessions for a while, but not new sessions
> ).
>
> Well - +1, as long as you're sure the close() not unblocking accept() bug
> is no longer there ( may have been 10 years ago in 1.1, can't remember :-)
>

Well, the javadocs for ServerSocket.close() says:
Closes this socket.
Any thread currently blocked in accept() will throw a SocketException.



Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
On Mon, May 7, 2012 at 11:05 PM, Mladen Turk <mt...@apache.org> wrote:

> On 05/08/2012 01:13 AM, Costin Manolache wrote:
>
>> On Mon, May 7, 2012 at 3:44 PM, Mladen Turk<mt...@apache.org>  wrote:
>>
>>  On 05/07/2012 11:05 PM, Costin Manolache wrote:
>>>
>>>  By 'unlockAccept' you mean the socket connection made to the acceptor to
>>>> force the accept() to unblock ? How are you getting the socket accept()
>>>> to
>>>> return, my understanding was that close() or thread interrupt don't work
>>>> in
>>>> all cases/VMs.
>>>>
>>>>
>>>>  Well, my question is why the need to break the accept() call
>>> at the first place?
>>> I mean if we are in the pause so that all sessions time out
>>> the extra connection would in worse case scenario double the
>>> cumulative session timeout, so you server might actually go down in
>>> 19.9 instead 10 minutes, but who cares.
>>>
>>>
>> Where do the 10 minutes come from ?
>> Stopping the server shouldn't take 10 min.
>>
>>
> Suppose you can hit pause and wait till all sessions times out.
> This allows shutting down the tomcat without loosing
> existing connections (graceful shutdown).
> Although this is dubious cause sessions can have huge timeouts,
> but I suppose this can be useful in some cases.


If you use sessions ( not everyone does ), and if you have huge timeouts.




>
>
>
>
>>
>>  The amount of complexity we are throwing inside unlockAccept is
>>> just getting insane, and it's still fragile.
>>>
>>>
>> What about not calling it on pause, but still call it on stop ?
>>
>>
> Stop really doesn't needs unlock. Just closing the listening
> socket breaks any accept call.


Maybe I have false memories, but I think the method was first added because
closing the listening socket didn't break the accept call ( in some VMs -
that may be long obsolete ).



>
>>
>>> Another alternate solution might be to setSoTimeout on
>>> accept socket and eat any SocketTimeoutException.
>>> It would mean that we would pull out of accept() each 10 seconds,
>>> and on busy server it might take more then 10 seconds to unlockAccept
>>> anyhow.
>>>
>>
>>
>> On a busy server unlockAccept() shouldn't have to be called.
>> Maybe wait a bit after running=false, and if the accept is still blocked
>> call unlockAccept ?
>>
>>
> Your comment only proves my concerns about that logic.
> We all have our 'own' opinion what should be its purpose.
>
> The point is that on 'stop' we don't need unlock at all.



> The socket is going to be closed immediately after it
> returns from accept() call, so why bother at the first place.
>
> For real pause (stop accepting connections and wait till all sessions
> times out) this can be done safely by setting 10 second timeout
> on listening socket. It means that in worse case we would have
> session-timeout + 10s.


I see, you want a graceful shutdown, with a loadbalancer that uses the fact
that the server rejects connections to indicate 'unhealthy' ( instead of
some error code ). What about the listen backlog, even if you don't call
accept it'll still ACK few connections that will timeout.

Sorry, never used pause(), the lbs I know use status code from the health
check, and the server is supposed to keep accepting connections until the
LB figures things out ( and to be really 'graceful' the LB could keep
sending requests for established sessions for a while, but not new sessions
).

Well - +1, as long as you're sure the close() not unblocking accept() bug
is no longer there ( may have been 10 years ago in 1.1, can't remember :-)

Costin


>
>
>
> Regards
> --
> ^TM
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.**org<de...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/08/2012 01:13 AM, Costin Manolache wrote:
> On Mon, May 7, 2012 at 3:44 PM, Mladen Turk<mt...@apache.org>  wrote:
>
>> On 05/07/2012 11:05 PM, Costin Manolache wrote:
>>
>>> By 'unlockAccept' you mean the socket connection made to the acceptor to
>>> force the accept() to unblock ? How are you getting the socket accept() to
>>> return, my understanding was that close() or thread interrupt don't work
>>> in
>>> all cases/VMs.
>>>
>>>
>> Well, my question is why the need to break the accept() call
>> at the first place?
>> I mean if we are in the pause so that all sessions time out
>> the extra connection would in worse case scenario double the
>> cumulative session timeout, so you server might actually go down in
>> 19.9 instead 10 minutes, but who cares.
>>
>
> Where do the 10 minutes come from ?
> Stopping the server shouldn't take 10 min.
>

Suppose you can hit pause and wait till all sessions times out.
This allows shutting down the tomcat without loosing
existing connections (graceful shutdown).
Although this is dubious cause sessions can have huge timeouts,
but I suppose this can be useful in some cases.


>
>
>> The amount of complexity we are throwing inside unlockAccept is
>> just getting insane, and it's still fragile.
>>
>
> What about not calling it on pause, but still call it on stop ?
>

Stop really doesn't needs unlock. Just closing the listening
socket breaks any accept call.


>
>>
>> Another alternate solution might be to setSoTimeout on
>> accept socket and eat any SocketTimeoutException.
>> It would mean that we would pull out of accept() each 10 seconds,
>> and on busy server it might take more then 10 seconds to unlockAccept
>> anyhow.
>
>
> On a busy server unlockAccept() shouldn't have to be called.
> Maybe wait a bit after running=false, and if the accept is still blocked
> call unlockAccept ?
>

Your comment only proves my concerns about that logic.
We all have our 'own' opinion what should be its purpose.

The point is that on 'stop' we don't need unlock at all.
The socket is going to be closed immediately after it
returns from accept() call, so why bother at the first place.

For real pause (stop accepting connections and wait till all sessions
times out) this can be done safely by setting 10 second timeout
on listening socket. It means that in worse case we would have
session-timeout + 10s.



Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
On Mon, May 7, 2012 at 3:44 PM, Mladen Turk <mt...@apache.org> wrote:

> On 05/07/2012 11:05 PM, Costin Manolache wrote:
>
>> By 'unlockAccept' you mean the socket connection made to the acceptor to
>> force the accept() to unblock ? How are you getting the socket accept() to
>> return, my understanding was that close() or thread interrupt don't work
>> in
>> all cases/VMs.
>>
>>
> Well, my question is why the need to break the accept() call
> at the first place?
> I mean if we are in the pause so that all sessions time out
> the extra connection would in worse case scenario double the
> cumulative session timeout, so you server might actually go down in
> 19.9 instead 10 minutes, but who cares.
>

Where do the 10 minutes come from ?
Stopping the server shouldn't take 10 min.



> The amount of complexity we are throwing inside unlockAccept is
> just getting insane, and it's still fragile.
>

What about not calling it on pause, but still call it on stop ?


>
> Another alternate solution might be to setSoTimeout on
> accept socket and eat any SocketTimeoutException.
> It would mean that we would pull out of accept() each 10 seconds,
> and on busy server it might take more then 10 seconds to unlockAccept
> anyhow.


On a busy server unlockAccept() shouldn't have to be called.
Maybe wait a bit after running=false, and if the accept is still blocked
call unlockAccept ?

I agree it's too complex - maybe you can remove all the waiting at the end
and hardcode all options. Deferred accept handling still needed.


Costin



>
>
> Regards
> --
> ^TM
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.**org<de...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/07/2012 11:05 PM, Costin Manolache wrote:
> By 'unlockAccept' you mean the socket connection made to the acceptor to
> force the accept() to unblock ? How are you getting the socket accept() to
> return, my understanding was that close() or thread interrupt don't work in
> all cases/VMs.
>

Well, my question is why the need to break the accept() call
at the first place?
I mean if we are in the pause so that all sessions time out
the extra connection would in worse case scenario double the
cumulative session timeout, so you server might actually go down in
19.9 instead 10 minutes, but who cares.
The amount of complexity we are throwing inside unlockAccept is
just getting insane, and it's still fragile.

Another alternate solution might be to setSoTimeout on
accept socket and eat any SocketTimeoutException.
It would mean that we would pull out of accept() each 10 seconds,
and on busy server it might take more then 10 seconds to unlockAccept
anyhow.


Regards
-- 
^TM

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Purpose of unlockAccept

Posted by Costin Manolache <co...@gmail.com>.
By 'unlockAccept' you mean the socket connection made to the acceptor to
force the accept() to unblock ? How are you getting the socket accept() to
return, my understanding was that close() or thread interrupt don't work in
all cases/VMs.

Costin

On Mon, May 7, 2012 at 9:24 AM, Mladen Turk <mt...@apache.org> wrote:

> Hi
>
> Did some testing and removing unlockAccept works equally
> fine (and in some circumstances pause/continue and even shutdown is much
> faster).
> Using a simple socket close on stop/destroy and monitoring
> for pause 'after' acceptSocket() by not handling single
> connection received after pause eliminates the need for
> unlockAccept entirely.
> It also fixes various issues with disappearing network
> stack, link-local addresses, and various hang-outs on
> shutdown/pause.
>
>
> Any objections that I commit that to trunk?
> AFAICT this is not covered by any spec and the only
> change is that a single connection that could get accepted
> during pause phase can receive ECONNRESET instead ECONNREFUSED
> in case pause continues beyond client socket timeout.
>
>
> Regards
> --
> ^TM
>
> ------------------------------**------------------------------**---------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.**org<de...@tomcat.apache.org>
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

Re: Purpose of unlockAccept

Posted by Mladen Turk <mt...@apache.org>.
On 05/07/2012 08:48 PM, Mark Thomas wrote:
> I'm not entirely clear on what you are proposing. Can you provide a
> proposed patch for this?
>

Sure.
Need to port that to trunk and new AbstractEndpoint.
Attached is a patch for something similar to tomcat7 :)
just to get an idea.

>
>> AFAICT this is not covered by any spec and the only
>> change is that a single connection that could get accepted
>> during pause phase can receive ECONNRESET instead ECONNREFUSED
>> in case pause continues beyond client socket timeout.
>
> That will be bad for connections from a reverse proxy when taking a
> Tomcat instance out of a cluster. I'm leaning towards objecting to this
> patch on that basis alone.

Think not.
Currently acceptor thread waits before calling accept if paused.
The proposed solution will wait after accept returns.
So there are actually two scenarios.
a) Optimistic:
    No connections accepted between pause/start
    This is advantage over current impl, cause no unlock needed.
b) Pessimistic:
    While in pause accept() was fired and new socket created.
    while(pause) { sleep; }
    If stopped destroy socket and quit acceptor.
    If running continue processing accepted socket.

So in the worse case a single connection will get accepted and
sit in the while(paused) { } block.
Note that this is no different then having a backlog of 1 thought.


Regards
-- 
^TM

Re: Purpose of unlockAccept

Posted by Mark Thomas <ma...@apache.org>.
On 07/05/2012 17:24, Mladen Turk wrote:
> Hi
> 
> Did some testing and removing unlockAccept works equally
> fine (and in some circumstances pause/continue and even shutdown is much
> faster).
> Using a simple socket close on stop/destroy and monitoring
> for pause 'after' acceptSocket() by not handling single
> connection received after pause eliminates the need for
> unlockAccept entirely.
> It also fixes various issues with disappearing network
> stack, link-local addresses, and various hang-outs on
> shutdown/pause.

I'm not entirely clear on what you are proposing. Can you provide a
proposed patch for this?

> Any objections that I commit that to trunk?

At the minute, I'm not sure.

> AFAICT this is not covered by any spec and the only
> change is that a single connection that could get accepted
> during pause phase can receive ECONNRESET instead ECONNREFUSED
> in case pause continues beyond client socket timeout.

That will be bad for connections from a reverse proxy when taking a
Tomcat instance out of a cluster. I'm leaning towards objecting to this
patch on that basis alone.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org