Skip to content

Commit

Permalink
py3: Fix status comparison in ECGetResponseCollection
Browse files Browse the repository at this point in the history
The initial dummy bad bucket was previously stored using None as a
key. In some circumstances this None key might be compared with
integer response status, which is unsupported in py3.

Move away from having any sort of default value and instead return a
default dummy bucket if we don't have any responses or only have 404
responses.

Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
Change-Id: I6a2ec3739ac92affb0f3dac248f26a1fc4ac9493
Closes-Bug: #1900770
(cherry picked from commit a5fa3cf)
  • Loading branch information
tipabu committed Jan 15, 2021
1 parent 8e744ba commit acb742a
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
6 changes: 5 additions & 1 deletion swift/proxy/controllers/obj.py
Expand Up @@ -2118,7 +2118,8 @@ def __init__(self, policy):
"""
self.policy = policy
self.buckets = {}
self.bad_buckets = {None: ECGetResponseBucket(self.policy, None)}
self.default_bad_bucket = ECGetResponseBucket(self.policy, None)
self.bad_buckets = {}
self.node_iter_count = 0

def _get_bucket(self, timestamp):
Expand Down Expand Up @@ -2247,6 +2248,9 @@ def least_bad_bucket(self):
"""
Return the bad_bucket with the smallest shortfall
"""
if all(status == 404 for status in self.bad_buckets):
# NB: also covers an empty self.bad_buckets
return self.default_bad_bucket
# we want "enough" 416s to prevent "extra" requests - but we keep
# digging on 404s
short, status = min((bucket.shortfall, status)
Expand Down
27 changes: 27 additions & 0 deletions test/unit/proxy/controllers/test_obj.py
Expand Up @@ -3949,6 +3949,33 @@ def get_response(req):
self.assertEqual(resp.status_int, 416)
self.assertEqual(len(log), 2 * self.replicas())

@patch_policies(
[ECStoragePolicy(0, name='ec', is_default=True,
ec_type=DEFAULT_TEST_EC_TYPE, ec_ndata=4,
ec_nparity=4, ec_segment_size=4096)],
fake_ring_args=[{'replicas': 8}]
)
def test_GET_ndata_equals_nparity_with_missing_and_errors(self):
# when ec_ndata == ec_nparity it is possible for the shortfall of a bad
# bucket (412's) to equal ec_ndata; verify that the 412 bucket is still
# chosen ahead of the initial 'dummy' bad bucket
POLICIES.default.object_ring.max_more_nodes = 8
responses = [
StubResponse(412, frag_index=0),
StubResponse(412, frag_index=1),
]

def get_response(req):
return responses.pop(0) if responses else StubResponse(404)

req = swob.Request.blank('/v1/a/c/o', headers={
'Range': 'bytes=%s-' % 100000000000000})
with capture_http_requests(get_response) as log:
resp = req.get_response(self.app)

self.assertEqual(resp.status_int, 412)
self.assertEqual(len(log), 2 * 8)

def test_GET_with_success_and_507_will_503(self):
responses = [ # only 9 good nodes
StubResponse(200),
Expand Down

0 comments on commit acb742a

Please sign in to comment.