Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
Rajith, I think we need to have a think about what we want from Failover. It is so poorly tested just now having it disabled by default is probably a very good idea. IIRC the NoFailover was added to be more explicit than setting retry to 0. For me retry and failover are two different concepts that have been wrapped up in the one reconnection strategy. Failover happens between server in my mind so the SingleServerFailover was always a bit of an odd name (and yes I know I named it:) ). From an end users point of view it would make sence to have a retry layer and a failover layer. Question of how we implement that is debatable. Cheers Martin 2009/6/24 Rajith Attapattu rajit...@gmail.com: I agree with you that NoFailover is redundent and I suggest we remove it. Currently the AMQConnection defaults to FailoverSingleServer by default if only one broker is specified in the URL. In most situations I have come across users prefer not to reconnect by default, so IMO we should not retry. The other option is to make NoFailover the default, but then if people want to retry they will have to explicitly specify FailoverSingleServer as the failover method which is not ideal. Most end users expect safe default behaviour. Retrying without explicit instructions is not a good thing from my experience. So I think the best solution is to leave things as it is and explicitly specify retries in the URL if you want retries. Regards, Rajith On Wed, Jun 24, 2009 at 11:49 AM, Martin Ritchieritch...@apache.org wrote: I'd have to question what the point of the class is then as it is it is basically a much more complicated version of 'NoFailover'. It should at least allow failover to occur once. If not once per successfull connection Martin 2009/6/24 Rajith Attapattu rajit...@gmail.com: Martin, When we discussed this issue we agreed that by default the SingleFailoverMethod should not retry at all. So in order to make the initial connection attempt work, we made AMQConnection use getCurrentBrokerDetails() instead of the getNextBrokerDetails() to get the initial broker to connect. Regards, Rajith On Wed, Jun 24, 2009 at 11:27 AM, Martin Ritchieritch...@apache.org wrote: Hi Rajith, Why did you set the default retries to 0 as well as starting the _currentRetries from 0? I thought the original problem was that it would always try twice with this commit we effectively reduced the retries by 2. Do you have time to write a test to ensure that FailoverSingleServer only retires once not twice :) /** The default number of times to rety a conection to this server */ -public static final int DEFAULT_SERVER_RETRIES = 1; +public static final int DEFAULT_SERVER_RETRIES = 0; This commit has resulted in the SingleServerFailover not working. When I make my connection failover I get : main 2009-06-24 16:21:45,105 DEBUG [qpid.client.protocol.AMQProtocolHandler] Session closed called with failover state currently FailoverState: NOT STARTED main 2009-06-24 16:21:45,105 DEBUG [apache.qpid.jms.FailoverPolicy] All failover methods exhausted main 2009-06-24 16:21:45,106 DEBUG [qpid.client.protocol.AMQProtocolHandler] Failover not allowed by policy. main 2009-06-24 16:21:45,106 DEBUG [apache.qpid.jms.FailoverPolicy] All failover methods exhausted main 2009-06-24 16:21:45,106 DEBUG [qpid.client.protocol.AMQProtocolHandler] Failover Policy: Failover not allowed Failover policy methods Single Server: Max Retries:0 Current Retry:0 tcp://localhost:5672 As the current retry is 0 and the Max is 0 then failover doesn't occur. Am I missing something? Cheers Martin 2009/5/14 raj...@apache.org: Author: rajith Date: Thu May 14 19:50:23 2009 New Revision: 774899 URL: http://svn.apache.org/viewvc?rev=774899view=rev Log: This is a fix for QPID-1859 For FailoverSingleServer the default for retries is set to 0 and the current_retries start from 0 instead of -1. For FailoverRoundRobinServers the current_broker_index now starts from 0 instead of -1. The AMQConnection now uses the getCurrentBrokerDetails() instead of the getNextBrokerDetails() to get the initial broker to connect. Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java Modified: qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java URL: http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java?rev=774899r1=774898r2=774899view=diff == --- qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java (original) +++
Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
2009/6/25 Martin Ritchie ritch...@apache.org Rajith, I think we need to have a think about what we want from Failover. It is so poorly tested just now having it disabled by default is probably a very good idea. I am 100% in agreement that we should not have failover (retry) switched on by default. the behaviour is not what you would expect from a JMS client. If people understand the retry mechanism and its limitations it is fine for them to switch it on (if it works). We should also think about how we differentiate between retries/failover where no state is lost (which is possible in 0-10) and retries/failover where state is lost - they are most definitely not the same thing, and their presentation to the client application should be different (i.e. where no state is lost the failover can be invisible to the client application. If state is potentially lost then the mechanism by which the client is informed of this should be made obvious. -- Rob
Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
On Thu, Jun 25, 2009 at 6:11 AM, Robert Godfreyrob.j.godf...@gmail.com wrote: 2009/6/25 Martin Ritchie ritch...@apache.org Rajith, I think we need to have a think about what we want from Failover. It is so poorly tested just now having it disabled by default is probably a very good idea. I am 100% in agreement that we should not have failover (retry) switched on by default. the behaviour is not what you would expect from a JMS client. Totally agreed. There were some nasty issues with some end users due to the java client retrying by default and also retrying more than expected. So by experience I also agree that we shouldn't retry by default. I also agree that we need to differentiate between failover and retry. If people understand the retry mechanism and its limitations it is fine for them to switch it on (if it works). We should also think about how we differentiate between retries/failover where no state is lost (which is possible in 0-10) and retries/failover where state is lost - they are most definitely not the same thing, and their presentation to the client application should be different (i.e. where no state is lost the failover can be invisible to the client application. If state is potentially lost then the mechanism by which the client is informed of this should be made obvious. -- Rob -- Regards, Rajith Attapattu Red Hat http://rajith.2rlabs.com/ - Apache Qpid - AMQP Messaging Implementation Project: http://qpid.apache.org Use/Interact: mailto:dev-subscr...@qpid.apache.org
Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
Hi Rajith,
Why did you set the default retries to 0 as well as starting the
_currentRetries from 0? I thought the original problem was that it
would always try twice with this commit we effectively reduced the
retries by 2. Do you have time to write a test to ensure that
FailoverSingleServer only retires once not twice :)
/** The default number of times to rety a conection to this server */
-public static final int DEFAULT_SERVER_RETRIES = 1;
+public static final int DEFAULT_SERVER_RETRIES = 0;
This commit has resulted in the SingleServerFailover not working.
When I make my connection failover I get :
main 2009-06-24 16:21:45,105 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Session closed called with
failover state currently FailoverState: NOT STARTED
main 2009-06-24 16:21:45,105 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover not allowed by
policy.
main 2009-06-24 16:21:45,106 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover Policy:
Failover not allowed
Failover policy methods
Single Server:
Max Retries:0
Current Retry:0
tcp://localhost:5672
As the current retry is 0 and the Max is 0 then failover doesn't occur.
Am I missing something?
Cheers
Martin
2009/5/14 raj...@apache.org:
Author: rajith
Date: Thu May 14 19:50:23 2009
New Revision: 774899
URL: http://svn.apache.org/viewvc?rev=774899view=rev
Log:
This is a fix for QPID-1859
For FailoverSingleServer the default for retries is set to 0 and the
current_retries start from 0 instead of -1.
For FailoverRoundRobinServers the current_broker_index now starts from 0
instead of -1.
The AMQConnection now uses the getCurrentBrokerDetails() instead of the
getNextBrokerDetails() to get the initial broker to connect.
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
Thu May 14 19:50:23 2009
@@ -455,7 +455,7 @@
}
_failoverPolicy = new FailoverPolicy(connectionURL, this);
-BrokerDetails brokerDetails = _failoverPolicy.getNextBrokerDetails();
+BrokerDetails brokerDetails =
_failoverPolicy.getCurrentBrokerDetails();
if (brokerDetails.getTransport().equals(BrokerDetails.VM))
{
_delegate = new AMQConnectionDelegate_8_0(this);
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
Thu May 14 19:50:23 2009
@@ -35,19 +35,19 @@
public static final int DEFAULT_SERVER_RETRIES = 0;
/** The index into the hostDetails array of the broker to which we are
connected */
-private int _currentBrokerIndex = -1;
+private int _currentBrokerIndex = 0;
/** The number of times to retry connecting for each server */
private int _serverRetries;
/** The current number of retry attempts made */
-private int _currentServerRetry;
+private int _currentServerRetry = 0;
/** The number of times to cycle through the servers */
private int _cycleRetries;
/** The current number of cycles performed. */
-private int _currentCycleRetries;
+private int _currentCycleRetries = 0;
/** Array of BrokerDetail used to make connections. */
protected ConnectionURL _connectionDetails;
@@ -62,7 +62,7 @@
_connectionDetails = connectionDetails;
// There is no current broker at startup so set it to -1.
-_currentBrokerIndex = -1;
+_currentBrokerIndex = 0;
String cycleRetries =
Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
Martin,
When we discussed this issue we agreed that by default the
SingleFailoverMethod should not retry at all.
So in order to make the initial connection attempt work, we made
AMQConnection use getCurrentBrokerDetails() instead of the
getNextBrokerDetails() to get the initial broker to connect.
Regards,
Rajith
On Wed, Jun 24, 2009 at 11:27 AM, Martin Ritchieritch...@apache.org wrote:
Hi Rajith,
Why did you set the default retries to 0 as well as starting the
_currentRetries from 0? I thought the original problem was that it
would always try twice with this commit we effectively reduced the
retries by 2. Do you have time to write a test to ensure that
FailoverSingleServer only retires once not twice :)
/** The default number of times to rety a conection to this server */
-public static final int DEFAULT_SERVER_RETRIES = 1;
+public static final int DEFAULT_SERVER_RETRIES = 0;
This commit has resulted in the SingleServerFailover not working.
When I make my connection failover I get :
main 2009-06-24 16:21:45,105 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Session closed called with
failover state currently FailoverState: NOT STARTED
main 2009-06-24 16:21:45,105 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover not allowed by
policy.
main 2009-06-24 16:21:45,106 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover Policy:
Failover not allowed
Failover policy methods
Single Server:
Max Retries:0
Current Retry:0
tcp://localhost:5672
As the current retry is 0 and the Max is 0 then failover doesn't occur.
Am I missing something?
Cheers
Martin
2009/5/14 raj...@apache.org:
Author: rajith
Date: Thu May 14 19:50:23 2009
New Revision: 774899
URL: http://svn.apache.org/viewvc?rev=774899view=rev
Log:
This is a fix for QPID-1859
For FailoverSingleServer the default for retries is set to 0 and the
current_retries start from 0 instead of -1.
For FailoverRoundRobinServers the current_broker_index now starts from 0
instead of -1.
The AMQConnection now uses the getCurrentBrokerDetails() instead of the
getNextBrokerDetails() to get the initial broker to connect.
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
Thu May 14 19:50:23 2009
@@ -455,7 +455,7 @@
}
_failoverPolicy = new FailoverPolicy(connectionURL, this);
-BrokerDetails brokerDetails =
_failoverPolicy.getNextBrokerDetails();
+BrokerDetails brokerDetails =
_failoverPolicy.getCurrentBrokerDetails();
if (brokerDetails.getTransport().equals(BrokerDetails.VM))
{
_delegate = new AMQConnectionDelegate_8_0(this);
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
Thu May 14 19:50:23 2009
@@ -35,19 +35,19 @@
public static final int DEFAULT_SERVER_RETRIES = 0;
/** The index into the hostDetails array of the broker to which we are
connected */
-private int _currentBrokerIndex = -1;
+private int _currentBrokerIndex = 0;
/** The number of times to retry connecting for each server */
private int _serverRetries;
/** The current number of retry attempts made */
-private int _currentServerRetry;
+private int _currentServerRetry = 0;
/** The number of times to cycle through the servers */
private int _cycleRetries;
/** The current number of cycles performed. */
-private int _currentCycleRetries;
+private int
Re: svn commit: r774899 - in /qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid: client/AMQConnection.java jms/failover/FailoverRoundRobinServers.java jms/failover/FailoverSingleServer.java
I'd have to question what the point of the class is then as it is it
is basically a much more complicated version of 'NoFailover'.
It should at least allow failover to occur once. If not once per
successfull connection
Martin
2009/6/24 Rajith Attapattu rajit...@gmail.com:
Martin,
When we discussed this issue we agreed that by default the
SingleFailoverMethod should not retry at all.
So in order to make the initial connection attempt work, we made
AMQConnection use getCurrentBrokerDetails() instead of the
getNextBrokerDetails() to get the initial broker to connect.
Regards,
Rajith
On Wed, Jun 24, 2009 at 11:27 AM, Martin Ritchieritch...@apache.org wrote:
Hi Rajith,
Why did you set the default retries to 0 as well as starting the
_currentRetries from 0? I thought the original problem was that it
would always try twice with this commit we effectively reduced the
retries by 2. Do you have time to write a test to ensure that
FailoverSingleServer only retires once not twice :)
/** The default number of times to rety a conection to this server */
-public static final int DEFAULT_SERVER_RETRIES = 1;
+public static final int DEFAULT_SERVER_RETRIES = 0;
This commit has resulted in the SingleServerFailover not working.
When I make my connection failover I get :
main 2009-06-24 16:21:45,105 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Session closed called with
failover state currently FailoverState: NOT STARTED
main 2009-06-24 16:21:45,105 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover not allowed by
policy.
main 2009-06-24 16:21:45,106 DEBUG [apache.qpid.jms.FailoverPolicy]
All failover methods exhausted
main 2009-06-24 16:21:45,106 DEBUG
[qpid.client.protocol.AMQProtocolHandler] Failover Policy:
Failover not allowed
Failover policy methods
Single Server:
Max Retries:0
Current Retry:0
tcp://localhost:5672
As the current retry is 0 and the Max is 0 then failover doesn't occur.
Am I missing something?
Cheers
Martin
2009/5/14 raj...@apache.org:
Author: rajith
Date: Thu May 14 19:50:23 2009
New Revision: 774899
URL: http://svn.apache.org/viewvc?rev=774899view=rev
Log:
This is a fix for QPID-1859
For FailoverSingleServer the default for retries is set to 0 and the
current_retries start from 0 instead of -1.
For FailoverRoundRobinServers the current_broker_index now starts from 0
instead of -1.
The AMQConnection now uses the getCurrentBrokerDetails() instead of the
getNextBrokerDetails() to get the initial broker to connect.
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverSingleServer.java
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
Thu May 14 19:50:23 2009
@@ -455,7 +455,7 @@
}
_failoverPolicy = new FailoverPolicy(connectionURL, this);
-BrokerDetails brokerDetails =
_failoverPolicy.getNextBrokerDetails();
+BrokerDetails brokerDetails =
_failoverPolicy.getCurrentBrokerDetails();
if (brokerDetails.getTransport().equals(BrokerDetails.VM))
{
_delegate = new AMQConnectionDelegate_8_0(this);
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java?rev=774899r1=774898r2=774899view=diff
==
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/jms/failover/FailoverRoundRobinServers.java
Thu May 14 19:50:23 2009
@@ -35,19 +35,19 @@
public static final int DEFAULT_SERVER_RETRIES = 0;
/** The index into the hostDetails array of the broker to which we are
connected */
-private int _currentBrokerIndex = -1;
+private int _currentBrokerIndex = 0;
/** The number of times to retry connecting for each server */
private int _serverRetries;
/** The current number of retry attempts made */
-private int
