You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Aidan Skinner <ai...@apache.org> on 2009/10/15 03:10:06 UTC

Re: svn commit: r823087 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/handler/ client/src/main/java/org/apache/qpid/client/failover/ systests/src/main/java/org/apache/qpid/server/failover/ systests/src/main/java/org/apache/q

On Thu, Oct 8, 2009 at 9:17 AM,  <ri...@apache.org> 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.

- 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: r823087 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/handler/ client/src/main/java/org/apache/qpid/client/failover/ systests/src/main/java/org/apache/qpid/server/failover/ systests/src/main/java/org/apache/q

Posted by Aidan Skinner <ai...@apache.org>.
On Thu, Oct 15, 2009 at 9:36 AM, Marnie McCormack
<ma...@googlemail.com> wrote:
> Aidan,
>
> Have you got a list if JIRAs you're planning for 0.6 ?

Jira's normally a pretty good indication of what I'm working on,
here's a search for things assigned to me, with a fix-for for 0.6:

http://issues.apache.org/jira/secure/IssueNavigator.jspa?reset=true&&pid=12310520&fixfor=12313728&resolution=-1&assigneeSelect=specificuser&assignee=aidan&sorter/field=priority&sorter/order=DESC

I'll normally add things to that version once I'm pretty sure I'll get
it in, no point in chopping and changing scope randomly.

- 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: r823087 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/handler/ client/src/main/java/org/apache/qpid/client/failover/ systests/src/main/java/org/apache/qpid/server/failover/ systests/src/main/java/org/apache/q

Posted by Marnie McCormack <ma...@googlemail.com>.
Aidan,

Have you got a list if JIRAs you're planning for 0.6 ?

Marnie

On Thu, Oct 15, 2009 at 9:30 AM, Aidan Skinner <ai...@apache.org> wrote:

> Trunk still uses Mina, it's just not as tightly coupled. I haven't
> ported the 0-10 side of things to these interfaces yet. Once I've done
> that it'll be possible to choose alternate implementations.
>
> - Aidan
>
> On 10/15/09, Martin Ritchie <ri...@apache.org> wrote:
> > 2009/10/15 Aidan Skinner <ai...@apache.org>:
> >> On Thu, Oct 8, 2009 at 9:17 AM,  <ri...@apache.org> 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:commits-subscribe@qpid.apache.org
> >>
> >>
> >
> >
> >
> > --
> > Martin Ritchie
> >
> > ---------------------------------------------------------------------
> > Apache Qpid - AMQP Messaging Implementation
> > Project:      http://qpid.apache.org
> > Use/Interact: mailto:dev-subscribe@qpid.apache.org
> >
> >
>
> --
> Sent from Google Mail for mobile | mobile.google.com
>
> 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: r823087 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/handler/ client/src/main/java/org/apache/qpid/client/failover/ systests/src/main/java/org/apache/qpid/server/failover/ systests/src/main/java/org/apache/q

Posted by Aidan Skinner <ai...@apache.org>.
Trunk still uses Mina, it's just not as tightly coupled. I haven't
ported the 0-10 side of things to these interfaces yet. Once I've done
that it'll be possible to choose alternate implementations.

- Aidan

On 10/15/09, Martin Ritchie <ri...@apache.org> wrote:
> 2009/10/15 Aidan Skinner <ai...@apache.org>:
>> On Thu, Oct 8, 2009 at 9:17 AM,  <ri...@apache.org> 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:commits-subscribe@qpid.apache.org
>>
>>
>
>
>
> --
> Martin Ritchie
>
> ---------------------------------------------------------------------
> Apache Qpid - AMQP Messaging Implementation
> Project:      http://qpid.apache.org
> Use/Interact: mailto:dev-subscribe@qpid.apache.org
>
>

-- 
Sent from Google Mail for mobile | mobile.google.com

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: r823087 - in /qpid/trunk/qpid/java: broker/src/main/java/org/apache/qpid/server/handler/ client/src/main/java/org/apache/qpid/client/failover/ systests/src/main/java/org/apache/qpid/server/failover/ systests/src/main/java/org/apache/q

Posted by Martin Ritchie <ri...@apache.org>.
2009/10/15 Aidan Skinner <ai...@apache.org>:
> On Thu, Oct 8, 2009 at 9:17 AM,  <ri...@apache.org> 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:commits-subscribe@qpid.apache.org
>
>



-- 
Martin Ritchie

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