openstack
/
swift
Code Issues Proposed changes

Add "If-None-Match: *" support to PUT 

A common pattern that we see clients do is send a HEAD request before a
PUT to see if it exists.  This can slow things down quite a bit
especially since 404s on HEAD are currently a bit expensive.

This change will allow a client to include a "If-None-Match: *" header
with a PUT request.  In combination with "Expect: 100-Continue" this
allows the server to return that it already has a copy of the object
before any data is sent.

I attempted to also include etag support with the If-None-Match header,
but that turned up having too many hairy edge cases, so was left as a
future excercise.

DocImpact

Change-Id: I94e3754923dbe5faba065719c7a9afa9969652dd
This commit is contained in:
Chuck Thier 2014-03-19 21:51:17 +00:00 committed by John Dickinson
parent a5cd0935e3
commit 0b893825eb
6 changed files with 191 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
swift/obj/server.py
View File

@ -388,6 +388,16 @@ class ObjectController(object):
orig_metadata = disk_file.read_metadata()
except (DiskFileNotExist, DiskFileQuarantined):
orig_metadata = {}
# Checks for If-None-Match
if request.if_none_match is not None and orig_metadata:
if '*' in request.if_none_match:
# File exists already so return 412
return HTTPPreconditionFailed(request=request)
if orig_metadata.get('ETag') in request.if_none_match:
# The current ETag matches, so return 412
return HTTPPreconditionFailed(request=request)
orig_timestamp = orig_metadata.get('X-Timestamp')
if orig_timestamp and orig_timestamp >= request.headers['x-timestamp']:
return HTTPConflict(request=request)

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

@ -47,7 +47,7 @@ from swift.common.exceptions import ChunkReadTimeout, \
from swift.common.http import is_success, is_client_error, HTTP_CONTINUE, \
HTTP_CREATED, HTTP_MULTIPLE_CHOICES, HTTP_NOT_FOUND, \
HTTP_INTERNAL_SERVER_ERROR, HTTP_SERVICE_UNAVAILABLE, \
HTTP_INSUFFICIENT_STORAGE
HTTP_INSUFFICIENT_STORAGE, HTTP_PRECONDITION_FAILED
from swift.proxy.controllers.base import Controller, delay_denial, \
cors_validation
from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
@ -382,6 +382,11 @@ class ObjectController(Controller):
conn.resp = resp
conn.node = node
return conn
elif headers['If-None-Match'] is not None and \
resp.status == HTTP_PRECONDITION_FAILED:
conn.resp = resp
conn.node = node
return conn
elif resp.status == HTTP_INSUFFICIENT_STORAGE:
self.app.error_limit(node, _('ERROR Insufficient Storage'))
except (Exception, Timeout):
@ -438,6 +443,10 @@ class ObjectController(Controller):
@delay_denial
def PUT(self, req):
"""HTTP PUT request handler."""
if req.if_none_match is not None and '*' not in req.if_none_match:
# Sending an etag with if-none-match isn't currently supported
return HTTPBadRequest(request=req, content_type='text/plain',
body='If-None-Match only supports *')
container_info = self.container_info(
self.account_name, self.container_name, req)
container_partition = container_info['partition']
@ -653,6 +662,16 @@ class ObjectController(Controller):
conns = [conn for conn in pile if conn]
min_conns = quorum_size(len(nodes))
if req.if_none_match is not None and '*' in req.if_none_match:
statuses = [conn.resp.status for conn in conns if conn.resp]
if HTTP_PRECONDITION_FAILED in statuses:
# If we find any copy of the file, it shouldn't be uploaded
self.app.logger.debug(
_('Object PUT returning 412, %(statuses)r'),
{'statuses': statuses})
return HTTPPreconditionFailed(request=req)
if len(conns) < min_conns:
self.app.logger.error(
_('Object PUT returning 503, %(conns)s/%(nodes)s '

28
test/functional/test_object.py
View File

@ -85,6 +85,32 @@ class TestObject(unittest.TestCase):
resp.read()
self.assertEquals(resp.status, 204)
def test_if_none_match(self):
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s/%s' % (
parsed.path, self.container, 'if_none_match_test'), '',
{'X-Auth-Token': token,
'Content-Length': '0',
'If-None-Match': '*'})
return check_response(conn)
resp = retry(put)
resp.read()
self.assertEquals(resp.status, 201)
resp = retry(put)
resp.read()
self.assertEquals(resp.status, 412)
def put(url, token, parsed, conn):
conn.request('PUT', '%s/%s/%s' % (
parsed.path, self.container, 'if_none_match_test'), '',
{'X-Auth-Token': token,
'Content-Length': '0',
'If-None-Match': 'somethingelse'})
return check_response(conn)
resp = retry(put)
resp.read()
self.assertEquals(resp.status, 400)
def test_copy_object(self):
if skip:
raise SkipTest
@ -880,7 +906,7 @@ class TestObject(unittest.TestCase):
conn.request(
'PUT', '%s/%s' % (parsed.path, self.container),
'', {'X-Auth-Token': token,
'X-Container-Meta-Access-Control-Allow-Origin': orig})
'X-Container-Meta-Access-Control-Allow-Origin': orig})
return check_response(conn)
def put_obj(url, token, parsed, conn, obj):

2
test/unit/__init__.py
View File

@ -471,6 +471,8 @@ def fake_http_connect(*code_iter, **kwargs):
return FakeConn(507)
if self.expect_status == -4:
return FakeConn(201)
if self.expect_status == 412:
return FakeConn(412)
return FakeConn(100)
def getheaders(self):

48
test/unit/obj/test_server.py
View File

@ -485,6 +485,54 @@ class TestObjectController(unittest.TestCase):
resp = req.get_response(self.object_controller)
self.assertEqual(resp.status_int, 400)
def test_PUT_if_none_match_star(self):
# First PUT should succeed
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': timestamp,
'Content-Length': '6',
'Content-Type': 'application/octet-stream',
'If-None-Match': '*'})
req.body = 'VERIFY'
resp = req.get_response(self.object_controller)
self.assertEquals(resp.status_int, 201)
# File should already exist so it should fail
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': timestamp,
'Content-Length': '6',
'Content-Type': 'application/octet-stream',
'If-None-Match': '*'})
req.body = 'VERIFY'
resp = req.get_response(self.object_controller)
self.assertEquals(resp.status_int, 412)
def test_PUT_if_none_match(self):
# PUT with if-none-match set and nothing there should succede
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': timestamp,
'Content-Length': '6',
'Content-Type': 'application/octet-stream',
'If-None-Match': 'notthere'})
req.body = 'VERIFY'
resp = req.get_response(self.object_controller)
self.assertEquals(resp.status_int, 201)
# PUT with if-none-match of the object etag should fail
timestamp = normalize_timestamp(time())
req = Request.blank(
'/sda1/p/a/c/o', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Timestamp': timestamp,
'Content-Length': '6',
'Content-Type': 'application/octet-stream',
'If-None-Match': '0b4c12d7e0a73840c1c4f148fda3b037'})
req.body = 'VERIFY'
resp = req.get_response(self.object_controller)
self.assertEquals(resp.status_int, 412)
def test_PUT_common(self):
timestamp = normalize_timestamp(time())
req = Request.blank(

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

@ -22,7 +22,7 @@ import mock
import swift
from swift.proxy import server as proxy_server
from swift.common.swob import HTTPException
from test.unit import FakeRing, FakeMemcache, fake_http_connect
from test.unit import FakeRing, FakeMemcache, fake_http_connect, debug_logger
@contextmanager
@ -90,26 +90,96 @@ class TestObjControllerWriteAffinity(unittest.TestCase):
class TestObjController(unittest.TestCase):
def setUp(self):
logger = debug_logger('proxy-server')
logger.thread_locals = ('txn1', '127.0.0.2')
self.app = proxy_server.Application(
None, FakeMemcache(), account_ring=FakeRing(),
container_ring=FakeRing(), object_ring=FakeRing(),
logger=logger)
self.controller = proxy_server.ObjectController(self.app,
'a', 'c', 'o')
self.controller.container_info = mock.MagicMock(return_value={
'partition': 1,
'nodes': [
{'ip': '127.0.0.1', 'port': '1', 'device': 'sda'},
{'ip': '127.0.0.1', 'port': '2', 'device': 'sda'},
{'ip': '127.0.0.1', 'port': '3', 'device': 'sda'},
],
'write_acl': None,
'read_acl': None,
'sync_key': None,
'versions': None})
def test_PUT_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
req.headers['content-length'] = '0'
with set_http_connect(201, 201, 201):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
def test_PUT_if_none_match(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
req.headers['if-none-match'] = '*'
req.headers['content-length'] = '0'
with set_http_connect(201, 201, 201):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 201)
def test_PUT_if_none_match_denied(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
req.headers['if-none-match'] = '*'
req.headers['content-length'] = '0'
with set_http_connect(201, (412, 412), 201):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 412)
def test_PUT_if_none_match_not_star(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
req.headers['if-none-match'] = 'somethingelse'
req.headers['content-length'] = '0'
with set_http_connect(201, 201, 201):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 400)
def test_GET_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
with set_http_connect(200):
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 200)
def test_DELETE_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
with set_http_connect(204, 204, 204):
resp = self.controller.DELETE(req)
self.assertEquals(resp.status_int, 204)
def test_POST_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
with set_http_connect(200, 200, 200, 201, 201, 201):
resp = self.controller.POST(req)
self.assertEquals(resp.status_int, 202)
def test_COPY_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
with set_http_connect(200, 200, 200, 201, 201, 201):
resp = self.controller.POST(req)
self.assertEquals(resp.status_int, 202)
def test_HEAD_simple(self):
req = swift.common.swob.Request.blank('/v1/a/c/o')
with set_http_connect(200, 200, 200, 201, 201, 201):
resp = self.controller.POST(req)
self.assertEquals(resp.status_int, 202)
def test_PUT_log_info(self):
# mock out enough to get to the area of the code we want to test
with mock.patch('swift.proxy.controllers.obj.check_object_creation',
mock.MagicMock(return_value=None)):
app = mock.MagicMock()
app.container_ring.get_nodes.return_value = (1, [2])
app.object_ring.get_nodes.return_value = (1, [2])
controller = proxy_server.ObjectController(app, 'a', 'c', 'o')
controller.container_info = mock.MagicMock(return_value={
'partition': 1,
'nodes': [{}],
'write_acl': None,
'sync_key': None,
'versions': None})
# and now test that we add the header to log_info
req = swift.common.swob.Request.blank('/v1/a/c/o')
req.headers['x-copy-from'] = 'somewhere'
try:
controller.PUT(req)
self.controller.PUT(req)
except HTTPException:
pass
self.assertEquals(
@ -119,7 +189,7 @@ class TestObjController(unittest.TestCase):
req.method = 'POST'
req.headers['x-copy-from'] = 'elsewhere'
try:
controller.PUT(req)
self.controller.PUT(req)
except HTTPException:
pass
self.assertEquals(req.environ.get('swift.log_info'), None)