From 563e1671cf0583767d652a72ebc4d73d7cc07435 Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Tue, 25 Jun 2019 11:53:32 -0500 Subject: [PATCH] Return 503 when primary containers can't respond Closes-Bug: #1833612 Change-Id: I53ed04b5de20c261ddd79c98c629580472e09961 --- swift/proxy/controllers/base.py | 14 +++++++--- test/unit/__init__.py | 15 ++++++++--- test/unit/proxy/controllers/test_container.py | 5 ++-- test/unit/proxy/test_server.py | 27 +++++++++++++++++++ 4 files changed, 52 insertions(+), 9 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index e3e0ad4e23..3ddc8393ec 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -1233,6 +1233,10 @@ class ResumingGetter(object): _('Trying to %(method)s %(path)s') % {'method': self.req_method, 'path': self.req_path}) return False + + src_headers = dict( + (k.lower(), v) for k, v in + possible_source.getheaders()) if self.is_good_source(possible_source): # 404 if we know we don't have a synced copy if not float(possible_source.getheader('X-PUT-Timestamp', 1)): @@ -1242,9 +1246,6 @@ class ResumingGetter(object): self.source_headers.append([]) close_swift_conn(possible_source) else: - src_headers = dict( - (k.lower(), v) for k, v in - possible_source.getheaders()) if self.used_source_etag and \ self.used_source_etag != src_headers.get( 'x-object-sysmeta-ec-etag', @@ -1271,7 +1272,12 @@ class ResumingGetter(object): if not self.newest: # one good source is enough return True else: - + if self.server_type != 'Object' and 'handoff_index' in node and \ + possible_source.status == HTTP_NOT_FOUND and \ + not Timestamp(src_headers.get('x-backend-timestamp', 0)): + # throw out 404s from handoff nodes unless the db is really + # on disk and had been DELETEd + return False self.statuses.append(possible_source.status) self.reasons.append(possible_source.reason) self.bodies.append(possible_source.read()) diff --git a/test/unit/__init__.py b/test/unit/__init__.py index 9d4133dce8..7c163ae544 100644 --- a/test/unit/__init__.py +++ b/test/unit/__init__.py @@ -878,7 +878,7 @@ def fake_http_connect(*code_iter, **kwargs): SLOW_READS = 4 SLOW_WRITES = 4 - def __init__(self, status, etag=None, body=b'', timestamp='1', + def __init__(self, status, etag=None, body=b'', timestamp=-1, headers=None, expect_headers=None, connection_id=None, give_send=None, give_expect=None): if not isinstance(status, FakeStatus): @@ -893,7 +893,15 @@ def fake_http_connect(*code_iter, **kwargs): self.body = body self.headers = headers or {} self.expect_headers = expect_headers or {} - self.timestamp = timestamp + if timestamp == -1: + # -1 is reserved to mean "magic default" + if status.status != 404: + self.timestamp = '1' + else: + self.timestamp = '0' + else: + # tests may specify int, string, Timestamp or None + self.timestamp = timestamp self.connection_id = connection_id self.give_send = give_send self.give_expect = give_expect @@ -1010,7 +1018,8 @@ def fake_http_connect(*code_iter, **kwargs): def close(self): self.closed = True - timestamps_iter = iter(kwargs.get('timestamps') or ['1'] * len(code_iter)) + # unless tests provide timestamps we use the "magic default" + timestamps_iter = iter(kwargs.get('timestamps') or [-1] * len(code_iter)) etag_iter = iter(kwargs.get('etags') or [None] * len(code_iter)) if isinstance(kwargs.get('headers'), (list, tuple)): headers_iter = iter(kwargs['headers']) diff --git a/test/unit/proxy/controllers/test_container.py b/test/unit/proxy/controllers/test_container.py index c34c19700b..fa6617eed9 100644 --- a/test/unit/proxy/controllers/test_container.py +++ b/test/unit/proxy/controllers/test_container.py @@ -344,9 +344,10 @@ class TestContainerController(TestRingBase): ([200], 200), ([404, 200], 200), ([404] * nodes + [200], 200), - ([Timeout()] * nodes + [404] * handoffs, 404), + ([Timeout()] * nodes + [404] * handoffs, 503), ([Timeout()] * (nodes + handoffs), 503), - ([Timeout()] * (nodes + handoffs - 1) + [404], 404), + ([Timeout()] * (nodes + handoffs - 1) + [404], 503), + ([Timeout()] * (nodes - 1) + [404] * (handoffs + 1), 404), ([503, 200], 200), ([507, 200], 200), ] diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 5c5fa54abe..f892fa15f1 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -4733,6 +4733,33 @@ class TestReplicatedObjectController( # object is not created! self.assertEqual(resp.status_int, 503) + def test_PUT_object_to_primary_containers_timeout(self): + self.app.container_ring.max_more_nodes = 2 # 2 handoffs + + req = Request.blank('/v1/a/c/o', method='PUT', content_length=0) + account_status = [200] + # primary timeout db lock & handoffs 404 + container_status = [Timeout()] * 3 + [404] * 2 + status = account_status + container_status + with mocked_http_conn(*status) as fake_conn: + resp = req.get_response(self.app) + + account_requests = fake_conn.requests[:len(account_status)] + self.assertEqual(['HEAD'], + [r['method'] for r in account_requests]) + self.assertEqual(['/a'], [ + r['path'][len('/sdX/0'):] for r in account_requests]) + + container_requests = fake_conn.requests[ + len(account_status):len(account_status) + len(container_status)] + self.assertEqual(['HEAD'] * 5, + [r['method'] for r in container_requests]) + self.assertEqual(['/a/c'] * 5, [ + r['path'][len('/sdX/0'):] for r in container_requests]) + + # object is not created! + self.assertEqual(resp.status_int, 503) + def test_bad_metadata(self): with save_globals(): controller = ReplicatedObjectController(