diff options
author | Tim Burke <tim.burke@gmail.com> | 2023-02-28 23:03:23 -0800 |
---|---|---|
committer | Tim Burke <tim.burke@gmail.com> | 2023-03-01 15:02:42 -0800 |
commit | b68cc893f74bf61a03439549d1ca5412cfc8d0ff (patch) | |
tree | 9571db87ec543500d1bc4646079219997a22769f | |
parent | 2a90eadc0d20cef9833a0e7737f5c36becf46b3b (diff) | |
download | swift-b68cc893f74bf61a03439549d1ca5412cfc8d0ff.tar.gz |
proxy: Reduce round-trips to memcache and backend on info misses
Following a memcache restart in a SAIO, I've seen the following happen
during an object HEAD:
- etag_quoter wants to get account/container info to decide whether to
quote-wrap or not
- account info is a cache miss, so we make a no-auth'ed HEAD to the next
filter in the pipeline
- eventually this gets down to ratelimit, which *also* wants to get
account info
- still a cache miss, so we make a *separate* HEAD that eventually talks
to the backend and populates cache
- ratelimit realizes it can't ratelimit the request and lets the
original HEAD through to the backend
There's a related bug about how something similar can happen when the
backend gets overloaded, but *everything is working* -- we just ought to
be talking straight to the proxy app.
Note that there's likely something similar going on with container info,
but the hardcoded 10% sampling rate makes it harder to see if you're
monitoring raw metric streams.
I thought I fixed this in the related change, but no :-/
Change-Id: I49447c62abf9375541f396f984c91e128b8a05d5
Related-Change: If9249a42b30e2a2e7c4b0b91f947f24bf891b86f
Related-Bug: #1883214
-rw-r--r-- | swift/proxy/controllers/base.py | 10 | ||||
-rw-r--r-- | test/unit/proxy/controllers/test_base.py | 26 | ||||
-rw-r--r-- | test/unit/proxy/test_server.py | 6 |
3 files changed, 40 insertions, 2 deletions
diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index d7d893b72..caf27967b 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -438,6 +438,11 @@ def get_container_info(env, app, swift_source=None): account = wsgi_to_str(wsgi_account) container = wsgi_to_str(wsgi_container) + # Try to cut through all the layers to the proxy app + try: + app = app._pipeline_final_app + except AttributeError: + pass # Check in environment cache and in memcache (in that order) info = _get_info_from_caches(app, env, account, container) @@ -526,6 +531,11 @@ def get_account_info(env, app, swift_source=None): account = wsgi_to_str(wsgi_account) + # Try to cut through all the layers to the proxy app + try: + app = app._pipeline_final_app + except AttributeError: + pass # Check in environment cache and in memcache (in that order) info = _get_info_from_caches(app, env, account) diff --git a/test/unit/proxy/controllers/test_base.py b/test/unit/proxy/controllers/test_base.py index 8ed352842..a2d134f43 100644 --- a/test/unit/proxy/controllers/test_base.py +++ b/test/unit/proxy/controllers/test_base.py @@ -152,7 +152,10 @@ class ZeroCacheDynamicResponseFactory(DynamicResponseFactory): class FakeApp(object): recheck_container_existence = 30 + container_existence_skip_cache = 0 recheck_account_existence = 30 + account_existence_skip_cache = 0 + logger = None def __init__(self, response_factory=None, statuses=None): self.responses = response_factory or \ @@ -352,6 +355,29 @@ class TestFuncs(BaseTest): self.assertEqual([e['swift.source'] for e in app.captured_envs], ['MC', 'MC']) + def test_get_container_info_in_pipeline(self): + final_app = FakeApp() + + def factory(app): + def wsgi_filter(env, start_response): + # lots of middlewares get info... + if env['PATH_INFO'].count('/') > 2: + get_container_info(env, app) + else: + get_account_info(env, app) + # ...then decide to no-op based on the result + return app(env, start_response) + + wsgi_filter._pipeline_final_app = final_app + return wsgi_filter + + # build up a pipeline + filtered_app = factory(factory(factory(final_app))) + req = Request.blank("/v1/a/c/o", environ={'swift.cache': FakeCache()}) + req.get_response(filtered_app) + self.assertEqual([e['PATH_INFO'] for e in final_app.captured_envs], + ['/v1/a', '/v1/a/c', '/v1/a/c/o']) + def test_get_object_info_swift_source(self): app = FakeApp() req = Request.blank("/v1/a/c/o", diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 5ac90171d..e6de59552 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -513,7 +513,8 @@ class TestController(unittest.TestCase): def test_get_account_info_returns_values_as_strings(self): app = mock.MagicMock() - app._pipeline_final_app.account_existence_skip_cache = 0.0 + app._pipeline_final_app = app + app.account_existence_skip_cache = 0.0 memcache = mock.MagicMock() memcache.get = mock.MagicMock() memcache.get.return_value = { @@ -539,7 +540,8 @@ class TestController(unittest.TestCase): def test_get_container_info_returns_values_as_strings(self): app = mock.MagicMock() - app._pipeline_final_app.container_existence_skip_cache = 0.0 + app._pipeline_final_app = app + app.container_existence_skip_cache = 0.0 memcache = mock.MagicMock() memcache.get = mock.MagicMock() memcache.get.return_value = { |