Swift MemcacheRing (set) interface is incompatible with python-memcached

[Guang Yee]

The set() interface in MemcacheRing is incompatible with python-memcached.

MemcacheRing:

def set(self, key, value, serialize=True, timeout=0):
        """
        Set a key/value pair in memcache

        :param key: key
        :param value: value
        :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
        """

python-memcached:

def set(self, key, val, time=0, min_compress_len=0):
        '''Unconditionally sets a key to a given value in the memcache.

        The C{key} can optionally be an tuple, with the first element
        being the server hash value and the second being the key.
        If you want to avoid making this module calculate a hash value.
        You may prefer, for example, to keep all of a given user's objects
        on the same memcache server, so you could use the user's unique
        id as the hash value.

        @return: Nonzero on success.
        @rtype: int
        @param time: Tells memcached the time which this value should expire, either
        as a delta number of seconds, or an absolute unix time-since-the-epoch
        value. See the memcached protocol docs section "Storage Commands"
        for more info on <exptime>. We default to 0 == cache forever.
        @param min_compress_len: The threshold length to kick in auto-compression
        of the value using the zlib.compress() routine. If the value being cached is
        a string, then the length of the string is measured, else if the value is an
        object, then the length of the pickle result is measured. If the resulting
        attempt at compression yeilds a larger string than the input, then it is
        discarded. For backwards compatability, this parameter defaults to 0,
        indicating don't ever try to compress.
        '''

Notice the named parameters inconsistencies: "time" and "timeout", "min_compress_len" and "serialize".

This inconsistency makes it difficult to switch between MemcacheRing and python-memcached. For service-neutral middleware such as Keystone auth_token, we have to special-case cache set if MemcacheRing is in used. For example,

if swift_cache:
    cache.set(key, value, timeout=60)
else:
    cache.set(key, value, time=60)

If would be nice if Swift MemcacheRing interfaces are compatible with python-memcached so service-neutral middlewares doesn't have to worry about which memcache client it is using.

[Samuel Merritt (torgomatic)]

Alright, so what does Keystone's auth_token care about? Your example mentions "time" vs. "timeout", and that would be a straightforward fix, as they're semantically equivalent.

Does auth_token ever use the "min_compress_len" parameter? Swift's MemcacheRing doesn't compress the stored values, so that parameter is meaningless there. If it's necessary (and only then), MemcacheRing could take a "min_compress_len" keyword arg that it ignores, which would at least give some compatibility.

python-memcached's "key" argument can be either a string or a 2-tuple, while MemcacheRing's can only be a string. Does auth_token ever use the 2-tuple form of that argument?

I don't want to introduce unused compatibility cruft, so if you could enumerate which pieces of incompatibility are causing pain in Keystone-land, that would be helpful.

[Guang Yee (guang-yee)]

I am more concerned about changing "time" to "timeout" at the moment. This could potentially cause security problems. Consider this ...

1. a valid token is cached with (timeout=0 as default), which means the token is cached for the duration at the discretion of memcached
2. token is revoked from Keystone
3. however, the token is still marked as valid in memcached for as long as it lives

Keystone auth_token don't use "min_compress_len". But I can't guarantee other (OpenStack or others) middleware won't. My point is we need a consistent memcache client interface (for OpenStack) so middleware don't need to know about its implementation.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/21553

[Tong Li (litong01)]

The patch has been submitted.
https://review.openstack.org/#/c/21553/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/21553
Committed: http://github.com/openstack/swift/commit/2267b030bfe5921f0cdde2facff284b4ceba3839
Submitter: Jenkins
Branch: master

commit 2267b030bfe5921f0cdde2facff284b4ceba3839
Author: Tong Li <email address hidden>
Date: Wed Feb 13 13:54:51 2013 -0500

    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