openstack/swift
Code Issues Proposed changes

Use a delta timeout for memcache where possible

Browse Source 
We use a delta timeout value for timeouts under 30 days (in seconds)
since that is the limit which the memcached protocols will recognize a
timeout as a delta. Greater than 30 days and it interprets it as an
absolute time in seconds since the epoch.

This helps to address an often difficult-to-debug problem of time
drift between memcache clients and the memcache servers. Prior to this
change, if a client's time drifts behind the servers, short timeouts
run the danger of not being cached at all. If a client's time drifts
ahead of the servers, short timeouts run the danger of persisting too
long. Using delta's avoids this affect. For absolute timeouts 30 days
or more in the future small time drifts between clients and servers
are inconsequential.

See also bug 1076148 (https://bugs.launchpad.net/swift/+bug/1076148).

This also fixes incr and decr to handle timeout values in the same way
timeouts are handled for set operations.

Change-Id: Ie36dbcedfe9b4db9f77ed4ea9b70ff86c5773310
Signed-off-by: Peter Portante <peter.portante@redhat.com>
This commit is contained in:
Peter Portante 2012-11-16 00:09:14 -05:00
parent ac91ab9e9d
commit 1ac7b88a27
2 changed files with 52 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
swift/common/memcached.py
View File

@ -53,6 +53,18 @@ def md5hash(key):
return md5(key).hexdigest()
def sanitize_timeout(timeout):
"""
Sanitize a timeout value to use an absolute expiration time if the delta
is greater than 30 days (in seconds). Note that the memcached server
translates negative values to mean a delta of 30 days in seconds (and 1
additional second), client beware.
"""
if timeout > (30 * 24 * 60 * 60):
timeout += time.time()
return timeout
class MemcacheConnectionError(Exception):
pass
@ -145,8 +157,7 @@ class MemcacheRing(object):
:param timeout: ttl in memcache
"""
key = md5hash(key)
if timeout > 0:
timeout += time.time()
timeout = sanitize_timeout(timeout)
flags = 0
if serialize and self._allow_pickle:
value = pickle.dumps(value, PICKLE_PROTOCOL)
@ -217,6 +228,7 @@ class MemcacheRing(object):
if delta < 0:
command = 'decr'
delta = str(abs(int(delta)))
timeout = sanitize_timeout(timeout)
for (server, fp, sock) in self._get_conns(key):
try:
sock.sendall('%s %s %s\r\n' % (command, key, delta))
@ -284,8 +296,7 @@ class MemcacheRing(object):
:param timeout: ttl for memcache
"""
server_key = md5hash(server_key)
if timeout > 0:
timeout += time.time()
timeout = sanitize_timeout(timeout)
msg = ''
for key, value in mapping.iteritems():
key = md5hash(key)

39
test/unit/common/test_memcached.py
View File

@ -168,6 +168,7 @@ class TestMemcached(unittest.TestCase):
memcache_client._client_cache['1.2.3.4:11211'] = [(mock, mock)] * 2
memcache_client.set('some_key', [1, 2, 3])
self.assertEquals(memcache_client.get('some_key'), [1, 2, 3])
self.assertEquals(mock.cache.values()[0][1], '0')
memcache_client.set('some_key', [4, 5, 6])
self.assertEquals(memcache_client.get('some_key'), [4, 5, 6])
memcache_client.set('some_key', ['simple str', 'utf8 str éà'])
@ -176,8 +177,11 @@ class TestMemcached(unittest.TestCase):
self.assertEquals(
memcache_client.get('some_key'), ['simple str', u'utf8 str éà'])
self.assert_(float(mock.cache.values()[0][1]) == 0)
esttimeout = time.time() + 10
memcache_client.set('some_key', [1, 2, 3], timeout=10)
self.assertEquals(mock.cache.values()[0][1], '10')
sixtydays = 60 * 24 * 60 * 60
esttimeout = time.time() + sixtydays
memcache_client.set('some_key', [1, 2, 3], timeout=sixtydays)
self.assert_(-1 <= float(mock.cache.values()[0][1]) - esttimeout <= 1)
def test_incr(self):
@ -198,6 +202,29 @@ class TestMemcached(unittest.TestCase):
self.assertRaises(memcached.MemcacheConnectionError,
memcache_client.incr, 'some_key', delta=-15)
def test_incr_w_timeout(self):
memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'])
mock = MockMemcached()
memcache_client._client_cache['1.2.3.4:11211'] = [(mock, mock)] * 2
memcache_client.incr('some_key', delta=5, timeout=55)
self.assertEquals(memcache_client.get('some_key'), '5')
self.assertEquals(mock.cache.values()[0][1], '55')
memcache_client.delete('some_key')
self.assertEquals(memcache_client.get('some_key'), None)
fiftydays = 50 * 24 * 60 * 60
esttimeout = time.time() + fiftydays
memcache_client.incr('some_key', delta=5, timeout=fiftydays)
self.assertEquals(memcache_client.get('some_key'), '5')
self.assert_(-1 <= float(mock.cache.values()[0][1]) - esttimeout <= 1)
memcache_client.delete('some_key')
self.assertEquals(memcache_client.get('some_key'), None)
memcache_client.incr('some_key', delta=5)
self.assertEquals(memcache_client.get('some_key'), '5')
self.assertEquals(mock.cache.values()[0][1], '0')
memcache_client.incr('some_key', delta=5, timeout=55)
self.assertEquals(memcache_client.get('some_key'), '10')
self.assertEquals(mock.cache.values()[0][1], '0')
def test_decr(self):
memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'])
mock = MockMemcached()
@ -244,10 +271,18 @@ class TestMemcached(unittest.TestCase):
self.assertEquals(
memcache_client.get_multi(('some_key2', 'some_key1'), 'multi_key'),
[[4, 5, 6], [1, 2, 3]])
esttimeout = time.time() + 10
self.assertEquals(mock.cache.values()[0][1], '0')
self.assertEquals(mock.cache.values()[1][1], '0')
memcache_client.set_multi(
{'some_key1': [1, 2, 3], 'some_key2': [4, 5, 6]}, 'multi_key',
timeout=10)
self.assertEquals(mock.cache.values()[0][1], '10')
self.assertEquals(mock.cache.values()[1][1], '10')
fortydays = 50 * 24 * 60 * 60
esttimeout = time.time() + fortydays
memcache_client.set_multi(
{'some_key1': [1, 2, 3], 'some_key2': [4, 5, 6]}, 'multi_key',
timeout=fortydays)
self.assert_(-1 <= float(mock.cache.values()[0][1]) - esttimeout <= 1)
self.assert_(-1 <= float(mock.cache.values()[1][1]) - esttimeout <= 1)
self.assertEquals(memcache_client.get_multi(