From b68cc893f74bf61a03439549d1ca5412cfc8d0ff Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Tue, 28 Feb 2023 23:03:23 -0800
Subject: [PATCH] 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
---
 swift/proxy/controllers/base.py          | 10 +++++++++
 test/unit/proxy/controllers/test_base.py | 26 ++++++++++++++++++++++++
 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 d7d893b726..caf27967b1 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 8ed3528422..a2d134f434 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 5ac90171dc..e6de595529 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 = {