You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Rajith Attapattu <ra...@gmail.com> on 2012/08/09 17:10:23 UTC

[JMS] Failover Timeout

While looking up the code to answer a question from a user, I noticed
that we don't use "connecttimeout" any where.
This property is retrieved using the "getTimeout()" method in
AMQBrokerDetails.java

However the getTimeout() method (or the property) is not used any
where else in the code.
Instead it appears we use "qpid.failover_method_timeout" with a
default of 120000
(We should also document these system properties in the docs)

Is this behaviour correct and as expected ?
Could anybody who is familiar with this change/feature shed some light on this?

Regards,

Rajith

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Re: [JMS] Failover Timeout

Posted by Rajith Attapattu <ra...@gmail.com>.
On Thu, Aug 9, 2012 at 4:51 PM, Robbie Gemmell <ro...@gmail.com> wrote:
> On 9 August 2012 21:31, Rajith Attapattu <ra...@gmail.com> wrote:
>
>> On Thu, Aug 9, 2012 at 4:13 PM, Robbie Gemmell <ro...@gmail.com>
>> wrote:
>> > No, "connecttimeout" being unused isn't intended, but it appears to have
>> > been that way since 0.6 went out, see:
>> > https://issues.apache.org/jira/browse/QPID-4051.
>>
>> So perhaps a bug ? perhaps we should fix this for the next release ?
>>
>>
> Defintiely a bug, I actually meant to fix it for this release but you can
> kinda tell from the comments on the JIRA that I went on holiday (<gasp>)
> and forgot about it :P

I saw your comments on the JIRA :)
Now that we are all working on the same code base we tend to catch
things more often.

>
>> > "qpid.failover_method_timeout" is not intended to be used instead of it
>> but
>> > is instead a very recently introduced system property that can be used to
>> > control a previously hard coded value in an entirely different area of
>> > code. As someone once said in a commit message "hard coding makes coding
>> > hard". This is a good example of one of those 'we dont really expect/want
>> > people to use and so shouldnt bother to confuse them by documenting it'
>> > system properties I just mentioned in my other email.
>>
>> However due to connecttimeout not working (or not used) it appears
>> this property was somewhat useful to an end user.
>> But I agree that if this isn't generally useful then better not to
>> document it.
>> Instead maybe we should fix connecttimeout.
>>
>>
> Given that connecttimeout has been broken for 3 years before being noticed
> I wouldn't bother to document this one (which actually does something
> different, not normally all that useful to a user, particularly if they
> need to timeout a connect() operation) but instead fix connecttimeout which
> is documented and only ever mention this if its actually necessary.
>
> We have put a connect() timeout in for 0.16 based on a user patch
> (QPID-4047), albeit it isnt configurable due to QPID-4051 but in this case
> thats still better than no timeout in the previous releases.
>
>
>> P.S  I guess we will have to do some of these minor fixes until we
>> provide a fully working new JMS client.
>> The connect timeout seems a useful feature when used with the
>> singlebroker situation.
>>
>
> Agreed, a connect() timeout could be useful with any connection situation
> so we should definitely fix it.

Indeed! Thanks Robbie!

Rajith


>
>> Rajith
>>
>> > Robbie
>>
>>
>> >
>> > On 9 August 2012 16:10, Rajith Attapattu <ra...@gmail.com> wrote:
>> >
>> >> While looking up the code to answer a question from a user, I noticed
>> >> that we don't use "connecttimeout" any where.
>> >> This property is retrieved using the "getTimeout()" method in
>> >> AMQBrokerDetails.java
>> >>
>> >> However the getTimeout() method (or the property) is not used any
>> >> where else in the code.
>> >> Instead it appears we use "qpid.failover_method_timeout" with a
>> >> default of 120000
>> >> (We should also document these system properties in the docs)
>> >>
>> >> Is this behaviour correct and as expected ?
>> >> Could anybody who is familiar with this change/feature shed some light
>> on
>> >> this?
>> >>
>> >> Regards,
>> >>
>> >> Rajith
>> >>
>> >> ---------------------------------------------------------------------
>> >> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
>> >> For additional commands, e-mail: dev-help@qpid.apache.org
>> >>
>> >>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
>> For additional commands, e-mail: dev-help@qpid.apache.org
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Re: [JMS] Failover Timeout

Posted by Robbie Gemmell <ro...@gmail.com>.
On 9 August 2012 21:31, Rajith Attapattu <ra...@gmail.com> wrote:

> On Thu, Aug 9, 2012 at 4:13 PM, Robbie Gemmell <ro...@gmail.com>
> wrote:
> > No, "connecttimeout" being unused isn't intended, but it appears to have
> > been that way since 0.6 went out, see:
> > https://issues.apache.org/jira/browse/QPID-4051.
>
> So perhaps a bug ? perhaps we should fix this for the next release ?
>
>
Defintiely a bug, I actually meant to fix it for this release but you can
kinda tell from the comments on the JIRA that I went on holiday (<gasp>)
and forgot about it :P


> > "qpid.failover_method_timeout" is not intended to be used instead of it
> but
> > is instead a very recently introduced system property that can be used to
> > control a previously hard coded value in an entirely different area of
> > code. As someone once said in a commit message "hard coding makes coding
> > hard". This is a good example of one of those 'we dont really expect/want
> > people to use and so shouldnt bother to confuse them by documenting it'
> > system properties I just mentioned in my other email.
>
> However due to connecttimeout not working (or not used) it appears
> this property was somewhat useful to an end user.
> But I agree that if this isn't generally useful then better not to
> document it.
> Instead maybe we should fix connecttimeout.
>
>
Given that connecttimeout has been broken for 3 years before being noticed
I wouldn't bother to document this one (which actually does something
different, not normally all that useful to a user, particularly if they
need to timeout a connect() operation) but instead fix connecttimeout which
is documented and only ever mention this if its actually necessary.

We have put a connect() timeout in for 0.16 based on a user patch
(QPID-4047), albeit it isnt configurable due to QPID-4051 but in this case
thats still better than no timeout in the previous releases.


> P.S  I guess we will have to do some of these minor fixes until we
> provide a fully working new JMS client.
> The connect timeout seems a useful feature when used with the
> singlebroker situation.
>

Agreed, a connect() timeout could be useful with any connection situation
so we should definitely fix it.


> Rajith
>
> > Robbie
>
>
> >
> > On 9 August 2012 16:10, Rajith Attapattu <ra...@gmail.com> wrote:
> >
> >> While looking up the code to answer a question from a user, I noticed
> >> that we don't use "connecttimeout" any where.
> >> This property is retrieved using the "getTimeout()" method in
> >> AMQBrokerDetails.java
> >>
> >> However the getTimeout() method (or the property) is not used any
> >> where else in the code.
> >> Instead it appears we use "qpid.failover_method_timeout" with a
> >> default of 120000
> >> (We should also document these system properties in the docs)
> >>
> >> Is this behaviour correct and as expected ?
> >> Could anybody who is familiar with this change/feature shed some light
> on
> >> this?
> >>
> >> Regards,
> >>
> >> Rajith
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> >> For additional commands, e-mail: dev-help@qpid.apache.org
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
>
>

Re: [JMS] Failover Timeout

Posted by Rajith Attapattu <ra...@gmail.com>.
On Thu, Aug 9, 2012 at 4:13 PM, Robbie Gemmell <ro...@gmail.com> wrote:
> No, "connecttimeout" being unused isn't intended, but it appears to have
> been that way since 0.6 went out, see:
> https://issues.apache.org/jira/browse/QPID-4051.

So perhaps a bug ? perhaps we should fix this for the next release ?

> "qpid.failover_method_timeout" is not intended to be used instead of it but
> is instead a very recently introduced system property that can be used to
> control a previously hard coded value in an entirely different area of
> code. As someone once said in a commit message "hard coding makes coding
> hard". This is a good example of one of those 'we dont really expect/want
> people to use and so shouldnt bother to confuse them by documenting it'
> system properties I just mentioned in my other email.

However due to connecttimeout not working (or not used) it appears
this property was somewhat useful to an end user.
But I agree that if this isn't generally useful then better not to document it.
Instead maybe we should fix connecttimeout.

P.S  I guess we will have to do some of these minor fixes until we
provide a fully working new JMS client.
The connect timeout seems a useful feature when used with the
singlebroker situation.

Rajith

> Robbie


>
> On 9 August 2012 16:10, Rajith Attapattu <ra...@gmail.com> wrote:
>
>> While looking up the code to answer a question from a user, I noticed
>> that we don't use "connecttimeout" any where.
>> This property is retrieved using the "getTimeout()" method in
>> AMQBrokerDetails.java
>>
>> However the getTimeout() method (or the property) is not used any
>> where else in the code.
>> Instead it appears we use "qpid.failover_method_timeout" with a
>> default of 120000
>> (We should also document these system properties in the docs)
>>
>> Is this behaviour correct and as expected ?
>> Could anybody who is familiar with this change/feature shed some light on
>> this?
>>
>> Regards,
>>
>> Rajith
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
>> For additional commands, e-mail: dev-help@qpid.apache.org
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org


Re: [JMS] Failover Timeout

Posted by Robbie Gemmell <ro...@gmail.com>.
No, "connecttimeout" being unused isn't intended, but it appears to have
been that way since 0.6 went out, see:
https://issues.apache.org/jira/browse/QPID-4051.

"qpid.failover_method_timeout" is not intended to be used instead of it but
is instead a very recently introduced system property that can be used to
control a previously hard coded value in an entirely different area of
code. As someone once said in a commit message "hard coding makes coding
hard". This is a good example of one of those 'we dont really expect/want
people to use and so shouldnt bother to confuse them by documenting it'
system properties I just mentioned in my other email.

Robbie

On 9 August 2012 16:10, Rajith Attapattu <ra...@gmail.com> wrote:

> While looking up the code to answer a question from a user, I noticed
> that we don't use "connecttimeout" any where.
> This property is retrieved using the "getTimeout()" method in
> AMQBrokerDetails.java
>
> However the getTimeout() method (or the property) is not used any
> where else in the code.
> Instead it appears we use "qpid.failover_method_timeout" with a
> default of 120000
> (We should also document these system properties in the docs)
>
> Is this behaviour correct and as expected ?
> Could anybody who is familiar with this change/feature shed some light on
> this?
>
> Regards,
>
> Rajith
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
> For additional commands, e-mail: dev-help@qpid.apache.org
>
>