openstack/swift
Code Issues Proposed changes

proxy: Get rid of iter_nodes helper

Browse Source 
All it did was proxy through to NodeIter, anyway.

Change-Id: Ifec8d3a40f00141a73f6e50efe0b53b382ab2ef3
This commit is contained in:
Tim Burke 2023-05-10 12:59:39 -07:00
parent cc59929412
commit e5d730dc56
8 changed files with 74 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
swift/proxy/controllers/account.py
View File

@ -21,7 +21,7 @@ from swift.common.utils import public
from swift.common.constraints import check_metadata
from swift.common.http import HTTP_NOT_FOUND, HTTP_GONE
from swift.proxy.controllers.base import Controller, clear_info_cache, \
set_info_cache
set_info_cache, NodeIter
from swift.common.middleware import listing_formats
from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed
from swift.common.request_helpers import get_sys_meta_prefix
@ -63,7 +63,7 @@ class AccountController(Controller):
partition = self.app.account_ring.get_part(self.account_name)
concurrency = self.app.account_ring.replica_count \
if self.app.get_policy_options(None).concurrent_gets else 1
node_iter = self.app.iter_nodes(self.app.account_ring, partition,
node_iter = NodeIter(self.app, self.app.account_ring, partition,
self.logger, req)
params = req.params
params['format'] = 'json'

2
swift/proxy/controllers/base.py
View File

@ -2007,7 +2007,7 @@ class Controller(object):
:returns: a swob.Response object
"""
nodes = GreenthreadSafeIterator(
node_iterator or self.app.iter_nodes(ring, part, self.logger, req)
node_iterator or NodeIter(self.app, ring, part, self.logger, req)
)
node_number = node_count or len(ring.get_part_nodes(part))
pile = GreenAsyncPile(node_number)

4
swift/proxy/controllers/container.py
View File

@ -27,7 +27,7 @@ from swift.common.constraints import check_metadata, CONTAINER_LISTING_LIMIT
from swift.common.http import HTTP_ACCEPTED, is_success
from swift.common.request_helpers import get_sys_meta_prefix, get_param, \
constrain_req_limit, validate_container_params
from swift.proxy.controllers.base import Controller, delay_denial, \
from swift.proxy.controllers.base import Controller, delay_denial, NodeIter, \
cors_validation, set_info_cache, clear_info_cache, get_container_info, \
record_cache_op_metrics, get_cache_key, headers_from_container_info, \
update_headers
@ -103,7 +103,7 @@ class ContainerController(Controller):
self.account_name, self.container_name)
concurrency = self.app.container_ring.replica_count \
if self.app.get_policy_options(None).concurrent_gets else 1
node_iter = self.app.iter_nodes(self.app.container_ring, part,
node_iter = NodeIter(self.app, self.app.container_ring, part,
self.logger, req)
resp = self.GETorHEAD_base(
req, 'Container', node_iter, part,

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

@ -70,7 +70,7 @@ from swift.common.storage_policy import (POLICIES, REPL_POLICY, EC_POLICY,
from swift.proxy.controllers.base import Controller, delay_denial, \
cors_validation, update_headers, bytes_to_skip, ByteCountEnforcer, \
record_cache_op_metrics, get_cache_key, GetterBase, GetterSource, \
is_good_source
is_good_source, NodeIter
from swift.common.swob import HTTPAccepted, HTTPBadRequest, HTTPNotFound, \
HTTPPreconditionFailed, HTTPRequestEntityTooLarge, HTTPRequestTimeout, \
HTTPServerError, HTTPServiceUnavailable, HTTPClientDisconnect, \
@ -201,7 +201,7 @@ class BaseObjectController(Controller):
policy_options = self.app.get_policy_options(policy)
is_local = policy_options.write_affinity_is_local_fn
if is_local is None:
return self.app.iter_nodes(ring, partition, self.logger, request,
return NodeIter(self.app, ring, partition, self.logger, request,
policy=policy)
primary_nodes = ring.get_part_nodes(partition)
@ -235,7 +235,7 @@ class BaseObjectController(Controller):
(node for node in all_nodes if node not in preferred_nodes)
)
return self.app.iter_nodes(ring, partition, self.logger, request,
return NodeIter(self.app, ring, partition, self.logger, request,
node_iter=node_iter, policy=policy)
def GETorHEAD(self, req):
@ -255,7 +255,7 @@ class BaseObjectController(Controller):
return aresp
partition = obj_ring.get_part(
self.account_name, self.container_name, self.object_name)
node_iter = self.app.iter_nodes(obj_ring, partition, self.logger, req,
node_iter = NodeIter(self.app, obj_ring, partition, self.logger, req,
policy=policy)
resp = self._get_or_head_response(req, node_iter, partition, policy)

7
swift/proxy/server.py
View File

@ -41,7 +41,7 @@ from swift.common.registry import register_swift_info
from swift.common.constraints import check_utf8, valid_api_version
from swift.proxy.controllers import AccountController, ContainerController, \
ObjectControllerRouter, InfoController
from swift.proxy.controllers.base import get_container_info, NodeIter, \
from swift.proxy.controllers.base import get_container_info, \
DEFAULT_RECHECK_CONTAINER_EXISTENCE, DEFAULT_RECHECK_ACCOUNT_EXISTENCE, \
DEFAULT_RECHECK_UPDATING_SHARD_RANGES, DEFAULT_RECHECK_LISTING_SHARD_RANGES
from swift.common.swob import HTTPBadRequest, HTTPForbidden, \
@ -743,11 +743,6 @@ class Application(object):
return ok
def iter_nodes(self, ring, partition, logger, request, node_iter=None,
policy=None):
return NodeIter(self, ring, partition, logger, request=request,
node_iter=node_iter, policy=policy, )
def exception_occurred(self, node, typ, additional_info,
**kwargs):
"""

3
test/unit/proxy/controllers/test_container.py
View File

@ -403,7 +403,8 @@ class TestContainerController(TestRingBase):
def __iter__(self):
return iter([])
with mocked_http_conn(), mock.patch.object(self.app, 'iter_nodes',
with mocked_http_conn(), mock.patch(
'swift.proxy.controllers.container.NodeIter',
return_value=FakeIter()):
req = Request.blank('/v1/a/c')
resp = req.get_response(self.app)

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

@ -45,7 +45,8 @@ from swift.common.utils import Timestamp, list_from_csv, md5, FileLikeIter
from swift.proxy import server as proxy_server
from swift.proxy.controllers import obj
from swift.proxy.controllers.base import \
get_container_info as _real_get_container_info, GetterSource
get_container_info as _real_get_container_info, GetterSource, \
NodeIter
from swift.common.storage_policy import POLICIES, ECDriverError, \
StoragePolicy, ECStoragePolicy
from swift.common.swob import Request
@ -2775,9 +2776,9 @@ class TestECObjController(ECObjectControllerMixin, unittest.TestCase):
def test_feed_remaining_primaries(self):
controller = self.controller_cls(
self.app, 'a', 'c', 'o')
safe_iter = utils.GreenthreadSafeIterator(self.app.iter_nodes(
self.policy.object_ring, 0, self.logger, policy=self.policy,
request=Request.blank('')))
safe_iter = utils.GreenthreadSafeIterator(NodeIter(
self.app, self.policy.object_ring, 0, self.logger,
policy=self.policy, request=Request.blank('')))
controller._fragment_GET_request = lambda *a, **k: next(safe_iter)
pile = utils.GreenAsyncPile(self.policy.ec_ndata)
for i in range(self.policy.ec_ndata):

76
test/unit/proxy/test_server.py
View File

@ -1097,7 +1097,7 @@ class TestProxyServer(unittest.TestCase):
conn = FakeConn(ip, *args, **kargs)
return conn
with mock.patch('swift.proxy.server.Application.iter_nodes',
with mock.patch('swift.proxy.controllers.account.NodeIter',
fake_iter_nodes):
with mock.patch('swift.common.bufferedhttp.http_connect_raw',
myfake_http_connect_raw):
@ -5472,8 +5472,8 @@ class TestReplicatedObjectController(
'container',
'object')
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 5)
@ -5484,8 +5484,8 @@ class TestReplicatedObjectController(
'container',
'object')
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 9)
@ -5499,8 +5499,8 @@ class TestReplicatedObjectController(
'container',
'object')
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 7)
@ -5517,8 +5517,8 @@ class TestReplicatedObjectController(
last_error=(2 ** 63 - 1))
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 7)
@ -5539,8 +5539,8 @@ class TestReplicatedObjectController(
last_error=(2 ** 63 - 1))
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 7)
@ -5565,8 +5565,8 @@ class TestReplicatedObjectController(
last_error=(2 ** 63 - 1))
collected_nodes = []
for node in self.app.iter_nodes(object_ring, partition,
self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, partition, self.logger,
request=Request.blank('')):
collected_nodes.append(node)
self.assertEqual(len(collected_nodes), 10)
@ -5596,7 +5596,8 @@ class TestReplicatedObjectController(
with mock.patch.object(self.app, 'sort_nodes',
side_effect=fake_sort_nodes):
object_ring = self.app.get_object_ring(None)
for node in self.app.iter_nodes(object_ring, 0, self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')):
pass
self.assertEqual(called, [
@ -5607,10 +5608,12 @@ class TestReplicatedObjectController(
with mock.patch.object(self.app, 'sort_nodes',
lambda n, *args, **kwargs: n):
object_ring = self.app.get_object_ring(None)
first_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, request=Request.blank('')))
second_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, request=Request.blank('')))
first_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')))
second_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')))
self.assertIn(first_nodes[0], second_nodes)
self.assertEqual(
@ -5629,14 +5632,16 @@ class TestReplicatedObjectController(
('Node will be error limited for 60.00s: %s, error: %s'
% (node_to_string(first_nodes[0]), 'test')), line)
second_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, request=Request.blank('')))
second_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')))
self.assertNotIn(first_nodes[0], second_nodes)
self.assertEqual(
1, self.logger.statsd_client.get_increment_counts().get(
'error_limiter.is_limited', 0))
third_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, request=Request.blank('')))
third_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')))
self.assertNotIn(first_nodes[0], third_nodes)
self.assertEqual(
2, self.logger.statsd_client.get_increment_counts().get(
@ -5649,10 +5654,12 @@ class TestReplicatedObjectController(
mock.patch.object(self.app, 'request_node_count',
lambda r: 6), \
mock.patch.object(object_ring, 'max_more_nodes', 99):
first_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, request=Request.blank('')))
first_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')))
second_nodes = []
for node in self.app.iter_nodes(object_ring, 0, self.logger,
for node in proxy_base.NodeIter(
self.app, object_ring, 0, self.logger,
request=Request.blank('')):
if not second_nodes:
self.app.error_limit(node, 'test')
@ -5670,8 +5677,8 @@ class TestReplicatedObjectController(
lambda n, *args, **kwargs: n), \
mock.patch.object(self.app, 'request_node_count',
lambda r: 3):
got_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, Request.blank(''),
got_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger, Request.blank(''),
node_iter=iter(node_list)))
self.assertEqual(expected[:3], got_nodes)
@ -5682,8 +5689,9 @@ class TestReplicatedObjectController(
lambda n, *args, **kwargs: n), \
mock.patch.object(self.app, 'request_node_count',
lambda r: 1000000):
got_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, req, node_iter=iter(node_list)))
got_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger, req,
node_iter=iter(node_list)))
self.assertEqual(expected, got_nodes)
def test_iter_nodes_with_replication_network(self):
@ -5697,8 +5705,9 @@ class TestReplicatedObjectController(
lambda n, *args, **kwargs: n), \
mock.patch.object(self.app, 'request_node_count',
lambda r: 3):
got_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, req, node_iter=iter(node_list)))
got_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger, req,
node_iter=iter(node_list)))
expected = [dict(n, use_replication=True) for n in node_list]
self.assertEqual(expected[:3], got_nodes)
req = Request.blank(
@ -5708,8 +5717,9 @@ class TestReplicatedObjectController(
lambda n, *args, **kwargs: n), \
mock.patch.object(self.app, 'request_node_count',
lambda r: 13):
got_nodes = list(self.app.iter_nodes(
object_ring, 0, self.logger, req, node_iter=iter(node_list)))
got_nodes = list(proxy_base.NodeIter(
self.app, object_ring, 0, self.logger, req,
node_iter=iter(node_list)))
self.assertEqual(expected, got_nodes)
def test_best_response_sets_headers(self):