Swift MemcacheRing (set) interface is incompatible fixes

This patch fixes the Swift MemcacheRing set and set_multi
interface incompatible problem with python memcache. The fix
added two extra named parameters to both set and set_multi
method. When only time or timeout parameter is present, then one
of the value will be used. When both time and timeout are present,
the time parameter will be used.

Named parameter min_compress_len is added for pure compatibility
purposes. The current implementation ignores this parameter.

To make swift memcached methods all consistent cross the board,
method incr and decr have also been changed to include a new
named parameter time.

In future OpenStack releases, the named parameter timeout will be
removed, keep the named parameter timeout around for now is
to make sure that mismatched releases between client and server
will still work.

From now on, when a call is made to set, set_multi, decr, incr
by using timeout parametner, a warning message will be logged to
indicate the deprecation of the parameter.

Fixes: bug #1095730
Change-Id: I07af784a54d7d79395fc3265e74145f92f38a893

swift/common/memcached.py

@@ -142,7 +142,8 @@ class MemcacheRing(object):
         """ Returns a server connection to the pool """
         self._client_cache[server].append((fp, sock))
 
-    def set(self, key, value, serialize=True, timeout=0):
+    def set(self, key, value, serialize=True, timeout=0, time=0,
+            min_compress_len=0):
         """
         Set a key/value pair in memcache
 
@@ -151,10 +152,22 @@ class MemcacheRing(object):
         :param serialize: if True, value is serialized with JSON before sending
                           to memcache, or with pickle if configured to use
                           pickle instead of JSON (to avoid cache poisoning)
-        :param timeout: ttl in memcache
+        :param timeout: ttl in memcache, this parameter is now deprecated. It
+                        will be removed in next release of OpenStack,
+                        use time parameter instead in the future
+        :time: equivalent to timeout, this parameter is added to keep the
+               signature compatible with python-memcached interface. This
+               implementation will take this value and sign it to the
+               parameter timeout
+        :min_compress_len: minimum compress length, this parameter was added
+                           to keep the signature compatible with
+                           python-memcached interface. This implementation
+                           ignores it.
         """
         key = md5hash(key)
-        timeout = sanitize_timeout(timeout)
+        if timeout:
+            logging.warn("parameter timeout has been deprecated, use time")
+        timeout = sanitize_timeout(time or timeout)
         flags = 0
         if serialize and self._allow_pickle:
             value = pickle.dumps(value, PICKLE_PROTOCOL)
@@ -204,7 +217,7 @@ class MemcacheRing(object):
             except Exception, e:
                 self._exception_occurred(server, e)
 
-    def incr(self, key, delta=1, timeout=0):
+    def incr(self, key, delta=1, time=0, timeout=0):
         """
         Increments a key which has a numeric value by delta.
         If the key can't be found, it's added as delta or 0 if delta < 0.
@@ -217,15 +230,21 @@ class MemcacheRing(object):
         :param key: key
         :param delta: amount to add to the value of key (or set as the value
                       if the key is not found) will be cast to an int
-        :param timeout: ttl in memcache
+        :param time: the time to live. This parameter deprecates parameter
+                     timeout. The addition of this parameter is to make the
+                     interface consistent with set and set_multi methods
+        :param timeout: ttl in memcache, deprecated, will be removed in future
+                        OpenStack releases
         :raises MemcacheConnectionError:
         """
+        if timeout:
+            logging.warn("parameter timeout has been deprecated, use time")
         key = md5hash(key)
         command = 'incr'
         if delta < 0:
             command = 'decr'
         delta = str(abs(int(delta)))
-        timeout = sanitize_timeout(timeout)
+        timeout = sanitize_timeout(time or timeout)
         for (server, fp, sock) in self._get_conns(key):
             try:
                 sock.sendall('%s %s %s\r\n' % (command, key, delta))
@@ -251,7 +270,7 @@ class MemcacheRing(object):
                 self._exception_occurred(server, e)
         raise MemcacheConnectionError("No Memcached connections succeeded.")
 
-    def decr(self, key, delta=1, timeout=0):
+    def decr(self, key, delta=1, time=0, timeout=0):
         """
         Decrements a key which has a numeric value by delta. Calls incr with
         -delta.
@@ -260,10 +279,17 @@ class MemcacheRing(object):
         :param delta: amount to subtract to the value of key (or set the
                       value to 0 if the key is not found) will be cast to
                       an int
-        :param timeout: ttl in memcache
+        :param time: the time to live. This parameter depcates parameter
+                     timeout. The addition of this parameter is to make the
+                     interface consistent with set and set_multi methods
+        :param timeout: ttl in memcache, deprecated, will be removed in future
+                        OpenStack releases
         :raises MemcacheConnectionError:
         """
-        self.incr(key, delta=-delta, timeout=timeout)
+        if timeout:
+            logging.warn("parameter timeout has been deprecated, use time")
+
+        self.incr(key, delta=-delta, time=(time or timeout))
 
     def delete(self, key):
         """
@@ -280,7 +306,8 @@ class MemcacheRing(object):
             except Exception, e:
                 self._exception_occurred(server, e)
 
-    def set_multi(self, mapping, server_key, serialize=True, timeout=0):
+    def set_multi(self, mapping, server_key, serialize=True, timeout=0,
+                  time=0, min_compress_len=0):
         """
         Sets multiple key/value pairs in memcache.
 
@@ -290,10 +317,23 @@ class MemcacheRing(object):
         :param serialize: if True, value is serialized with JSON before sending
                           to memcache, or with pickle if configured to use
                           pickle instead of JSON (to avoid cache poisoning)
-        :param timeout: ttl for memcache
+        :param timeout: ttl for memcache. This parameter is now deprecated, it
+                        will be removed in next release of OpenStack, use time
+                        parameter instead in the future
+        :time: equalvent to timeout, this parameter is added to keep the
+               signature compatible with python-memcached interface. This
+               implementation will take this value and sign it to parameter
+               timeout
+        :min_compress_len: minimum compress length, this parameter was added
+                           to keep the signature compatible with
+                           python-memcached interface. This implementation
+                           ignores it
         """
+        if timeout:
+            logging.warn("parameter timeout has been deprecated, use time")
+
         server_key = md5hash(server_key)
-        timeout = sanitize_timeout(timeout)
+        timeout = sanitize_timeout(time or timeout)
         msg = ''
         for key, value in mapping.iteritems():
             key = md5hash(key)

swift/common/middleware/cname_lookup.py

@@ -108,7 +108,7 @@ class CNAMELookupMiddleware(object):
                     if self.memcache:
                         memcache_key = ''.join(['cname-', given_domain])
                         self.memcache.set(memcache_key, found_domain,
-                                          timeout=ttl)
+                                          time=ttl)
                 if found_domain is None or found_domain == a_domain:
                     # no CNAME records or we're at the last lookup
                     error = True

swift/common/middleware/formpost.py

@@ -487,7 +487,7 @@ class FormPost(object):
                 pass
             key = key[0]
             if key and memcache:
-                memcache.set('temp-url-key/%s' % account, key, timeout=60)
+                memcache.set('temp-url-key/%s' % account, key, time=60)
         return key
 
 

swift/common/middleware/staticweb.py

@@ -213,7 +213,7 @@ class _StaticWebContext(WSGIContext):
                 memcache_client.set(memcache_key,
                                     (self._index, self._error, self._listings,
                                      self._listings_css),
-                                    timeout=self.cache_timeout)
+                                    time=self.cache_timeout)
 
     def _listing(self, env, start_response, prefix=None):
         """

swift/common/middleware/tempauth.py

@@ -441,12 +441,12 @@ class TempAuth(object):
             # Save token
             memcache_token_key = '%s/token/%s' % (self.reseller_prefix, token)
             memcache_client.set(memcache_token_key, (expires, groups),
-                                timeout=float(expires - time()))
+                                time=float(expires - time()))
             # Record the token with the user info for future use.
             memcache_user_key = \
                 '%s/user/%s' % (self.reseller_prefix, account_user)
             memcache_client.set(memcache_user_key, token,
-                                timeout=float(expires - time()))
+                                time=float(expires - time()))
         resp = Response(request=req, headers={
             'x-auth-token': token, 'x-storage-token': token})
         url = self.users[account_user]['url'].replace('$HOST', resp.host_url)

swift/common/middleware/tempurl.py

@@ -343,7 +343,7 @@ class TempURL(object):
                 pass
             key = key[0]
             if key and memcache:
-                memcache.set('temp-url-key/%s' % account, key, timeout=60)
+                memcache.set('temp-url-key/%s' % account, key, time=60)
         return key
 
     def _get_hmac(self, env, expires, key, request_method=None):

swift/proxy/controllers/base.py

@@ -401,7 +401,7 @@ class Controller(object):
             self.app.memcache.set(cache_key,
                                   {'status': result_code,
                                   'container_count': container_count},
-                                  timeout=cache_timeout)
+                                  time=cache_timeout)
         if result_code == HTTP_OK:
             return partition, nodes, container_count
         return None, None, None
@@ -472,11 +472,11 @@ class Controller(object):
             if container_info['status'] == HTTP_OK:
                 self.app.memcache.set(
                     cache_key, container_info,
-                    timeout=self.app.recheck_container_existence)
+                    time=self.app.recheck_container_existence)
             elif container_info['status'] == HTTP_NOT_FOUND:
                 self.app.memcache.set(
                     cache_key, container_info,
-                    timeout=self.app.recheck_container_existence * 0.1)
+                    time=self.app.recheck_container_existence * 0.1)
         if container_info['status'] == HTTP_OK:
             container_info['partition'] = part
             container_info['nodes'] = nodes

swift/proxy/controllers/container.py

@@ -79,7 +79,7 @@ class ContainerController(Controller):
             self.app.memcache.set(
                 cache_key,
                 headers_to_container_info(resp.headers, resp.status_int),
-                timeout=self.app.recheck_container_existence)
+                time=self.app.recheck_container_existence)
 
         if 'swift.authorize' in req.environ:
             req.acl = resp.headers.get('x-container-read')

test/unit/common/middleware/test_formpost.py

@@ -32,7 +32,7 @@ class FakeMemcache(object):
     def get(self, key):
         return self.store.get(key)
 
-    def set(self, key, value, timeout=0):
+    def set(self, key, value, time=0):
         self.store[key] = value
         return True
 

test/unit/common/middleware/test_ratelimit.py

@@ -36,11 +36,11 @@ class FakeMemcache(object):
     def get(self, key):
         return self.store.get(key)
 
-    def set(self, key, value, serialize=False, timeout=0):
+    def set(self, key, value, serialize=False, time=0):
         self.store[key] = value
         return True
 
-    def incr(self, key, delta=1, timeout=0):
+    def incr(self, key, delta=1, time=0):
         if self.error_on_incr:
             raise MemcacheConnectionError('Memcache restarting')
         if self.init_incr_return_neg:
@@ -52,8 +52,8 @@ class FakeMemcache(object):
             self.store[key] = 0
         return int(self.store[key])
 
-    def decr(self, key, delta=1, timeout=0):
-        return self.incr(key, delta=-delta, timeout=timeout)
+    def decr(self, key, delta=1, time=0):
+        return self.incr(key, delta=-delta, time=time)
 
     @contextmanager
     def soft_lock(self, key, timeout=0, retries=5):

test/unit/common/middleware/test_staticweb.py

@@ -33,11 +33,11 @@ class FakeMemcache(object):
     def get(self, key):
         return self.store.get(key)
 
-    def set(self, key, value, timeout=0):
+    def set(self, key, value, time=0):
         self.store[key] = value
         return True
 
-    def incr(self, key, timeout=0):
+    def incr(self, key, time=0):
         self.store[key] = self.store.setdefault(key, 0) + 1
         return self.store[key]
 

test/unit/common/middleware/test_tempauth.py

@@ -29,11 +29,11 @@ class FakeMemcache(object):
     def get(self, key):
         return self.store.get(key)
 
-    def set(self, key, value, timeout=0):
+    def set(self, key, value, time=0):
         self.store[key] = value
         return True
 
-    def incr(self, key, timeout=0):
+    def incr(self, key, time=0):
         self.store[key] = self.store.setdefault(key, 0) + 1
         return self.store[key]
 

test/unit/common/middleware/test_tempurl.py

@@ -31,11 +31,11 @@ class FakeMemcache(object):
     def get(self, key):
         return self.store.get(key)
 
-    def set(self, key, value, timeout=0):
+    def set(self, key, value, time=0):
         self.store[key] = value
         return True
 
-    def incr(self, key, timeout=0):
+    def incr(self, key, time=0):
         self.store[key] = self.store.setdefault(key, 0) + 1
         return self.store[key]
 

test/unit/common/test_memcached.py

@@ -179,10 +179,15 @@ class TestMemcached(unittest.TestCase):
         self.assert_(float(mock.cache.values()[0][1]) == 0)
         memcache_client.set('some_key', [1, 2, 3], timeout=10)
         self.assertEquals(mock.cache.values()[0][1], '10')
+        memcache_client.set('some_key', [1, 2, 3], time=20)
+        self.assertEquals(mock.cache.values()[0][1], '20')
+
         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)
+        memcache_client.set('some_key', [1, 2, 3], time=sixtydays)
+        self.assert_(-1 <= float(mock.cache.values()[0][1]) - esttimeout <= 1)
 
     def test_incr(self):
         memcache_client = memcached.MemcacheRing(['1.2.3.4:11211'])
@@ -206,14 +211,14 @@ class TestMemcached(unittest.TestCase):
         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)
+        memcache_client.incr('some_key', delta=5, time=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)
+        memcache_client.incr('some_key', delta=5, time=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')
@@ -221,7 +226,7 @@ class TestMemcached(unittest.TestCase):
         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)
+        memcache_client.incr('some_key', delta=5, time=55)
         self.assertEquals(memcache_client.get('some_key'), '10')
         self.assertEquals(mock.cache.values()[0][1], '0')
 
@@ -278,6 +283,12 @@ class TestMemcached(unittest.TestCase):
             timeout=10)
         self.assertEquals(mock.cache.values()[0][1], '10')
         self.assertEquals(mock.cache.values()[1][1], '10')
+        memcache_client.set_multi(
+            {'some_key1': [1, 2, 3], 'some_key2': [4, 5, 6]}, 'multi_key',
+            time=20)
+        self.assertEquals(mock.cache.values()[0][1], '20')
+        self.assertEquals(mock.cache.values()[1][1], '20')
+
         fortydays = 50 * 24 * 60 * 60
         esttimeout = time.time() + fortydays
         memcache_client.set_multi(

test/unit/proxy/test_server.py

@@ -358,11 +358,11 @@ class FakeMemcache(object):
     def keys(self):
         return self.store.keys()
 
-    def set(self, key, value, timeout=0):
+    def set(self, key, value, time=0):
         self.store[key] = value
         return True
 
-    def incr(self, key, timeout=0):
+    def incr(self, key, time=0):
         self.store[key] = self.store.setdefault(key, 0) + 1
         return self.store[key]