Use a delta timeout for memcache where possible

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>

swift/common/memcached.py

@@ -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 @@ def set(self, key, value, serialize=True, timeout=0):
         :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 @@ def incr(self, key, delta=1, timeout=0):
         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 @@ def set_multi(self, mapping, server_key, serialize=True, timeout=0):
         :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)

test/unit/common/test_memcached.py

@@ -168,6 +168,7 @@ def test_set_get(self):
         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 @@ def test_set_get(self):
         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 @@ def test_incr(self):
         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 @@ def test_multi(self):
         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(