Skip to content

Commit

Permalink
Only use in-memory cache for database settings, set ttl=5 (#12166)
Browse files Browse the repository at this point in the history
* Only use in-memory cache for database settings

Make necessary adjustments to monkeypatch
  as it is very vunerable to recursion
  Remove migration exception that is now redundant

Clear cache if a setting is changed

* Use dedicated middleware for setting cache stuff
  Clear cache for each request

* Add tests for in-memory cache
  • Loading branch information
AlanCoding committed May 11, 2022
1 parent f3725c7 commit aaad634
Show file tree
Hide file tree
Showing 8 changed files with 61 additions and 6 deletions.
3 changes: 2 additions & 1 deletion awx/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,10 @@ def oauth2_getattribute(self, attr):
# Custom method to override
# oauth2_provider.settings.OAuth2ProviderSettings.__getattribute__
from django.conf import settings
from oauth2_provider.settings import DEFAULTS

val = None
if 'migrate' not in sys.argv:
if (isinstance(attr, str)) and (attr in DEFAULTS) and (not attr.startswith('_')):
# certain Django OAuth Toolkit migrations actually reference
# setting lookups for references to model classes (e.g.,
# oauth2_settings.REFRESH_TOKEN_MODEL)
Expand Down
11 changes: 7 additions & 4 deletions awx/conf/settings.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# Python
import contextlib
import logging
import sys
import threading
import time
import os
Expand Down Expand Up @@ -31,7 +30,7 @@

logger = logging.getLogger('awx.conf.settings')

SETTING_MEMORY_TTL = 5 if 'callback_receiver' in ' '.join(sys.argv) else 0
SETTING_MEMORY_TTL = 5

# Store a special value to indicate when a setting is not set in the database.
SETTING_CACHE_NOTSET = '___notset___'
Expand Down Expand Up @@ -403,11 +402,15 @@ def SETTINGS_MODULE(self):
key=lambda *args, **kwargs: SettingsWrapper.hashkey(*args, **kwargs),
lock=lambda self: self.__dict__['_awx_conf_memoizedcache_lock'],
)
def _get_local_with_cache(self, name):
"""Get value while accepting the in-memory cache if key is available"""
with _ctit_db_wrapper(trans_safe=True):
return self._get_local(name)

def __getattr__(self, name):
value = empty
if name in self.all_supported_settings:
with _ctit_db_wrapper(trans_safe=True):
value = self._get_local(name)
value = self._get_local_with_cache(name)
if value is not empty:
return value
return self._get_default(name)
Expand Down
3 changes: 3 additions & 0 deletions awx/conf/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ def handle_setting_change(key, for_delete=False):
cache_keys = {Setting.get_cache_key(k) for k in setting_keys}
cache.delete_many(cache_keys)

# if we have changed a setting, we want to avoid mucking with the in-memory cache entirely
settings._awx_conf_memoizedcache.clear()

# Send setting_changed signal with new value for each setting.
for setting_key in setting_keys:
setting_changed.send(sender=Setting, setting=setting_key, value=getattr(settings, setting_key, None), enter=not bool(for_delete))
Expand Down
32 changes: 32 additions & 0 deletions awx/conf/tests/unit/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
from uuid import uuid4
import time

from unittest import mock

from django.conf import LazySettings
from django.core.cache.backends.locmem import LocMemCache
from django.core.exceptions import ImproperlyConfigured
Expand Down Expand Up @@ -299,3 +301,33 @@ def rot13(obj, attribute):
cache.set('AWX_ENCRYPTED', 'SECRET!')
assert cache.get('AWX_ENCRYPTED') == 'SECRET!'
assert native_cache.get('AWX_ENCRYPTED') == 'FRPERG!'


@pytest.mark.defined_in_file(AWX_VAR='DEFAULT')
def test_in_memory_cache_only_for_registered_settings(settings):
"Test that we only make use of the in-memory TTL cache for registered settings"
settings._awx_conf_memoizedcache.clear()
settings.MIDDLEWARE
assert len(settings._awx_conf_memoizedcache) == 0 # does not cache MIDDLEWARE
settings.registry.register('AWX_VAR', field_class=fields.CharField, category=_('System'), category_slug='system')
settings._wrapped.__dict__['all_supported_settings'] = ['AWX_VAR'] # because it is cached_property
settings._awx_conf_memoizedcache.clear()
assert settings.AWX_VAR == 'DEFAULT'
assert len(settings._awx_conf_memoizedcache) == 1 # caches registered settings


@pytest.mark.defined_in_file(AWX_VAR='DEFAULT')
def test_in_memory_cache_works(settings):
settings._awx_conf_memoizedcache.clear()
settings.registry.register('AWX_VAR', field_class=fields.CharField, category=_('System'), category_slug='system')
settings._wrapped.__dict__['all_supported_settings'] = ['AWX_VAR']

settings._awx_conf_memoizedcache.clear()

with mock.patch('awx.conf.settings.SettingsWrapper._get_local', return_value='DEFAULT') as mock_get:
assert settings.AWX_VAR == 'DEFAULT'
mock_get.assert_called_once_with('AWX_VAR')

with mock.patch.object(settings, '_get_local') as mock_get:
assert settings.AWX_VAR == 'DEFAULT'
mock_get.assert_not_called()
11 changes: 11 additions & 0 deletions awx/main/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,17 @@
perf_logger = logging.getLogger('awx.analytics.performance')


class SettingsCacheMiddleware(MiddlewareMixin):
"""
Clears the in-memory settings cache at the beginning of a request.
We do this so that a script can POST to /api/v2/settings/all/ and then
right away GET /api/v2/settings/all/ and see the updated value.
"""

def process_request(self, request):
settings._awx_conf_memoizedcache.clear()


class TimingMiddleware(threading.local, MiddlewareMixin):

dest = '/var/log/tower/profile'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ def test_encrypted_setting_values(self):
settings.cache.delete('REDHAT_PASSWORD')

# verify that the old SECRET_KEY doesn't work
settings._awx_conf_memoizedcache.clear()
with pytest.raises(InvalidToken):
settings.REDHAT_PASSWORD

# verify that the new SECRET_KEY *does* work
settings._awx_conf_memoizedcache.clear()
with override_settings(SECRET_KEY=new_key):
assert settings.REDHAT_PASSWORD == 'sensitive'

Expand Down
4 changes: 3 additions & 1 deletion awx/main/tests/functional/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from awx.main.models.inventory import Inventory, InventorySource
from awx.main.models.jobs import JobTemplate

from django.test.utils import override_settings


@pytest.mark.django_db
def test_get_notification_template_list(get, user, notification_template):
Expand Down Expand Up @@ -163,7 +165,7 @@ def test_custom_environment_injection(post, user, organization):
)
assert response.status_code == 201
template = NotificationTemplate.objects.get(pk=response.data['id'])
with pytest.raises(ConnectionError), mock.patch('django.conf.settings.AWX_TASK_ENV', {'HTTPS_PROXY': '192.168.50.100:1234'}), mock.patch.object(
with pytest.raises(ConnectionError), override_settings(AWX_TASK_ENV={'HTTPS_PROXY': '192.168.50.100:1234'}), mock.patch.object(
HTTPAdapter, 'send'
) as fake_send:

Expand Down
1 change: 1 addition & 0 deletions awx/settings/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,7 @@ def IS_TESTING(argv=None):

MIDDLEWARE = [
'django_guid.middleware.guid_middleware',
'awx.main.middleware.SettingsCacheMiddleware',
'awx.main.middleware.TimingMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'awx.main.middleware.MigrationRanCheckMiddleware',
Expand Down

0 comments on commit aaad634

Please sign in to comment.