You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@qpid.apache.org by ra...@apache.org on 2009/09/22 05:11:56 UTC
svn commit: r817487 -
/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Author: rajith
Date: Tue Sep 22 03:11:56 2009
New Revision: 817487
URL: http://svn.apache.org/viewvc?rev=817487&view=rev
Log:
This is a fix for QPID-1956
Added a check in the getNextBrokerDetails method to return null when the current broker equals the only remaining broker in the list
A test case for this will be added once I finalized the test case for the failover exchange method
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java?rev=817487&r1=817486&r2=817487&view=diff
==============================================================================
--- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java (original)
+++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java Tue Sep 22 03:11:56 2009
@@ -189,7 +189,8 @@
{
synchronized (_brokerListLock)
{
- return _connectionDetails.getBrokerDetails(_currentBrokerIndex);
+ _currentBrokerDetail = _connectionDetails.getBrokerDetails(_currentBrokerIndex);
+ return _currentBrokerDetail;
}
}
@@ -214,7 +215,15 @@
broker.getHost().equals(_currentBrokerDetail.getHost()) &&
broker.getPort() == _currentBrokerDetail.getPort())
{
- return getNextBrokerDetails();
+ if (_connectionDetails.getBrokerCount() > 1)
+ {
+ return getNextBrokerDetails();
+ }
+ else
+ {
+ _failedAttemps ++;
+ return null;
+ }
}
String delayStr = broker.getProperty(BrokerDetails.OPTIONS_CONNECT_DELAY);
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:commits-subscribe@qpid.apache.org
Re: svn commit: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Martin Ritchie <ri...@apache.org>.
2009/9/22 Rajith Attapattu <ra...@gmail.com>:
> On Tue, Sep 22, 2009 at 11:18 AM, Martin Ritchie <ri...@apache.org> wrote:
>> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>>> On Tue, Sep 22, 2009 at 8:58 AM, Martin Ritchie <ri...@apache.org> wrote:
>>>> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>>>>> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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?
>
> Ah I have forgotten to commit this bit last night: (:
> Thx for questioning this.
>
> The null check was added bcos both FailoverExchangeMethod and
> FailoverRoundRobinServers could return null for getNextBrokerDetails.
I still beleieve the better option is to put the logic inside
failoverAllowed and have it return false. That way we can guarrantee
getNextBrokerDetails never returns null when failover is allowed to
take place.
Cheers
Martin
>> 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().
>
> I'd have to go through the code again and see how this can be done.
> Most likely this is doable.
>
>>> 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: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
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
>>
>
>
>
> --
> Regards,
>
> Rajith Attapattu
> Red Hat
> http://rajith.2rlabs.com/
>
> ---------------------------------------------------------------------
> 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: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Rajith Attapattu <ra...@gmail.com>.
On Tue, Sep 22, 2009 at 11:18 AM, Martin Ritchie <ri...@apache.org> wrote:
> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>> On Tue, Sep 22, 2009 at 8:58 AM, Martin Ritchie <ri...@apache.org> wrote:
>>> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>>>> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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?
Ah I have forgotten to commit this bit last night: (:
Thx for questioning this.
The null check was added bcos both FailoverExchangeMethod and
FailoverRoundRobinServers could return null for getNextBrokerDetails.
> 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().
I'd have to go through the code again and see how this can be done.
Most likely this is doable.
>> 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: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
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>
>
--
Regards,
Rajith Attapattu
Red Hat
http://rajith.2rlabs.com/
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Martin Ritchie <ri...@apache.org>.
2009/9/22 Rajith Attapattu <ra...@gmail.com>:
> On Tue, Sep 22, 2009 at 8:58 AM, Martin Ritchie <ri...@apache.org> wrote:
>> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>>> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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: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
>>
>>
>
> ---------------------------------------------------------------------
> 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: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Rajith Attapattu <ra...@gmail.com>.
On Tue, Sep 22, 2009 at 8:58 AM, Martin Ritchie <ri...@apache.org> wrote:
> 2009/9/22 Rajith Attapattu <ra...@gmail.com>:
>> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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)
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.
>> ---------------------------------------------------------------------
>> 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
>
>
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Martin Ritchie <ri...@apache.org>.
2009/9/22 Rajith Attapattu <ra...@gmail.com>:
> On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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
> ---------------------------------------------------------------------
> 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: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Rajith Attapattu <ra...@gmail.com>.
On Tue, Sep 22, 2009 at 7:13 AM, Martin Ritchie <ri...@apache.org> 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
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org
Re: svn commit: r817487 - /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
Posted by Martin Ritchie <ri...@apache.org>.
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
Martin
2009/9/22 <ra...@apache.org>:
> Author: rajith
> Date: Tue Sep 22 03:11:56 2009
> New Revision: 817487
>
> URL: http://svn.apache.org/viewvc?rev=817487&view=rev
> Log:
> This is a fix for QPID-1956
> Added a check in the getNextBrokerDetails method to return null when the current broker equals the only remaining broker in the list
> A test case for this will be added once I finalized the test case for the failover exchange method
>
> Modified:
> qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
>
> Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java
> URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java?rev=817487&r1=817486&r2=817487&view=diff
> ==============================================================================
> --- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java (original)
> +++ qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverExchangeMethod.java Tue Sep 22 03:11:56 2009
> @@ -189,7 +189,8 @@
> {
> synchronized (_brokerListLock)
> {
> - return _connectionDetails.getBrokerDetails(_currentBrokerIndex);
> + _currentBrokerDetail = _connectionDetails.getBrokerDetails(_currentBrokerIndex);
> + return _currentBrokerDetail;
> }
> }
>
> @@ -214,7 +215,15 @@
> broker.getHost().equals(_currentBrokerDetail.getHost()) &&
> broker.getPort() == _currentBrokerDetail.getPort())
> {
> - return getNextBrokerDetails();
> + if (_connectionDetails.getBrokerCount() > 1)
> + {
> + return getNextBrokerDetails();
> + }
> + else
> + {
> + _failedAttemps ++;
> + return null;
> + }
> }
>
> String delayStr = broker.getProperty(BrokerDetails.OPTIONS_CONNECT_DELAY);
>
>
>
> ---------------------------------------------------------------------
> 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