openstack/swift
Code Issues Proposed changes

Remove the X-Newest pre-flight request on X-Timestamp

Browse Source 
There is a standard LBYL race that can better be addressed by making the
EAFP case safer.

Capture 409 Conflict during expect on PUT

Similarly to how the proxy handles 412 on PUT, we will gather 409
responses in the proxy during _connect_put_node.  Rather than skipping
backend servers that already have a synced copy of an object we will
accept their response and return 202 immediately.

This is particularly useful to internal clients who are using
X-Timestamp to sync transfers (e.g. container-sync and
container-reconciler).

No observable change in client facing behavior except that swift is
faster to respond Accepted when it already has the data the client is
purposing to send.

Change-Id: Ie400d5bfd9ab28b290abce2e790889d78726095f
This commit is contained in:
Clay Gerrard 2014-06-25 20:36:33 -07:00
parent 4cdb51418c
commit 3782bf1b56
2 changed files with 69 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
swift/proxy/controllers/obj.py
View File

@ -51,13 +51,13 @@ from swift.common.http import (
is_success, is_client_error, is_server_error, HTTP_CONTINUE, HTTP_CREATED,
HTTP_MULTIPLE_CHOICES, HTTP_NOT_FOUND, HTTP_INTERNAL_SERVER_ERROR,
HTTP_SERVICE_UNAVAILABLE, HTTP_INSUFFICIENT_STORAGE,
HTTP_PRECONDITION_FAILED)
HTTP_PRECONDITION_FAILED, HTTP_CONFLICT)
from swift.proxy.controllers.base import Controller, delay_denial, \
cors_validation
from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \
HTTPServerError, HTTPServiceUnavailable, Request, \
HTTPClientDisconnect
HTTPClientDisconnect, HeaderKeyDict
from swift.common.request_helpers import is_sys_or_user_meta, is_sys_meta, \
remove_items, copy_header_subset
@ -340,7 +340,7 @@ class ObjectController(Controller):
conn.resp = None
conn.node = node
return conn
elif is_success(resp.status):
elif is_success(resp.status) or resp.status == HTTP_CONFLICT:
conn.resp = resp
conn.node = node
return conn
@ -494,10 +494,8 @@ class ObjectController(Controller):
partition, nodes = obj_ring.get_nodes(
self.account_name, self.container_name, self.object_name)
# do a HEAD request for container sync and checking object versions
if 'x-timestamp' in req.headers or \
(object_versions and not
req.environ.get('swift_versioned_copy')):
# do a HEAD request for checking object versions
if object_versions and not req.environ.get('swift_versioned_copy'):
# make sure proxy-server uses the right policy index
_headers = {'X-Backend-Storage-Policy-Index': policy_index,
'X-Newest': 'True'}
@ -511,9 +509,6 @@ class ObjectController(Controller):
if 'x-timestamp' in req.headers:
try:
req_timestamp = Timestamp(req.headers['X-Timestamp'])
if hresp.environ and 'swift_x_timestamp' in hresp.environ and \
hresp.environ['swift_x_timestamp'] >= req_timestamp:
return HTTPAccepted(request=req)
except ValueError:
return HTTPBadRequest(
request=req, content_type='text/plain',
@ -571,8 +566,8 @@ class ObjectController(Controller):
else:
src_account_name = acct
src_container_name, src_obj_name = check_copy_from_header(req)
source_header = '/%s/%s/%s/%s' % (ver, src_account_name,
src_container_name, src_obj_name)
source_header = '/%s/%s/%s/%s' % (
ver, src_account_name, src_container_name, src_obj_name)
source_req = req.copy_get()
# make sure the source request uses it's container_info
@ -674,6 +669,17 @@ class ObjectController(Controller):
{'statuses': statuses})
return HTTPPreconditionFailed(request=req)
if any(conn for conn in conns if conn.resp and
conn.resp.status == HTTP_CONFLICT):
timestamps = [HeaderKeyDict(conn.resp.getheaders()).get(
'X-Backend-Timestamp') for conn in conns if conn.resp]
self.app.logger.debug(
_('Object PUT returning 202 for 409: '
'%(req_timestamp)s <= %(timestamps)r'),
{'req_timestamp': req.timestamp.internal,
'timestamps': ', '.join(timestamps)})
return HTTPAccepted(request=req)
if len(conns) < min_conns:
self.app.logger.error(
_('Object PUT returning 503, %(conns)s/%(nodes)s '

73
test/unit/proxy/controllers/test_obj.py
View File

@ -494,12 +494,8 @@ class TestObjController(unittest.TestCase):
'/v1/a/c/o', method='PUT', headers={
'Content-Length': 0,
'X-Timestamp': put_timestamp})
ts_iter = itertools.repeat(put_timestamp)
head_resp = [404] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
put_resp = [201] * self.obj_ring.replicas
codes = head_resp + put_resp
with set_http_connect(*codes, timestamps=ts_iter):
codes = [201] * self.obj_ring.replicas
with set_http_connect(*codes):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 201)
@ -513,9 +509,7 @@ class TestObjController(unittest.TestCase):
'Content-Length': 0,
'X-Timestamp': put_timestamp})
ts_iter = itertools.repeat(put_timestamp)
head_resp = [200] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
codes = head_resp
codes = [409] * self.obj_ring.replicas
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 202)
@ -530,9 +524,7 @@ class TestObjController(unittest.TestCase):
'Content-Length': 0,
'X-Timestamp': ts.next().internal})
ts_iter = itertools.repeat(ts.next().internal)
head_resp = [200] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
codes = head_resp
codes = [409] * self.obj_ring.replicas
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 202)
@ -547,10 +539,7 @@ class TestObjController(unittest.TestCase):
'Content-Length': 0,
'X-Timestamp': ts.next().internal})
ts_iter = itertools.repeat(orig_timestamp)
head_resp = [200] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
put_resp = [201] * self.obj_ring.replicas
codes = head_resp + put_resp
codes = [201] * self.obj_ring.replicas
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 201)
@ -574,13 +563,53 @@ class TestObjController(unittest.TestCase):
'/v1/a/c/o', method='PUT', headers={
'Content-Length': 0,
'X-Timestamp': ts.next().internal})
head_resp = [404] * self.obj_ring.replicas + \
[404] * self.obj_ring.max_more_nodes
put_resp = [409] + [201] * self.obj_ring.replicas
codes = head_resp + put_resp
with set_http_connect(*codes):
ts_iter = iter([ts.next().internal, None, None])
codes = [409] + [201] * (self.obj_ring.replicas - 1)
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 201)
self.assertEqual(resp.status_int, 202)
def test_container_sync_put_x_timestamp_race(self):
ts = (utils.Timestamp(t) for t in itertools.count(int(time.time())))
test_indexes = [None] + [int(p) for p in POLICIES]
for policy_index in test_indexes:
put_timestamp = ts.next().internal
req = swob.Request.blank(
'/v1/a/c/o', method='PUT', headers={
'Content-Length': 0,
'X-Timestamp': put_timestamp})
# object nodes they respond 409 because another in-flight request
# finished and now the on disk timestamp is equal to the request.
put_ts = [put_timestamp] * self.obj_ring.replicas
codes = [409] * self.obj_ring.replicas
ts_iter = iter(put_ts)
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 202)
def test_container_sync_put_x_timestamp_unsynced_race(self):
ts = (utils.Timestamp(t) for t in itertools.count(int(time.time())))
test_indexes = [None] + [int(p) for p in POLICIES]
for policy_index in test_indexes:
put_timestamp = ts.next().internal
req = swob.Request.blank(
'/v1/a/c/o', method='PUT', headers={
'Content-Length': 0,
'X-Timestamp': put_timestamp})
# only one in-flight request finished
put_ts = [None] * (self.obj_ring.replicas - 1)
put_resp = [201] * (self.obj_ring.replicas - 1)
put_ts += [put_timestamp]
put_resp += [409]
ts_iter = iter(put_ts)
codes = put_resp
with set_http_connect(*codes, timestamps=ts_iter):
resp = req.get_response(self.app)
self.assertEqual(resp.status_int, 202)
def test_COPY_simple(self):
req = swift.common.swob.Request.blank(