Hi Andreas, Hi Oleg, This is some excellent detective work :) Thanks for looking into this. I'm glad that we now have a better understanding of the issue and the problems are already being fixed in the httpcore trunk.
In the mean time, what if we also fix Synapse to not set these buffer size values, unless the user has specifically requested them to be set? That is, if the following properties are not set, we can just let Synapse pick the system defaults instead of initializing them to 8k: http.socket.rcv-buffer-size http.socket.snd-buffer-size WDYT? Thanks, Hiranya On Nov 24, 2013, at 11:10 AM, Oleg Kalnichevski <[email protected]> wrote: > On Sun, 2013-11-24 at 17:29 +0100, Andreas Veithen wrote: >> Oleg, >> >> I had a closer look at the second part of the issue. The problem >> actually occurs between Synapse and the back-end server. The situation >> is quite similar to the first part of the issue: in the SYN packet >> sent by Synapse to the back-end, the TCP window size is set to 43690, >> and once the back-end starts sending the response, the window size >> drops to 8192. >> >> In this case, the problem is that httpcore-nio sets the receive buffer >> size _after_ connecting. With the following patch, the problem >> completely disappears: >> > > Andreas, > > I reviewed and committed the patch. While this change may theoretically > break some applications I would consider such possibility pretty > marginal. > > Oleg > >> Index: >> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultConnectingIOReactor.java >> =================================================================== >> --- >> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultConnectingIOReactor.java >> (revision 1544958) >> +++ >> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultConnectingIOReactor.java >> (working copy) >> @@ -176,21 +176,8 @@ >> } >> key.cancel(); >> if (channel.isConnected()) { >> - try { >> - try { >> - prepareSocket(channel.socket()); >> - } catch (final IOException ex) { >> - if (this.exceptionHandler == null >> - || !this.exceptionHandler.handle(ex)) { >> - throw new IOReactorException( >> - "Failure initalizing socket", ex); >> - } >> - } >> - final ChannelEntry entry = new >> ChannelEntry(channel, sessionRequest); >> - addChannel(entry); >> - } catch (final IOException ex) { >> - sessionRequest.failed(ex); >> - } >> + final ChannelEntry entry = new >> ChannelEntry(channel, sessionRequest); >> + addChannel(entry); >> } >> } >> >> @@ -269,9 +256,9 @@ >> sock.setReuseAddress(this.config.isSoReuseAddress()); >> sock.bind(request.getLocalAddress()); >> } >> + prepareSocket(socketChannel.socket()); >> final boolean connected = >> socketChannel.connect(request.getRemoteAddress()); >> if (connected) { >> - prepareSocket(socketChannel.socket()); >> final ChannelEntry entry = new >> ChannelEntry(socketChannel, request); >> addChannel(entry); >> return; >> >> Note that this change will require some more thorough review because >> the prepareSocket (in AbstractMultiworkerIOReactor) method is >> protected but not final. Therefore there could be code that overrides >> this method and that assumes that the socket is connected, which is no >> longer the case with may change. >> >> However, all httpcore unit tests still pass after that change, and >> there are also no failures in the integration tests in Synapse. >> >> Andreas >> >> On Sun, Nov 24, 2013 at 1:32 PM, Oleg Kalnichevski <[email protected]> wrote: >>> On Sun, 2013-11-24 at 13:02 +0100, Andreas Veithen wrote: >>>> All, >>>> >>>> While debugging this scenario (on Ubuntu with the default receive >>>> buffer size of 8192 and a payload of 1M), I noticed something else. >>>> Very early in the test execution, there are TCP retransmissions from >>>> the client to Synapse. This is of course weird and should not happen. >>>> While trying to understand why that occurs, I noticed that the TCP >>>> window size advertised by Synapse to the client is initially 43690, >>>> and then drops gradually to 8192. The latter value is expected because >>>> it corresponds to the receive buffer size. The question is why the TCP >>>> window is initially 43690. >>>> >>>> It turns out that this is because httpcore-nio sets the receive buffer >>>> size only on the sockets for new incoming connections (in >>>> AbstractMultiworkerIOReactor#prepareSocket), but not on the server >>>> socket itself [1]. Since the initial TCP window size is advertised in >>>> the SYN/ACK packet before the connection is accepted (and httpcore-nio >>>> gets a chance to set the receive buffer size), it will be the default >>>> receive buffer size, not 8192. >>>> >>>> To fix this, I modified httpcore-nio as follows: >>>> >>>> Index: >>>> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultListeningIOReactor.java >>>> =================================================================== >>>> --- >>>> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultListeningIOReactor.java >>>> (revision 1544958) >>>> +++ >>>> httpcore-nio/src/main/java/org/apache/http/impl/nio/reactor/DefaultListeningIOReactor.java >>>> (working copy) >>>> @@ -233,6 +233,9 @@ >>>> try { >>>> final ServerSocket socket = serverChannel.socket(); >>>> socket.setReuseAddress(this.config.isSoReuseAddress()); >>>> + if (this.config.getRcvBufSize() > 0) { >>>> + >>>> socket.setReceiveBufferSize(this.config.getRcvBufSize()); >>>> + } >>>> serverChannel.configureBlocking(false); >>>> socket.bind(address); >>>> } catch (final IOException ex) { >>>> >>>> This fixes the TCP window and retransmission problem, and it also >>>> appears to fix half of the overall issue: now transmitting the 1M >>>> request payload only takes a few 100 milliseconds instead of 20 >>>> seconds. However, the issue still exists in the return path. >>>> >>>> Andreas >>>> >>> >>> Andreas >>> >>> I committed the patch to SVN trunk. Please review. >>> >>> Could you please elaborate what do you mean by 'issue still exists in >>> the return path'. I am not sure I quite understand. >>> >>> Oleg >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: [email protected] >>> For additional commands, e-mail: [email protected] >>> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > -- Hiranya Jayathilaka Mayhem Lab/RACE Lab; Dept. of Computer Science, UCSB; http://cs.ucsb.edu E-mail: [email protected]; Mobile: +1 (805) 895-7443 Blog: http://techfeast-hiranya.blogspot.com
