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-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
