> 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. > >
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. - Gordon ----------------------------------------------------------- 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 > >
