> On June 25, 2012, 8:42 a.m., Gordon Sim wrote: > > /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp, line 162 > > <https://reviews.apache.org/r/5529/diff/1/?file=115975#file115975line162> > > > > I agree with Andrew. This addition is less than ideal, not least > > because it only addresses the code path for plain TCP, not SSL or RDMA. > > > > The appropriate way to close quietly is to call > > ConnectionOutputHandler::close() from within the broker code. Perhaps we > > can catch the exception in qpid::broker::Connection::received()? It's > > really a contract with components such as the connection observers around > > what different exception types mean. > > Alan Conway wrote: > Thanks for the feedback. Is there any consensus on which is the better > way to close the connnection: > > astitcher> Can't you handle this the same way as the recent connection > limiting code by failing authentication instead? > gsim> The appropriate way to close quietly is to call > ConnectionOutputHandler::close() from within the broker code. > > > > Gordon Sim wrote: > Those aren't (necessarily) entirely contradictory. The connection > limiting code throws a ConnectionForcedException. However this is caught in > ConnectionHandler and handled by issuing a connection.close control (it > doesn't actually close the connection however). For the observers you would > need to add (or move) the exception handling logic as the call to the > observers is not currently covered in that. > > Issuing a connection.close is probably a good idea (gives the client some > indication of the issue). However at some point at least I do think we want > to also force the connection to close eventually. AT present it seems we rely > on the client to respond with a close-ok.
That's something I didn't know about failing authentication (that it doesn't cause the connection to close by itself). The strong benefit of doing it like that as Gordon says is that the client can get an informative exception message. On a separate note then perhaps we need to force a connection close at the server end after we transmit the connection.close control - in theory the IO layer will finish transmitting everything queued up to transmit after AsynchIO::queueWriteClose() is called. So you should be able to queue the frame to be sent and the close at the same time - I suppose to be properly sure you'd also need to be sure that the read side was closed within a fairly short interval too. - Andrew ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/5529/#review8535 ----------------------------------------------------------- On June 22, 2012, 8:18 p.m., Alan Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/5529/ > ----------------------------------------------------------- > > (Updated June 22, 2012, 8:18 p.m.) > > > Review request for qpid, Andrew Stitcher and Gordon Sim. > > > Description > ------- > > A HA backup broker rejects client connections by throwing out of > ConnectionObserver::opened. > This is not an error, but generates a lot of noise in the form of error > log messages. > This patch throws a ClosedException in this case and has the exception > handler generate a debug > message instead of an error if it catches ClosedException. > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/ha/BackupConnectionExcluder.h 1353017 > /trunk/qpid/cpp/src/qpid/ha/ConnectionObserver.cpp 1353017 > /trunk/qpid/cpp/src/qpid/sys/AsynchIOHandler.cpp 1353017 > > Diff: https://reviews.apache.org/r/5529/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alan Conway > >
