Fix up how we memcache on py3

Previously, we stored the WSGI strings in memcached and returned them when
responding to get_account/container_info calls. This would lead to cache
corruption in a heterogenous py2/py3 cluster such as you would have during
a rolling upgrade.

Now, only store and return native strings.

Change-Id: I8d6f66dfe846493972e433f70bad76a33d204562
This commit is contained in:
Tim Burke 2019-05-29 18:14:17 -07:00
parent 74e1f2e053
commit ef8818a639
4 changed files with 54 additions and 8 deletions

View File

@ -138,7 +138,7 @@ class MemcacheConnPool(Pool):
sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1) sock.setsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY, 1)
with Timeout(self._connect_timeout): with Timeout(self._connect_timeout):
sock.connect(sockaddr) sock.connect(sockaddr)
return (sock.makefile(), sock) return (sock.makefile('rwb'), sock)
def get(self): def get(self):
fp, sock = super(MemcacheConnPool, self).get() fp, sock = super(MemcacheConnPool, self).get()

View File

@ -123,7 +123,8 @@ def _prep_headers_to_info(headers, server_type):
sysmeta = {} sysmeta = {}
other = {} other = {}
for key, val in dict(headers).items(): for key, val in dict(headers).items():
lkey = key.lower() lkey = wsgi_to_str(key).lower()
val = wsgi_to_str(val) if isinstance(val, str) else val
if is_user_meta(server_type, lkey): if is_user_meta(server_type, lkey):
meta[strip_user_meta_prefix(server_type, lkey)] = val meta[strip_user_meta_prefix(server_type, lkey)] = val
elif is_sys_meta(server_type, lkey): elif is_sys_meta(server_type, lkey):
@ -450,8 +451,22 @@ def get_cache_key(account, container=None, obj=None):
:param account: The name of the account :param account: The name of the account
:param container: The name of the container (or None if account) :param container: The name of the container (or None if account)
:param obj: The name of the object (or None if account or container) :param obj: The name of the object (or None if account or container)
:returns: a string cache_key :returns: a (native) string cache_key
""" """
if six.PY2:
def to_native(s):
if s is None or isinstance(s, str):
return s
return s.encode('utf8')
else:
def to_native(s):
if s is None or isinstance(s, str):
return s
return s.decode('utf8', 'surrogateescape')
account = to_native(account)
container = to_native(container)
obj = to_native(obj)
if obj: if obj:
if not (account and container): if not (account and container):

View File

@ -19,6 +19,7 @@
from collections import defaultdict from collections import defaultdict
import errno import errno
from hashlib import md5 from hashlib import md5
import io
import six import six
import socket import socket
import time import time
@ -206,6 +207,9 @@ class TestMemcached(unittest.TestCase):
while one or two: # Run until we match hosts one and two while one or two: # Run until we match hosts one and two
key = uuid4().hex.encode('ascii') key = uuid4().hex.encode('ascii')
for conn in memcache_client._get_conns(key): for conn in memcache_client._get_conns(key):
if 'b' not in getattr(conn[1], 'mode', ''):
self.assertIsInstance(conn[1], (
io.RawIOBase, io.BufferedIOBase))
peeripport = '%s:%s' % conn[2].getpeername() peeripport = '%s:%s' % conn[2].getpeername()
self.assertTrue(peeripport in (sock1ipport, sock2ipport)) self.assertTrue(peeripport in (sock1ipport, sock2ipport))
if peeripport == sock1ipport: if peeripport == sock1ipport:

View File

@ -25,7 +25,8 @@ from swift.proxy.controllers.base import headers_to_container_info, \
headers_to_account_info, headers_to_object_info, get_container_info, \ headers_to_account_info, headers_to_object_info, get_container_info, \
get_cache_key, get_account_info, get_info, get_object_info, \ get_cache_key, get_account_info, get_info, get_object_info, \
Controller, GetOrHeadHandler, bytes_to_skip Controller, GetOrHeadHandler, bytes_to_skip
from swift.common.swob import Request, HTTPException, RESPONSE_REASONS from swift.common.swob import Request, HTTPException, RESPONSE_REASONS, \
bytes_to_wsgi
from swift.common import exceptions from swift.common import exceptions
from swift.common.utils import split_path, ShardRange, Timestamp from swift.common.utils import split_path, ShardRange, Timestamp
from swift.common.header_key_dict import HeaderKeyDict from swift.common.header_key_dict import HeaderKeyDict
@ -73,6 +74,8 @@ class ContainerResponse(FakeResponse):
base_headers = { base_headers = {
'x-container-object-count': 1000, 'x-container-object-count': 1000,
'x-container-bytes-used': 6666, 'x-container-bytes-used': 6666,
'x-versions-location': bytes_to_wsgi(
u'\U0001F334'.encode('utf8')),
} }
@ -353,6 +356,10 @@ class TestFuncs(unittest.TestCase):
self.assertEqual(resp['storage_policy'], 0) self.assertEqual(resp['storage_policy'], 0)
self.assertEqual(resp['bytes'], 6666) self.assertEqual(resp['bytes'], 6666)
self.assertEqual(resp['object_count'], 1000) self.assertEqual(resp['object_count'], 1000)
expected = u'\U0001F334'
if six.PY2:
expected = expected.encode('utf8')
self.assertEqual(resp['versions'], expected)
def test_get_container_info_no_account(self): def test_get_container_info_no_account(self):
app = FakeApp(statuses=[404, 200]) app = FakeApp(statuses=[404, 200])
@ -382,10 +389,11 @@ class TestFuncs(unittest.TestCase):
self.assertEqual(resp['bytes'], 3333) self.assertEqual(resp['bytes'], 3333)
self.assertEqual(resp['object_count'], 10) self.assertEqual(resp['object_count'], 10)
self.assertEqual(resp['status'], 404) self.assertEqual(resp['status'], 404)
if six.PY3: expected = u'\U0001F4A9'
self.assertEqual(resp['versions'], u'\U0001f4a9') if six.PY2:
else: expected = expected.encode('utf8')
self.assertEqual(resp['versions'], "\xf0\x9f\x92\xa9") self.assertEqual(resp['versions'], expected)
for subdict in resp.values(): for subdict in resp.values():
if isinstance(subdict, dict): if isinstance(subdict, dict):
self.assertEqual([(k, type(k), v, type(v)) self.assertEqual([(k, type(k), v, type(v))
@ -393,6 +401,25 @@ class TestFuncs(unittest.TestCase):
[(k, str, v, str) [(k, str, v, str)
for k, v in subdict.items()]) for k, v in subdict.items()])
def test_get_cache_key(self):
self.assertEqual(get_cache_key("account", "cont"),
'container/account/cont')
self.assertEqual(get_cache_key(b"account", b"cont", b'obj'),
'object/account/cont/obj')
self.assertEqual(get_cache_key(u"account", u"cont", b'obj'),
'object/account/cont/obj')
# Expected result should always be native string
expected = u'container/\N{SNOWMAN}/\U0001F334'
if six.PY2:
expected = expected.encode('utf8')
self.assertEqual(get_cache_key(u"\N{SNOWMAN}", u"\U0001F334"),
expected)
self.assertEqual(get_cache_key(u"\N{SNOWMAN}".encode('utf8'),
u"\U0001F334".encode('utf8')),
expected)
def test_get_container_info_env(self): def test_get_container_info_env(self):
cache_key = get_cache_key("account", "cont") cache_key = get_cache_key("account", "cont")
req = Request.blank( req = Request.blank(