[ 
https://issues.apache.org/jira/browse/QPID-4534?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13806383#comment-13806383
 ] 

Robbie Gemmell commented on QPID-4534:
--------------------------------------

Sorry Keith, I <gasp> forgot about this and have been overlooking it for some 
time as it wasn't RTR.

I have remade the patch to take into account changes in the tree, and tweaked 
some bits along the way to make the diff less difficult to read. Beyond that, I 
made some 'actual' changes:

- Tweaked the wording in the documentation changes to make the defaults clearer.
- Added test to BrokerDetailsTest to check that a null value is returned for 
the heartbeat option when it isn't set.
- Renamed ConnectionSettings.getHeartbeat() to getHeartbeat08() to make it 
clearer later it isnt suitable for 0-10, just in case someone doesnt notice 
getHeartbeat010() at a later date.
- Changed ConnectionSettings.getHeartbeat08() to return an Integer instead of 
int, and updated ConnectionTuneParameters to use an Integer, both null by 
default rather than 0. The patch had removed the ability to override the broker 
value and disable heartbeating on 0-8/0-9/0-9-1 connections, so using null as 
the default allows distinguishing not-set and set-to-zero and using the latter 
to override the broker. A bit weird admittedly, but might be important at some 
point though as certain client+broker versions didn't support heartbeating but 
may still have had the config for it :)

I left it the way it has been changed to for the consistency, but I did note 
that that 0-10 spec actually specifies the timeout factor as 2x which is why 
that probably wasn't configurable to anything else previously (it still isn't 
configurable on the broker either).

I have attached the patch so you can take a look. Passed the 0-9-1 and 0-10 mms 
profiles. I will likely be MIA for the next few days so if you take a look and 
are happy with the changes just commit it.





> Unify client side heartbeat system properties/broker url options 
> -----------------------------------------------------------------
>
>                 Key: QPID-4534
>                 URL: https://issues.apache.org/jira/browse/QPID-4534
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Documentation, Java Client
>            Reporter: Keith Wall
>            Assignee: Robbie Gemmell
>         Attachments: 
> 0001-QPID-4534-unify-client-heartbeat-system-properties-c.patch, 
> 0001-QPID-4534-Unify-client-side-heartbeat-system-propert.patch
>
>
> The heartbeat controls/defaults offered by the 0-10 Java client are different 
> and to those used by the 0-8..0-9-1.  This has the potential to confuse the 
> end-user and makes clear documentation more difficult.
> The current situation looks like this:
> The 0-10 codepath defaults heart-beating to enabled with a period of 120s.  
> It may be configured on the client side by either:
> * broker url option ?heartbeat=... (value in secs)
> * system properties qpid.heartbeat (value in secs), amqj.heartbeat.delay 
> (secs), idle_timeout (ms).  The latter two are considered deprecated.
> The 0-8..0-9-1 codepath defaults heart-beating to disabled.    It may be 
> enabled on the client side using the system property amqj.heartbeat.delay.
> In the Java Broker's implementation of 0-8..0-9-1 allows heartbeating to be 
> enabled from the server side, by a mechanism where the server proposes a 
> heartbeat value to the client, and the client can either accept the value or 
> put forward its own.  The client does not understand the broker url option 
> 'heartbeat' nor the newer system property qpid.heartbeat.
> The Programming in Apache Qpid docbook reflects the 0-10 behaviour and makes 
> no mention to the differences in the other protocols.
> The intention is to change the code as follows:
> # make both code paths understand the qpid.heartbeat system property.  
> amqj.heartbeat.delay will continue to be supported as deprecated.
> # make the broker url option heartbeat=... will be understood by both code 
> paths
> # update docbook
>  



--
This message was sent by Atlassian JIRA
(v6.1#6144)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to