2009/9/22 Rajith Attapattu <[email protected]>:
> On Tue, Sep 22, 2009 at 8:58 AM, Martin Ritchie <[email protected]> wrote:
>> 2009/9/22 Rajith Attapattu <[email protected]>:
>>> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <[email protected]> wrote:
>>>> Hi Rajith,
>>>> Will this not cause both the 0-8 and 0-10 code path to throw a
>>>> NullPointer if none of the brokers are available on startup.
>>>> Is the FailoverPolicy cycle count not what stops the looping?
>>>>
>>>> Regards
>>>
>>> Hi Martin,
>>>
>>> If the initial broker is not available it will throw
>>> ConnectionException. My title for the JIRA is a bit misleading here.
>>> If the initial connection is successful and if it gets disconnected
>>> before it gets a chance to update its membership list via the failover
>>> exchange this looping can happen.
>>>
>>> The 0-8 codepath and a non-clustered 0-10 codepath should not be using
>>> failover exchange method.
>>> Perhaps we should add a check for this??
>>>
>>> This modification is done only for the FailoverExchange method where
>>> cycle count is not applicable.
>>> The failover exchange maintains a dynamic list of brokers based on the
>>> updates send by the failover exchange it is subscribed to.
>>> It will continue until total cluster failure.
>>>
>>> Regards,
>>>
>>> Rajith
>>
>> I see the dynamic list bits now but I'm still don't see how returning
>> null is checked? In the failover mode the returned value from
>> getNextBroker is passed straight in to makeBrokerConnection which
>> assumes the BrokerDetails is not null.
>>
>> I must be missing something.
>>
>> Martin
>
> In the initial connection it checks if brokerDetails is null
>
> while (!_connected && retryAllowed && brokerDetails != null)
>
> and during failover we do the following.
> public boolean attemptReconnection()
> {
> BrokerDetails broker = null;
> while (_failoverPolicy.failoverAllowed() && (broker =
> _failoverPolicy.getNextBrokerDetails()) != null)
Are you sure you've commited all the code?
AMQConnection:L515 there is no null check.
while (!_connected && retryAllowed)
....
retryAllowed = _failoverPolicy.failoverAllowed();
brokerDetails = _failoverPolicy.getNextBrokerDetails();
AMQConnection: L:692 doesn't do a null check.
public boolean attemptReconnection()
{
while (_failoverPolicy.failoverAllowed())
{
try
{
makeBrokerConnection(_failoverPolicy.getNextBrokerDetails());
I'm also not sure that we should have to change all our checks from
while (_failoverPolicy.failoverAllowed())
to
while (_failoverPolicy.failoverAllowed() && (broker =
_failoverPolicy.getNextBrokerDetails()) != null)
If you are changing the condition of failoverAllowed to require
_failoverPolicy.getNextBrokerDetails() to be non-null I'd add that as
part of the FailoverExchangeMethod's getNextBrokerDetails().
> On a separate note, I appreciate your questions. It is best some sort
> of review is done for most of the commits.
> If you aren't clear on something please don't hesitate to ask as it
> could potentially be an issue.
I try and review all the Java commits but there are so many. I too
appreciate people reviewing my work. We all make mistakes from time to
time, well except Rafi ;). I think it just helps us write better code,
if it is reviewed.
Cheers
martin
>>> ---------------------------------------------------------------------
>>> 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]
>>
>>
>
> ---------------------------------------------------------------------
> 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]
