From ce4c0fb14b6c92cab4b80e4143171df75c5f47b1 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 28 May 2020 22:20:21 -0700 Subject: [PATCH] Don't auto-create shard containers ...unless the client requests it specifically using a new flag: X-Backend-Auto-Create: true Previously, you could get real jittery listings during a rebalance: * Partition with a shard DB get reassigned, so one primary has no DB. * Proxy makes a listing, gets a 404, tries another node. Likely, one of the other shard replicas responds. Things are fine. * Update comes in. Since we use the auto_create_account_prefix namespace for shards, container DB gets created and we write the row. * Proxy makes another listing. There's a one-in-three chance that we claim there's only one object in that whole range. Note that unsharded databases would respond to the update with a 404 and wait for one of the other primaries (or the old primary that's now a hand-off) to rsync a whole DB over, keeping us in the happy state. Now, if the account is in the shards namespace, 404 the object update if we have no DB. Wait for replication like in the unsharded case. Continue to be willing to create the DB when the sharder is seeding all the CREATED databases before it starts cleaving, though. Change-Id: I15052f3f17999e6f432951ba7c0731dcdc9475bb Closes-Bug: #1881210 --- swift/container/server.py | 44 ++++++++---- swift/container/sharder.py | 3 +- test/unit/container/test_server.py | 108 ++++++++++++++--------------- 3 files changed, 82 insertions(+), 73 deletions(-) diff --git a/swift/container/server.py b/swift/container/server.py index c8d7647aa6..db9ac02910 100644 --- a/swift/container/server.py +++ b/swift/container/server.py @@ -155,6 +155,8 @@ class ContainerController(BaseStorageServer): conf['auto_create_account_prefix'] else: self.auto_create_account_prefix = AUTO_CREATE_ACCOUNT_PREFIX + self.shards_account_prefix = ( + self.auto_create_account_prefix + 'shards_') if config_true_value(conf.get('allow_versions', 'f')): self.save_headers.append('x-versions-location') if 'allow_versions' in conf: @@ -375,14 +377,12 @@ class ContainerController(BaseStorageServer): # auto create accounts) obj_policy_index = self.get_and_validate_policy_index(req) or 0 broker = self._get_container_broker(drive, part, account, container) - if account.startswith(self.auto_create_account_prefix) and obj and \ - not os.path.exists(broker.db_file): - try: - broker.initialize(req_timestamp.internal, obj_policy_index) - except DatabaseAlreadyExists: - pass - if not os.path.exists(broker.db_file): + if obj: + self._maybe_autocreate(broker, req_timestamp, account, + obj_policy_index, req) + elif not os.path.exists(broker.db_file): return HTTPNotFound() + if obj: # delete object # redirect if a shard range exists for the object name redirect = self._redirect_to_shard(req, broker, obj) @@ -449,11 +449,25 @@ class ContainerController(BaseStorageServer): broker.update_status_changed_at(timestamp) return recreated + def _should_autocreate(self, account, req): + auto_create_header = req.headers.get('X-Backend-Auto-Create') + if auto_create_header: + # If the caller included an explicit X-Backend-Auto-Create header, + # assume they know the behavior they want + return config_true_value(auto_create_header) + if account.startswith(self.shards_account_prefix): + # we have to specical case this subset of the + # auto_create_account_prefix because we don't want the updater + # accidently auto-creating shards; only the sharder creates + # shards and it will explicitly tell the server to do so + return False + return account.startswith(self.auto_create_account_prefix) + def _maybe_autocreate(self, broker, req_timestamp, account, - policy_index): + policy_index, req): created = False - if account.startswith(self.auto_create_account_prefix) and \ - not os.path.exists(broker.db_file): + should_autocreate = self._should_autocreate(account, req) + if should_autocreate and not os.path.exists(broker.db_file): if policy_index is None: raise HTTPBadRequest( 'X-Backend-Storage-Policy-Index header is required') @@ -506,8 +520,8 @@ class ContainerController(BaseStorageServer): # obj put expects the policy_index header, default is for # legacy support during upgrade. obj_policy_index = requested_policy_index or 0 - self._maybe_autocreate(broker, req_timestamp, account, - obj_policy_index) + self._maybe_autocreate( + broker, req_timestamp, account, obj_policy_index, req) # redirect if a shard exists for this object name response = self._redirect_to_shard(req, broker, obj) if response: @@ -531,8 +545,8 @@ class ContainerController(BaseStorageServer): for sr in json.loads(req.body)] except (ValueError, KeyError, TypeError) as err: return HTTPBadRequest('Invalid body: %r' % err) - created = self._maybe_autocreate(broker, req_timestamp, account, - requested_policy_index) + created = self._maybe_autocreate( + broker, req_timestamp, account, requested_policy_index, req) self._update_metadata(req, broker, req_timestamp, 'PUT') if shard_ranges: # TODO: consider writing the shard ranges into the pending @@ -805,7 +819,7 @@ class ContainerController(BaseStorageServer): requested_policy_index = self.get_and_validate_policy_index(req) broker = self._get_container_broker(drive, part, account, container) self._maybe_autocreate(broker, req_timestamp, account, - requested_policy_index) + requested_policy_index, req) try: objs = json.load(req.environ['wsgi.input']) except ValueError as err: diff --git a/swift/container/sharder.py b/swift/container/sharder.py index d9aa7c66de..d0652143e8 100644 --- a/swift/container/sharder.py +++ b/swift/container/sharder.py @@ -1148,7 +1148,8 @@ class ContainerSharder(ContainerReplicator): 'X-Backend-Storage-Policy-Index': broker.storage_policy_index, 'X-Container-Sysmeta-Shard-Quoted-Root': quote( broker.root_path), - 'X-Container-Sysmeta-Sharding': True} + 'X-Container-Sysmeta-Sharding': 'True', + 'X-Backend-Auto-Create': 'True'} # NB: we *used* to send along # 'X-Container-Sysmeta-Shard-Root': broker.root_path # but that isn't safe for container names with nulls or newlines diff --git a/test/unit/container/test_server.py b/test/unit/container/test_server.py index fc55ff05d8..4fd1fcf2e3 100644 --- a/test/unit/container/test_server.py +++ b/test/unit/container/test_server.py @@ -2380,15 +2380,17 @@ class TestContainerController(unittest.TestCase): 'X-Container-Sysmeta-Test': 'set', 'X-Container-Meta-Test': 'persisted'} - # PUT shard range to non-existent container with non-autocreate prefix - req = Request.blank('/sda1/p/a/c', method='PUT', headers=headers, - body=json.dumps([dict(shard_range)])) + # PUT shard range to non-existent container without autocreate flag + req = Request.blank( + '/sda1/p/.shards_a/shard_c', method='PUT', headers=headers, + body=json.dumps([dict(shard_range)])) resp = req.get_response(self.controller) self.assertEqual(404, resp.status_int) - # PUT shard range to non-existent container with autocreate prefix, + # PUT shard range to non-existent container with autocreate flag, # missing storage policy headers['X-Timestamp'] = next(ts_iter).internal + headers['X-Backend-Auto-Create'] = 't' req = Request.blank( '/sda1/p/.shards_a/shard_c', method='PUT', headers=headers, body=json.dumps([dict(shard_range)])) @@ -2397,7 +2399,7 @@ class TestContainerController(unittest.TestCase): self.assertIn(b'X-Backend-Storage-Policy-Index header is required', resp.body) - # PUT shard range to non-existent container with autocreate prefix + # PUT shard range to non-existent container with autocreate flag headers['X-Timestamp'] = next(ts_iter).internal policy_index = random.choice(POLICIES).idx headers['X-Backend-Storage-Policy-Index'] = str(policy_index) @@ -2407,7 +2409,7 @@ class TestContainerController(unittest.TestCase): resp = req.get_response(self.controller) self.assertEqual(201, resp.status_int) - # repeat PUT of shard range to autocreated container - 204 response + # repeat PUT of shard range to autocreated container - 202 response headers['X-Timestamp'] = next(ts_iter).internal headers.pop('X-Backend-Storage-Policy-Index') # no longer required req = Request.blank( @@ -2416,7 +2418,7 @@ class TestContainerController(unittest.TestCase): resp = req.get_response(self.controller) self.assertEqual(202, resp.status_int) - # regular PUT to autocreated container - 204 response + # regular PUT to autocreated container - 202 response headers['X-Timestamp'] = next(ts_iter).internal req = Request.blank( '/sda1/p/.shards_a/shard_c', method='PUT', @@ -4649,61 +4651,53 @@ class TestContainerController(unittest.TestCase): "%d on param %s" % (resp.status_int, param)) def test_put_auto_create(self): - headers = {'x-timestamp': Timestamp(1).internal, - 'x-size': '0', - 'x-content-type': 'text/plain', - 'x-etag': 'd41d8cd98f00b204e9800998ecf8427e'} + def do_test(expected_status, path, extra_headers=None, body=None): + headers = {'x-timestamp': Timestamp(1).internal, + 'x-size': '0', + 'x-content-type': 'text/plain', + 'x-etag': 'd41d8cd98f00b204e9800998ecf8427e'} + if extra_headers: + headers.update(extra_headers) + req = Request.blank('/sda1/p/' + path, + environ={'REQUEST_METHOD': 'PUT'}, + headers=headers, body=body) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, expected_status) - req = Request.blank('/sda1/p/a/c/o', - environ={'REQUEST_METHOD': 'PUT'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) + do_test(404, 'a/c/o') + do_test(404, '.a/c/o', {'X-Backend-Auto-Create': 'no'}) + do_test(201, '.a/c/o') + do_test(404, 'a/.c/o') + do_test(404, 'a/c/.o') + do_test(201, 'a/c/o', {'X-Backend-Auto-Create': 'yes'}) - req = Request.blank('/sda1/p/.a/c/o', - environ={'REQUEST_METHOD': 'PUT'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 201) - - req = Request.blank('/sda1/p/a/.c/o', - environ={'REQUEST_METHOD': 'PUT'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) - - req = Request.blank('/sda1/p/a/c/.o', - environ={'REQUEST_METHOD': 'PUT'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) + do_test(404, '.shards_a/c/o') + create_shard_headers = { + 'X-Backend-Record-Type': 'shard', + 'X-Backend-Storage-Policy-Index': '0'} + do_test(404, '.shards_a/c', create_shard_headers, '[]') + create_shard_headers['X-Backend-Auto-Create'] = 't' + do_test(201, '.shards_a/c', create_shard_headers, '[]') def test_delete_auto_create(self): - headers = {'x-timestamp': Timestamp(1).internal} + def do_test(expected_status, path, extra_headers=None): + headers = {'x-timestamp': Timestamp(1).internal} + if extra_headers: + headers.update(extra_headers) + req = Request.blank('/sda1/p/' + path, + environ={'REQUEST_METHOD': 'DELETE'}, + headers=headers) + resp = req.get_response(self.controller) + self.assertEqual(resp.status_int, expected_status) - req = Request.blank('/sda1/p/a/c/o', - environ={'REQUEST_METHOD': 'DELETE'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) - - req = Request.blank('/sda1/p/.a/c/o', - environ={'REQUEST_METHOD': 'DELETE'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 204) - - req = Request.blank('/sda1/p/a/.c/o', - environ={'REQUEST_METHOD': 'DELETE'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) - - req = Request.blank('/sda1/p/a/.c/.o', - environ={'REQUEST_METHOD': 'DELETE'}, - headers=dict(headers)) - resp = req.get_response(self.controller) - self.assertEqual(resp.status_int, 404) + do_test(404, 'a/c/o') + do_test(404, '.a/c/o', {'X-Backend-Auto-Create': 'false'}) + do_test(204, '.a/c/o') + do_test(404, 'a/.c/o') + do_test(404, 'a/.c/.o') + do_test(404, '.shards_a/c/o') + do_test(204, 'a/c/o', {'X-Backend-Auto-Create': 'true'}) + do_test(204, '.shards_a/c/o', {'X-Backend-Auto-Create': 'true'}) def test_content_type_on_HEAD(self): Request.blank('/sda1/p/a/o',