You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ri...@apache.org on 2009/10/19 13:10:33 UTC

svn commit: r826638 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client: failover/FailoverHandler.java state/AMQStateManager.java

Author: ritchiem
Date: Mon Oct 19 11:10:33 2009
New Revision: 826638

URL: http://svn.apache.org/viewvc?rev=826638&view=rev
Log:
QPID-1816 : When client fails over due to an error, that error is still held bt the StateManager and will prvent the connection from working. During failover check and see if the Connection had been marked closed if so remove any set exception.

Modified:
    qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/failover/FailoverHandler.java
    qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java

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=826638&r1=826637&r2=826638&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 Mon Oct 19 11:10:33 2009
@@ -24,6 +24,7 @@
 import org.apache.qpid.AMQException;
 import org.apache.qpid.client.protocol.AMQProtocolHandler;
 import org.apache.qpid.client.state.AMQStateManager;
+import org.apache.qpid.client.state.AMQState;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -192,6 +193,22 @@
                 // Set the new Protocol Session in the StateManager.               
                 existingStateManager.setProtocolSession(_amqProtocolHandler.getProtocolSession());
 
+                // Now that the ProtocolHandler has been reconnected clean up
+                // the state of the old state manager. As if we simply reinstate
+                // it any old exception that had occured prior to failover may
+                // prohibit reconnection.
+                // e.g. During testing when the broker is shutdown gracefully.
+                // The broker 
+                // Clear any exceptions we gathered
+                if (existingStateManager.getCurrentState() != AMQState.CONNECTION_OPEN)
+                {
+                    // Clear the state of the previous state manager as it may
+                    // have received an exception
+                    existingStateManager.clearLastException();
+                    existingStateManager.changeState(AMQState.CONNECTION_OPEN);
+                }
+
+
                 //Restore Existing State Manager
                 _amqProtocolHandler.setStateManager(existingStateManager);
                 try

Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java?rev=826638&r1=826637&r2=826638&view=diff
==============================================================================
--- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java (original)
+++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/state/AMQStateManager.java Mon Oct 19 11:10:33 2009
@@ -213,4 +213,9 @@
     {
         return _lastException;
     }
+
+    public void clearLastException()
+    {
+        _lastException = null;
+    }
 }



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org


Re: svn commit: r826638 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client: failover/FailoverHandler.java state/AMQStateManager.java

Posted by Martin Ritchie <ri...@apache.org>.
2009/10/19 Aidan Skinner <ai...@gmail.com>:
> On Mon, Oct 19, 2009 at 4:21 PM, Martin Ritchie <ri...@apache.org> wrote:
>> 2009/10/19 Aidan Skinner <ai...@gmail.com>:
>>> On Mon, Oct 19, 2009 at 12:10 PM,  <ri...@apache.org> wrote:
>>>
>>>> Author: ritchiem
>>>> Date: Mon Oct 19 11:10:33 2009
>>>> New Revision: 826638
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=826638&view=rev
>>>> Log:
>>>> QPID-1816 : When client fails over due to an error, that error is still held bt the StateManager and will prvent the connection from working. During failover check and see if the Connection had been marked closed if so remove any set exception.
>>>
>>> Does this mean we can revert the try/catch in FailoverBaseCase.tearDown now?
>>
>> Not sure what you mean here. My FBC.tearDown only has try/finally..
>> which needs to stay. If there is an unexpected error we still need to
>> ensure we shutdown the secondary broker.
>
> Err, yeah, I actually meant SimpleACLTest (the change marnie made in
> r825510), which catches JMSException on teardown, which I think this
> change should make unnecessary.

Ah, right. That I don't know. The answer is perhaps, the test at least
needs to be run without the catch to see if it works. I'm not 100%
that the change will fix it as my change was aimed at connections that
have failed over. The SACLT connections simply fail IIRC.

So I don't think the fix I added gets executed. In the SACL test what
needs to happen is that either the (io Thread performing)
ConnectionCloseHandler OR the client thread calling close() needs to
do the close... problem is both are.

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:dev-subscribe@qpid.apache.org
>
>



-- 
Martin Ritchie

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: svn commit: r826638 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client: failover/FailoverHandler.java state/AMQStateManager.java

Posted by Aidan Skinner <ai...@gmail.com>.
On Mon, Oct 19, 2009 at 4:21 PM, Martin Ritchie <ri...@apache.org> wrote:
> 2009/10/19 Aidan Skinner <ai...@gmail.com>:
>> On Mon, Oct 19, 2009 at 12:10 PM,  <ri...@apache.org> wrote:
>>
>>> Author: ritchiem
>>> Date: Mon Oct 19 11:10:33 2009
>>> New Revision: 826638
>>>
>>> URL: http://svn.apache.org/viewvc?rev=826638&view=rev
>>> Log:
>>> QPID-1816 : When client fails over due to an error, that error is still held bt the StateManager and will prvent the connection from working. During failover check and see if the Connection had been marked closed if so remove any set exception.
>>
>> Does this mean we can revert the try/catch in FailoverBaseCase.tearDown now?
>
> Not sure what you mean here. My FBC.tearDown only has try/finally..
> which needs to stay. If there is an unexpected error we still need to
> ensure we shutdown the secondary broker.

Err, yeah, I actually meant SimpleACLTest (the change marnie made in
r825510), which catches JMSException on teardown, which I think this
change should make unnecessary.

- 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:dev-subscribe@qpid.apache.org


Re: svn commit: r826638 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client: failover/FailoverHandler.java state/AMQStateManager.java

Posted by Martin Ritchie <ri...@apache.org>.
2009/10/19 Aidan Skinner <ai...@gmail.com>:
> On Mon, Oct 19, 2009 at 12:10 PM,  <ri...@apache.org> wrote:
>
>> Author: ritchiem
>> Date: Mon Oct 19 11:10:33 2009
>> New Revision: 826638
>>
>> URL: http://svn.apache.org/viewvc?rev=826638&view=rev
>> Log:
>> QPID-1816 : When client fails over due to an error, that error is still held bt the StateManager and will prvent the connection from working. During failover check and see if the Connection had been marked closed if so remove any set exception.
>
> Does this mean we can revert the try/catch in FailoverBaseCase.tearDown now?

Not sure what you mean here. My FBC.tearDown only has try/finally..
which needs to stay. If there is an unexpected error we still need to
ensure we shutdown the secondary broker.

I also don't think this is the complete solution. I don't have time
right now to look at Ack failures, but they are much fewer than they
were. I think that the remaining ones are due to other race conditions
in the client close logic.

I hope to have time soon to look at the issues.

> - 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:dev-subscribe@qpid.apache.org
>
>



-- 
Martin Ritchie

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: svn commit: r826638 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client: failover/FailoverHandler.java state/AMQStateManager.java

Posted by Aidan Skinner <ai...@gmail.com>.
On Mon, Oct 19, 2009 at 12:10 PM,  <ri...@apache.org> wrote:

> Author: ritchiem
> Date: Mon Oct 19 11:10:33 2009
> New Revision: 826638
>
> URL: http://svn.apache.org/viewvc?rev=826638&view=rev
> Log:
> QPID-1816 : When client fails over due to an error, that error is still held bt the StateManager and will prvent the connection from working. During failover check and see if the Connection had been marked closed if so remove any set exception.

Does this mean we can revert the try/catch in FailoverBaseCase.tearDown now?

- 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:dev-subscribe@qpid.apache.org