Add some reasonably long and configurable absolute time limits.

[GoogleCodeExporter]

Node failures currently cause indefinite client hangs as there's
effectively an infinite timeout.  I should make this a finite timeout with
autocancellation to deal with failures.

Original issue reported on code.google.com by dsalli...@gmail.com on 2 Mar 2008 at 5:56

[GoogleCodeExporter]

Original comment by dsalli...@gmail.com on 4 Mar 2008 at 12:15

• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

perhaps another way at looking at this is to create a configurable timeout, 
that can 
be adjusted by the user.  for example, on the asyncget() command, you have a 
configurable timeout.  What would be interesting would be to have a timeout 
that 
could be configured on a clientwide and possibly overridden on a per request 
basis.

Original comment by goo...@wokits.com on 7 Mar 2008 at 11:17

[GoogleCodeExporter]

I mentioned on the mailing list that the incr and decr functions have no 
timeouts at
all currently. I am providing a patch here that adds overloads for those 
functions as
well as a default timeout for 1000 millis for the original methods.

We need a way to make the default timeout configurable, or at least take an
opinionated stance on what the default should be set at.

Original comment by raykrue...@gmail.com on 19 Mar 2008 at 6:29

Attachments:

• MemcachedClient.java.patch

[GoogleCodeExporter]

  Is it really valuable to have per-method overrides of these timeouts?

  I ask because we've added a couple of variations on some of these methods now and
java doesn't have a way to control that growth short of adding an Options 
object of
some sort.  As it is, every slight variation multiplies the number of methods.

  If it would be sufficient to set a global option, that would be far preferable.

Original comment by dsalli...@gmail.com on 19 Mar 2008 at 6:45

[GoogleCodeExporter]

I agree it's totally filthy.
Actually I didn't even want to include TimeUnit in the signature. The problem 
with
that was the incr(with override) and incr(without override, with timeout) 
collide.

I'm all for the global option at the end of the day. Whatever makes it not 
infinite :)

Original comment by raykrue...@gmail.com on 19 Mar 2008 at 6:52

[GoogleCodeExporter]

More and more I come across scenarios where this global timeout is really a
must-have. We cant have things like incr, decr, and getStats hanging 
indefinitely
till a missing memcached comes back. I've begun a patch around the global 
timeout for
places where we're waiting on a latch (like the ops I mentioned).

Adding timeouts to places where we're using Futures becomes an exercise in 
exception
handling. Any suggestions on what you'd like to do there? Eat it and return 
null?
Re-throw it as a RuntimeException? Create our own TimeoutException that extends
RuntimeException? Add it to Throws? 

Original comment by raykrue...@gmail.com on 26 Mar 2008 at 1:31

[GoogleCodeExporter]

Yes this is the problem I ran into when I last tried to do something with it.  
I'm
not a huge fan of checked exceptions for this reason.  I kind of like the idea 
of
making an unchecked timeout exception for this purpose.

It's somewhat rare that someone should see a timeout in practice, but it should 
also
not be very surprising when it happens.  A clear timeout message and a mention 
in the
docs is probably better than telling everyone they need to handle it all the 
time as
a checked exception.

Original comment by dsalli...@gmail.com on 29 Mar 2008 at 7:07

[GoogleCodeExporter]

Much thanks to Ray for doing this fix.  1eb2c9ec10c0ec37b3c04fa4b9fd874a642cb899

Original comment by dsalli...@gmail.com on 2 May 2008 at 3:55

• Changed state: Fixed