add lock to cachetools usage

* We observed daphne giving tracebacks when accessing logging settings.
  Originally, configure tower in tower settings was no a suspect because
  daphne is not multi-process. We've had issues with configure tower in
  tower settings and multi-process before. We later learned that Daphne
  is multi-threaded. Configure tower in tower was back to being a
  suspect. We constructed a minimal reproducer to show that multiple
  threads accessing settings can cause the same traceback that we saw in
  daphne. See
  https://gist.github.com/chrismeyersfsu/7aa4bdcf76e435efd617cb078c64d413
  for that recreator. These fixes stop the recreation.
This commit is contained in:
chris meyers 2021-11-08 11:27:03 -05:00 committed by Rebeccah
parent 36d3f9afdb
commit 21972c91dd
No known key found for this signature in database
GPG Key ID: A102D27DFFAF3552

View File

@ -234,6 +234,8 @@ class SettingsWrapper(UserSettingsHolder):
self.__dict__['_awx_conf_init_readonly'] = False
self.__dict__['cache'] = EncryptedCacheProxy(cache, registry)
self.__dict__['registry'] = registry
self.__dict__['_awx_conf_memoizedcache'] = cachetools.TTLCache(maxsize=2048, ttl=SETTING_MEMORY_TTL)
self.__dict__['_awx_conf_memoizedcache_lock'] = threading.Lock()
# record the current pid so we compare it post-fork for
# processes like the dispatcher and callback receiver
@ -396,7 +398,11 @@ class SettingsWrapper(UserSettingsHolder):
def SETTINGS_MODULE(self):
return self._get_default('SETTINGS_MODULE')
@cachetools.cached(cache=cachetools.TTLCache(maxsize=2048, ttl=SETTING_MEMORY_TTL))
@cachetools.cachedmethod(
cache=lambda self: self.__dict__['_awx_conf_memoizedcache'],
key=lambda *args, **kwargs: SettingsWrapper.hashkey(*args, **kwargs),
lock=lambda self: self.__dict__['_awx_conf_memoizedcache_lock'],
)
def __getattr__(self, name):
value = empty
if name in self.all_supported_settings:
@ -475,6 +481,23 @@ class SettingsWrapper(UserSettingsHolder):
set_on_default = getattr(self.default_settings, 'is_overridden', lambda s: False)(setting)
return set_locally or set_on_default
@classmethod
def hashkey(cls, *args, **kwargs):
"""
Usage of @cachetools.cached has changed to @cachetools.cachedmethod
The previous cachetools decorator called the hash function and passed in (self, key).
The new cachtools decorator calls the hash function with just (key).
Ideally, we would continue to pass self, however, the cachetools decorator interface
does not allow us to.
This hashkey function is to maintain that the key generated looks like
('<SettingsWrapper>', key). The thought is that maybe it is important to namespace
our cache to the SettingsWrapper scope in case some other usage of this cache exists.
I can not think of how any other system could and would use our private cache, but
for safety sake we are ensuring the key schema does not change.
"""
return cachetools.keys.hashkey(f"<{cls.__name__}>", *args, **kwargs)
def __getattr_without_cache__(self, name):
# Django 1.10 added an optimization to settings lookup: