Absolute/relative expire time and MemcachedClient.GetExpiration #96

Closed
leeliu opened this Issue Feb 1, 2012 · 2 comments

Projects

None yet

4 participants

@leeliu
leeliu commented Feb 1, 2012

The GetExpiration method was changed to absolute times to support > 30 days expiration or at least that's how I understand it...correct me if I'm wrong. I didn't realize this change happened during my recent update to the latest client. My application sets expiries < 5 seconds (for rate throttling purposes on certain API functions). Recently my web server and memcache server started drifting in time sync and it was causing crazy problems with rate throttling and some keys expiring immediately after set.

My question is this: If the 30 days is the reason for the change, why not autodetect the expiry time?
ie:
If < 30 days, use relative times
If > 30 days, use absolute times

That should work well for just about everybody and then something like a clock drift between servers wouldn't cause any major issues. Thoughts?

Lee

@rrmayer
rrmayer commented Mar 17, 2013

The existing codebase is in conflict with the Memcached specification. Per the specification:

Some commands involve a client sending some kind of expiration time (relative to an item or to an operation requested by the client) to the server. In all such cases, the actual value sent may either be Unix time (number of seconds since January 1, 1970, as a 32-bit value), or a number of seconds starting from current time. In the latter case, this number of seconds may not exceed 60_60_24*30 (number of seconds in 30 days); if the number sent by a client is larger than that, the server will consider it to be real Unix time value rather than an offset from current time.

I have implemented this change in my branch of the code. I will be committing my branch momentarily. In addition, I will eventually create a pull request with these changes after I have tested them.

Note: in my change, there was an improper use of nullable types instead of method overloads (the method signature was GetExpiration(TimeSpan?, DateTime?). This was unwieldy and resulted in duplicate code, so I pulled those two methods out into their own separate overloads.

@rrmayer rrmayer pushed a commit to rrmayer/EnyimMemcached that referenced this issue Mar 17, 2013
Rob Mayer Fixes expiration date issue
Ref. Issue #96 (enyim#96)

Signed-off-by: Rob Mayer <rmayer@stewartstrategy.com>
5cc4462
@rrmayer rrmayer pushed a commit to rrmayer/EnyimMemcached that referenced this issue Mar 18, 2013
Rob Mayer Fixes expiration date issue
Ref. Issue #96 (enyim#96)

Signed-off-by: Rob Mayer <rmayer@stewartstrategy.com>
fb9cf0e
@rrmayer rrmayer pushed a commit to rrmayer/EnyimMemcached that referenced this issue Mar 18, 2013
Rob Mayer Fixes expiration date issue
Ref. Issue #96 (enyim#96)

Signed-off-by: Rob Mayer <rmayer@stewartstrategy.com>
126c6ee
@rrmayer rrmayer referenced this issue Mar 18, 2013
Closed

Issue 96 #116

@rrmayer rrmayer pushed a commit to rrmayer/EnyimMemcached that referenced this issue Mar 18, 2013
Rob Mayer Fixes expiration date issue
Ref. Issue #96 (enyim#96)

Signed-off-by: Rob Mayer <rmayer@stewartstrategy.com>
7269cc4
@rrmayer rrmayer referenced this issue in couchbase/EnyimMemcached Mar 18, 2013
Open

Issue 96 #9

@rrmayer rrmayer pushed a commit to rrmayer/EnyimMemcached that referenced this issue Mar 18, 2013
Rob Mayer Fixes expiration date issue
Ref. Issue #96 (enyim#96)

Signed-off-by: Rob Mayer <rmayer@stewartstrategy.com>
aac4d46
@ashapka
ashapka commented Jan 23, 2015

Hi,

I've noticed that a similar fix exists in a beta EnyimMemcached2.

Are there any plans to release a new version of the EnyimMemcached (e.g. 2.13) with this fix?

Thanks!

@enyim enyim closed this Apr 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment