2009/10/15 Aidan Skinner <[email protected]>: > On Thu, Oct 8, 2009 at 9:17 AM, <[email protected]> wrote: >> Author: ritchiem >> Date: Thu Oct 8 08:17:33 2009 >> New Revision: 823087 >> >> URL: http://svn.apache.org/viewvc?rev=823087&view=rev >> Log: >> QPID-1950 : Problem is that the thrown exception whilst an IOException does >> not signify that the socket has closed. So the broker had two open >> connections to send messages on. Change was to ensure that the previous >> Socket/IOSession has been closed before failover starts. Also added >> protected to ChannelOpenHandler to guard against out of order frames causing >> a NPE. > > >> Modified: >> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java >> URL: >> http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java?rev=823087&r1=823086&r2=823087&view=diff >> ============================================================================== >> --- >> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java >> (original) >> +++ >> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java >> Thu Oct 8 08:17:33 2009 >> @@ -140,6 +140,17 @@ >> // a slightly more complex state model therefore I felt it was >> worthwhile doing this. >> AMQStateManager existingStateManager = >> _amqProtocolHandler.getStateManager(); >> >> + >> + // We are failing over so lets ensure any existing >> ProtocolSessions >> + // are closed. Closing them will update the stateManager which >> we >> + // probably don't want to record the change to the closed state. >> + // So lets make a new one. >> + _amqProtocolHandler.setStateManager(new AMQStateManager()); >> + >> + // Close the session, false says don't wait for it to close, >> just close it. >> + >> _amqProtocolHandler.getProtocolSession().closeProtocolSession(false); >> + >> + // Use a fresh new StateManager for the reconnection attempts >> _amqProtocolHandler.setStateManager(new AMQStateManager()); > > I don't think this is necessary in the new > ProtocolEngine/NetworkDriver world order, certainly tests > (SimpleACLTest, MessageDisappearWithIOExceptionTest) failed with the > equivalent getNetworkDriver.close() and all passed without it. > > But, FYI, I removed it when I landed the branch just now. Let me know > if that causes problems.
I'll have to take a look, still stuck finding new and exciting raceconditions in the client close logic. However, if the new network IO default does not also have a test profile that uses mina then it may be a problem. The MessageDisappearWithIOExceptionTest requires Mina to test so will check how you've changed it :) Martin > - Aidan > -- > Apache Qpid - AMQP, JMS, other messaging love http://qpid.apache.org > "A witty saying proves nothing" - Voltaire > > --------------------------------------------------------------------- > Apache Qpid - AMQP Messaging Implementation > Project: http://qpid.apache.org > Use/Interact: mailto:[email protected] > > -- Martin Ritchie --------------------------------------------------------------------- Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:[email protected]
