From 7bbc73a91c22e53c45d7097dfd320a9990580ce7 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Tue, 6 Apr 2021 12:12:32 +0100 Subject: [PATCH] sharding: constrain fill_gaps to own shard range bounds Restrict the fill_gaps behaviour of ContainerBroker.get_shard_ranges() to only insert a filler range to the upper bound of its own shard range. Change-Id: I16d91a5a3e5c785244497bbc2882b305d2b3ca56 Closes-Bug: #1922386 --- swift/container/backend.py | 13 ++-- test/unit/container/test_backend.py | 31 ++++++++++ test/unit/container/test_server.py | 96 +++++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 4 deletions(-) diff --git a/swift/container/backend.py b/swift/container/backend.py index 8afac530bc..92b39a12ea 100644 --- a/swift/container/backend.py +++ b/swift/container/backend.py @@ -1774,8 +1774,10 @@ class ContainerBroker(DatabaseBroker): names do not match the broker's path are included in the returned list. If True, those rows are not included, otherwise they are included. Default is False. - :param fill_gaps: if True, insert own shard range to fill any gaps in - at the tail of other shard ranges. + :param fill_gaps: if True, insert a modified copy of own shard range to + fill any gap between the end of any found shard ranges and the + upper bound of own shard range. Gaps enclosed within the found + shard ranges are not filled. :return: a list of instances of :class:`swift.common.utils.ShardRange` """ if reverse: @@ -1796,11 +1798,14 @@ class ContainerBroker(DatabaseBroker): marker, end_marker) if not includes and fill_gaps: + own_shard_range = self._own_shard_range() if shard_ranges: last_upper = shard_ranges[-1].upper else: - last_upper = marker or ShardRange.MIN - required_upper = end_marker or ShardRange.MAX + last_upper = max(marker or own_shard_range.lower, + own_shard_range.lower) + required_upper = min(end_marker or own_shard_range.upper, + own_shard_range.upper) if required_upper > last_upper: filler_sr = self.get_own_shard_range() filler_sr.lower = last_upper diff --git a/test/unit/container/test_backend.py b/test/unit/container/test_backend.py index 722e06e6bb..3a6bcc97cc 100644 --- a/test/unit/container/test_backend.py +++ b/test/unit/container/test_backend.py @@ -4062,6 +4062,37 @@ class TestContainerBroker(unittest.TestCase): [dict(sr) for sr in [shard_ranges[2], shard_ranges[4]]], [dict(sr) for sr in actual]) + # fill gaps + filler = own_shard_range.copy() + filler.lower = 'h' + with mock_timestamp_now() as now: + actual = broker.get_shard_ranges(fill_gaps=True) + filler.meta_timestamp = now + self.assertEqual([dict(sr) for sr in undeleted + [filler]], + [dict(sr) for sr in actual]) + with mock_timestamp_now() as now: + actual = broker.get_shard_ranges(fill_gaps=True, marker='a') + filler.meta_timestamp = now + self.assertEqual([dict(sr) for sr in undeleted + [filler]], + [dict(sr) for sr in actual]) + with mock_timestamp_now() as now: + actual = broker.get_shard_ranges(fill_gaps=True, end_marker='z') + filler.meta_timestamp = now + self.assertEqual([dict(sr) for sr in undeleted + [filler]], + [dict(sr) for sr in actual]) + with mock_timestamp_now() as now: + actual = broker.get_shard_ranges(fill_gaps=True, end_marker='k') + filler.meta_timestamp = now + filler.upper = 'k' + self.assertEqual([dict(sr) for sr in undeleted + [filler]], + [dict(sr) for sr in actual]) + # no filler needed... + actual = broker.get_shard_ranges(fill_gaps=True, end_marker='h') + self.assertEqual([dict(sr) for sr in undeleted], + [dict(sr) for sr in actual]) + actual = broker.get_shard_ranges(fill_gaps=True, end_marker='a') + self.assertEqual([], [dict(sr) for sr in actual]) + # get everything actual = broker.get_shard_ranges(include_own=True) self.assertEqual([dict(sr) for sr in undeleted + [own_shard_range]], diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index 28bbd9862d..d2d86ac432 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -2972,6 +2972,102 @@ class TestContainerController(unittest.TestCase): self.assertFalse(self.controller.logger.get_lines_for_level('warning')) self.assertFalse(self.controller.logger.get_lines_for_level('error')) + def test_GET_shard_ranges_from_compacted_shard(self): + # make a shrunk shard container with two acceptors that overlap with + # the shard's namespace + shard_path = '.shards_a/c_f' + ts_iter = make_timestamp_iter() + ts_now = Timestamp.now() # used when mocking Timestamp.now() + own_shard_range = ShardRange(shard_path, next(ts_iter), + 'b', 'f', 100, 1000, + meta_timestamp=next(ts_iter), + state=ShardRange.SHRUNK, + state_timestamp=next(ts_iter), + epoch=next(ts_iter)) + shard_ranges = [] + for lower, upper in (('a', 'd'), ('d', 'g')): + shard_ranges.append( + ShardRange('.shards_a/c_%s' % upper, next(ts_iter), + lower, upper, 100, 1000, + meta_timestamp=next(ts_iter), + state=ShardRange.ACTIVE, + state_timestamp=next(ts_iter))) + + # create container + headers = {'X-Timestamp': next(ts_iter).normal} + req = Request.blank( + '/sda1/p/%s' % shard_path, method='PUT', headers=headers) + self.assertIn( + req.get_response(self.controller).status_int, (201, 202)) + + # PUT the acceptor shard ranges and own shard range + headers = {'X-Timestamp': next(ts_iter).normal, + 'X-Container-Sysmeta-Shard-Root': 'a/c', + 'X-Backend-Record-Type': 'shard'} + body = json.dumps( + [dict(sr) for sr in shard_ranges + [own_shard_range]]) + req = Request.blank('/sda1/p/%s' % shard_path, method='PUT', + headers=headers, body=body) + self.assertEqual(202, req.get_response(self.controller).status_int) + + def do_get(params, extra_headers, expected): + expected = [dict(sr, last_modified=sr.timestamp.isoformat) + for sr in expected] + headers = {'X-Backend-Record-Type': 'shard'} + headers.update(extra_headers) + req = Request.blank('/sda1/p/%s?format=json%s' % + (shard_path, params), method='GET', + headers=headers) + with mock_timestamp_now(ts_now): + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, 200) + self.assertEqual(resp.content_type, 'application/json') + self.assertEqual(expected, json.loads(resp.body)) + self.assertIn('X-Backend-Record-Type', resp.headers) + self.assertEqual('shard', resp.headers['X-Backend-Record-Type']) + return resp + + # unsharded shard container... + do_get('', {}, shard_ranges) + do_get('&marker=e', {}, shard_ranges[1:]) + do_get('&end_marker=d', {}, shard_ranges[:1]) + do_get('&end_marker=k', {}, shard_ranges) + do_get('&marker=b&end_marker=f&states=listing', {}, shard_ranges) + do_get('&marker=b&end_marker=c&states=listing', {}, shard_ranges[:1]) + do_get('&marker=b&end_marker=z&states=listing', {}, shard_ranges) + do_get('&states=listing', {}, shard_ranges) + + # send X-Backend-Override-Shard-Name-Filter, but db is not yet sharded + # so this has no effect + extra_headers = {'X-Backend-Override-Shard-Name-Filter': 'sharded'} + resp = do_get('', extra_headers, shard_ranges) + self.assertNotIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + resp = do_get('&marker=e', extra_headers, shard_ranges[1:]) + self.assertNotIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + resp = do_get('&end_marker=d', extra_headers, shard_ranges[:1]) + self.assertNotIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + resp = do_get('&states=listing', {}, shard_ranges) + self.assertNotIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + + # set broker to sharded state so X-Backend-Override-Shard-Name-Filter + # does have effect + shard_broker = self.controller._get_container_broker( + 'sda1', 'p', '.shards_a', 'c_f') + self.assertTrue(shard_broker.set_sharding_state()) + self.assertTrue(shard_broker.set_sharded_state()) + + resp = do_get('', extra_headers, shard_ranges) + self.assertIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + self.assertTrue(resp.headers['X-Backend-Override-Shard-Name-Filter']) + + resp = do_get('&marker=e', extra_headers, shard_ranges) + self.assertIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + self.assertTrue(resp.headers['X-Backend-Override-Shard-Name-Filter']) + + resp = do_get('&end_marker=d', extra_headers, shard_ranges) + self.assertIn('X-Backend-Override-Shard-Name-Filter', resp.headers) + self.assertTrue(resp.headers['X-Backend-Override-Shard-Name-Filter']) + def test_GET_shard_ranges_using_state_aliases(self): # make a shard container ts_iter = make_timestamp_iter()