Skip to content

Commit cd6d332

Browse files
feat(cache): drop L2 (shared) tier for singleton settings cache; L1-only
The singleton settings cache (dojo/caching.py) becomes a single in-process L1 tier; the shared django.core.cache (L2) tier is removed. Cross-process/worker freshness is provided entirely by resetting L1 at each request and task boundary (middleware + DojoAsyncTask), so each request/task reads a singleton from the DB at most once and never serves a value cached during a prior request/task. Why no L2 for settings singletons: - The dedupe correctness fix only needs each task to see current config, which the per-task L1 reset already guarantees by re-reading the DB -- a shared L2 + cross-process invalidation is not required for correctness. - L2 added real cost and complexity for little gain on these tiny, hot singletons: a Redis dependency, pickled-vs-dict storage and version-keying, cross-process invalidation, and Redis-down fragility -- while the headline win (avoiding repeated reads within a bulk loop) comes from L1, not L2. - Trade-off: one small indexed DB read per singleton per request/task instead of amortising across processes via Redis. Acceptable for low-cardinality config. Removes SETTINGS_CACHE_L2_TTL and the L2 read/set/delete paths. DD_CACHE_URL and the Redis default django cache backend stay (used by other framework caching). Unit-test overrides drop the now-defunct L2 TTL var. test_caching rewritten to the L1-only contract.
1 parent 19cb710 commit cd6d332

6 files changed

Lines changed: 67 additions & 133 deletions

File tree

docker-compose.override.unit_tests.yml

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,11 @@ services:
2222
DD_DATABASE_USER: ${DD_DATABASE_USER:-defectdojo}
2323
DD_DATABASE_PASSWORD: ${DD_DATABASE_PASSWORD:-defectdojo}
2424
DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
25-
# No shared cache backend in unit tests (no Redis/valkey here) -> LocMemCache.
25+
# No Redis/valkey in unit tests -> default django cache is LocMemCache.
2626
DD_CACHE_URL: ''
27-
# L1 (in-process, request/test-scoped) stays ON for deterministic
28-
# assertNumQueries counts; reset per request (middleware) and per test
29-
# (dojo_test_case setUp). L2 is OFF (no shared backend; would persist).
27+
# In-process singleton cache (dojo/caching.py) stays ON for deterministic
28+
# assertNumQueries counts; reset per request (middleware) and per test.
3029
DD_SETTINGS_CACHE_L1_TTL: '30'
31-
DD_SETTINGS_CACHE_L2_TTL: '-1'
3230
DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
3331
celerybeat: !reset
3432
celeryworker: !reset

docker-compose.override.unit_tests_cicd.yml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,12 @@ services:
2323
DD_CELERY_BROKER_URL: 'sqla+sqlite:///dojo.celerydb.sqlite'
2424
DD_JIRA_EXTRA_ISSUE_TYPES: 'Vulnerability' # Shouldn't trigger a migration error
2525
DD_V3_FEATURE_LOCATIONS: ${DD_V3_FEATURE_LOCATIONS:-False}
26-
# No shared cache backend in unit tests (no Redis/valkey here) -> LocMemCache.
26+
# No Redis/valkey in unit tests -> default django cache is LocMemCache.
2727
DD_CACHE_URL: ''
28-
# L1 (in-process, request/test-scoped) stays ON so a singleton is read once
29-
# per request/test and assertNumQueries counts are deterministic; it is reset
30-
# per request (middleware) and per test (dojo_test_case setUp). L2 is OFF
31-
# (no shared backend here, and it would persist across tests/processes).
28+
# In-process singleton cache (dojo/caching.py) stays ON: a singleton is read
29+
# once per request/test (deterministic assertNumQueries), reset per request
30+
# (middleware) and per test (dojo_test_case setUp).
3231
DD_SETTINGS_CACHE_L1_TTL: '30'
33-
DD_SETTINGS_CACHE_L2_TTL: '-1'
3432
celerybeat: !reset
3533
celeryworker: !reset
3634
initializer: !reset

dojo/caching.py

Lines changed: 30 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,36 @@
11
"""
2-
Two-tier read-through cache for global, low-cardinality singleton config.
3-
4-
One in-process **L1** tier sits on top of the shared **L2** tier
5-
(``django.core.cache``, Redis in deployments). A getter is resolved L1 → L2 → DB:
6-
the first tier with a value wins; a ``None`` anywhere means "not cached, compute
7-
it" and is never stored. This is deliberately simple — it is only for global,
8-
user-INDEPENDENT, signal-invalidated singletons (feature flags, system settings,
9-
and the like), never per-user or per-object data.
2+
In-process read-through cache for global, low-cardinality singleton config.
3+
4+
A single per-thread **L1** tier resolves a getter L1 → DB: a hit wins; a ``None``
5+
result means "not cached, compute it" and is never stored. This is deliberately
6+
simple — it is only for global, user-INDEPENDENT, signal-invalidated singletons
7+
(feature flags, system settings, and the like), never per-user or per-object data.
8+
9+
There is intentionally **no shared/cross-process (L2) tier**: freshness is provided
10+
by resetting L1 at every request and task boundary (middleware + the Celery task
11+
base), so each request/task reads the singleton from the DB at most once and never
12+
serves a value cached during a prior request/task (e.g. a since-changed
13+
``System_Settings``). This keeps the design free of a Redis dependency, pickled
14+
model graphs, and cross-process invalidation — at the cost of one DB read per
15+
singleton per request/task. (The default ``django.core.cache`` backend may still be
16+
Redis for other uses; this module no longer reads or writes it.)
1017
1118
Values are stored as plain dicts/scalars (see ``model_to_cache_dict`` /
1219
``cache_dict_to_model``), never pickled model instances.
1320
14-
Two independent tiers, each turned off by setting its TTL to ``-1`` (both off makes
15-
the decorator a pass-through). Configuration (Django settings, wired from env in
16-
``settings.dist.py``):
21+
Configuration (Django setting, wired from env in ``settings.dist.py``):
1722
1823
* ``SETTINGS_CACHE_L1_TTL`` — per-thread in-process freshness budget in seconds
19-
(``-1`` disables L1). Keep it short — it bounds cross-process staleness. L1 is
20-
reset at each request/task boundary, so it is effectively request/task scoped.
21-
* ``SETTINGS_CACHE_L2_TTL`` — L2 timeout in seconds (``-1`` disables L2).
24+
(``-1`` disables the cache, making the decorator a pass-through). Keep it short.
25+
L1 is reset at each request/task boundary, so it is effectively request/task
26+
scoped.
2227
"""
2328

2429
import threading
2530
import time
2631
from functools import wraps
2732

2833
from django.conf import settings
29-
from django.core.cache import cache
3034

3135

3236
class _L1Store:
@@ -88,34 +92,23 @@ def clear(self):
8892

8993
def dojo_settings_cache(*, key: str):
9094
"""
91-
Read-through L1+L2 cache for a fixed-key singleton getter.
95+
Read-through in-process (L1) cache for a fixed-key singleton getter.
9296
93-
Resolves L1 → L2 → wrapped function. A ``None`` result is treated as "no
94-
value" and is not cached (so the next call retries). Becomes a pass-through
95-
when both tiers are disabled (``SETTINGS_CACHE_L1_TTL=-1`` and
96-
``SETTINGS_CACHE_L2_TTL=-1``).
97+
Resolves L1 → wrapped function. A ``None`` result is treated as "no value" and
98+
is not cached (so the next call retries). Becomes a pass-through when L1 is
99+
disabled (``SETTINGS_CACHE_L1_TTL=-1``). Freshness across processes comes from
100+
resetting L1 each request/task (see ``reset_l1_cache``), not a shared tier.
97101
"""
98102

99103
def decorator(fn):
100104
@wraps(fn)
101105
def wrapper(*args, **kwargs):
102-
l2_ttl = getattr(settings, "SETTINGS_CACHE_L2_TTL", 300)
103-
l2_on = l2_ttl >= 0 # SETTINGS_CACHE_L2_TTL == -1 disables L2
104-
105106
value = _L1_STORE.get(key) # ---- L1 ----
106107
if value is not None:
107108
return value
108109

109-
if l2_on: # ---- L2 ----
110-
value = cache.get(key)
111-
if value is not None:
112-
_L1_STORE.set(key, value)
113-
return value
114-
115110
value = fn(*args, **kwargs) # ---- miss: compute ----
116111
if value is not None:
117-
if l2_on:
118-
cache.set(key, value, timeout=l2_ttl)
119112
_L1_STORE.set(key, value)
120113
return value
121114

@@ -125,12 +118,12 @@ def wrapper(*args, **kwargs):
125118

126119

127120
def invalidate_dojo_settings_cache(key: str) -> None:
128-
"""Drop a cached singleton from L2 (all processes) and L1 (this process)."""
129-
# Only touch L2 when it is enabled; when SETTINGS_CACHE_L2_TTL == -1 the L2
130-
# tier is off and the backend (``cache``) may not even be reachable (e.g.
131-
# unit tests run with no Redis configured), so skip the delete entirely.
132-
if getattr(settings, "SETTINGS_CACHE_L2_TTL", 300) >= 0:
133-
cache.delete(key)
121+
"""
122+
Drop a cached singleton from L1 (this thread).
123+
124+
With no shared tier, other threads/processes self-heal at their next
125+
request/task boundary (L1 reset), so there is nothing cross-process to drop.
126+
"""
134127
_L1_STORE.invalidate(key)
135128

136129

dojo/settings/settings.dist.py

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -280,16 +280,15 @@
280280
DD_V3_FEATURE_LOCATIONS=(bool, True),
281281
# Dictates if v3 org/asset relabeling (+url routing) will be enabled (on by default as of 3.0.0; set to False to restore Product/Product Type labels and URLs)
282282
DD_ENABLE_V3_ORGANIZATION_ASSET_RELABEL=(bool, True),
283-
# Shared cache backend (django.core.cache). When set, used as the L2 tier for
284-
# dojo/caching.py. MUST be a cross-process store (e.g. redis://valkey:6379/1)
285-
# so cache invalidation propagates across uwsgi/celery processes; when empty
286-
# Django falls back to per-process LocMemCache (single-process only).
283+
# Shared cache backend (django.core.cache). When set, Django uses RedisCache
284+
# (e.g. redis://valkey:6379/1); when empty it falls back to LocMemCache. Used
285+
# by general framework caching; the singleton settings cache (dojo/caching.py)
286+
# is in-process only and does not read or write this backend.
287287
DD_CACHE_URL=(str, ""),
288-
# Two-tier read-through cache for global singleton getters (see dojo/caching.py).
289-
# Per-thread in-process L1 freshness budget in seconds; -1 disables L1.
288+
# In-process (L1) read-through cache for global singleton getters (see
289+
# dojo/caching.py). Per-thread freshness budget in seconds; -1 disables it.
290+
# Reset every request/task, so each request/task reads the singleton once.
290291
DD_SETTINGS_CACHE_L1_TTL=(int, 30),
291-
# L2 (django.core.cache) timeout in seconds; -1 disables L2.
292-
DD_SETTINGS_CACHE_L2_TTL=(int, 300),
293292
# Notification env-vars (SLA notify, alert refresh/counter/cap, system-level trump). Defined in dojo.notifications.settings.
294293
**NOTIFICATIONS_ENV_DEFAULTS,
295294
)
@@ -338,9 +337,9 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param
338337
# Raises django's ImproperlyConfigured exception if SECRET_KEY not in os.environ
339338
SECRET_KEY = env("DD_SECRET_KEY")
340339

341-
# Shared cache backend. A cross-process store (Redis) is required for cache
342-
# invalidation to propagate across uwsgi/celery; otherwise Django defaults to
343-
# per-process LocMemCache (fine only single-process).
340+
# Default cache backend (django.core.cache). Redis when DD_CACHE_URL is set,
341+
# else per-process LocMemCache. General framework caching only; the singleton
342+
# settings cache (dojo/caching.py) is in-process and does not use this backend.
344343
if env("DD_CACHE_URL"):
345344
CACHES = {
346345
"default": {
@@ -349,9 +348,8 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param
349348
},
350349
}
351350

352-
# Two-tier singleton cache (dojo/caching.py)
351+
# In-process singleton cache (dojo/caching.py)
353352
SETTINGS_CACHE_L1_TTL = env("DD_SETTINGS_CACHE_L1_TTL")
354-
SETTINGS_CACHE_L2_TTL = env("DD_SETTINGS_CACHE_L2_TTL")
355353

356354
# Local time zone for this installation. Choices can be found here:
357355
# http://en.wikipedia.org/wiki/List_of_tz_zones_by_name

unittests/test_caching.py

Lines changed: 15 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
1-
from unittest import mock
2-
31
from django.test import TestCase, override_settings
42

5-
from dojo import caching
63
from dojo.caching import (
74
_L1_STORE, # noqa: PLC2701 test needs the in-process store
85
cache_dict_to_model,
@@ -15,42 +12,20 @@
1512
from .dojo_test_case import DojoTestCase
1613

1714

18-
class _FakeCache:
19-
20-
"""Dict-backed stand-in for ``django.core.cache.cache`` that counts access."""
21-
22-
def __init__(self):
23-
self.store = {}
24-
self.gets = 0
25-
self.sets = 0
26-
self.deletes = 0
27-
28-
def get(self, key, default=None):
29-
self.gets += 1
30-
return self.store.get(key, default)
31-
32-
def set(self, key, value, timeout=None):
33-
self.sets += 1
34-
self.store[key] = value
35-
36-
def delete(self, key):
37-
self.deletes += 1
38-
self.store.pop(key, None)
39-
40-
41-
@override_settings(SETTINGS_CACHE_L2_TTL=300, SETTINGS_CACHE_L1_TTL=30)
15+
@override_settings(SETTINGS_CACHE_L1_TTL=30)
4216
class DojoSettingsCacheTest(TestCase):
4317

44-
"""Unit tests for the simplified L1+L2 read-through decorator (dojo/caching.py)."""
18+
"""
19+
Unit tests for the in-process (L1) read-through decorator (dojo/caching.py).
20+
21+
There is no shared/L2 tier: the decorator only memoizes in-process and relies
22+
on L1 reset at request/task boundaries for cross-process freshness.
23+
"""
4524

4625
def setUp(self):
47-
self.fake = _FakeCache()
48-
self._patch = mock.patch.object(caching, "cache", self.fake)
49-
self._patch.start()
5026
_L1_STORE.clear()
5127

5228
def tearDown(self):
53-
self._patch.stop()
5429
_L1_STORE.clear()
5530

5631
def _build_getter(self, *, key="k", returns=1):
@@ -63,68 +38,40 @@ def getter():
6338

6439
return getter, calls
6540

66-
@override_settings(SETTINGS_CACHE_L1_TTL=-1)
67-
def test_l1_disabled_consults_l2_every_call(self):
68-
getter, calls = self._build_getter()
69-
self.assertEqual(getter(), 1)
70-
self.assertEqual(getter(), 1) # served from L2, fn not re-run
71-
self.assertEqual(calls["n"], 1) # computed once
72-
self.assertEqual(self.fake.gets, 2) # L2 consulted every call
73-
74-
def test_l1_serves_without_l2(self):
41+
def test_l1_memoizes_in_process(self):
7542
getter, calls = self._build_getter()
7643
getter()
7744
getter()
7845
getter()
79-
self.assertEqual(calls["n"], 1)
80-
self.assertEqual(self.fake.gets, 1) # only the first call reaches L2
46+
self.assertEqual(calls["n"], 1) # computed once, then served from L1
8147

82-
@override_settings(SETTINGS_CACHE_L2_TTL=-1)
83-
def test_l2_ttl_negative_disables_l2(self):
84-
getter, calls = self._build_getter()
85-
getter()
86-
getter()
87-
self.assertEqual(self.fake.gets, 0) # L2 never consulted
88-
self.assertEqual(self.fake.sets, 0)
89-
self.assertEqual(calls["n"], 1) # but L1 still memoizes in-process
90-
91-
@override_settings(SETTINGS_CACHE_L1_TTL=-1, SETTINGS_CACHE_L2_TTL=-1)
92-
def test_both_tiers_disabled_is_passthrough(self):
48+
@override_settings(SETTINGS_CACHE_L1_TTL=-1)
49+
def test_l1_disabled_is_passthrough(self):
9350
getter, calls = self._build_getter()
9451
getter()
9552
getter()
96-
self.assertEqual(calls["n"], 2) # recomputed every call
97-
self.assertEqual(self.fake.gets, 0) # L2 never consulted
53+
self.assertEqual(calls["n"], 2) # recomputed every call when L1 off
9854

99-
def test_reset_clears_l1(self):
55+
def test_reset_recomputes(self):
10056
getter, calls = self._build_getter()
10157
getter()
10258
_L1_STORE.reset()
10359
getter()
104-
self.assertEqual(calls["n"], 1) # reset drops L1, but L2 still serves
105-
self.assertEqual(self.fake.gets, 2) # 2nd call re-reads L2 after reset
60+
self.assertEqual(calls["n"], 2) # reset drops L1, so it recomputes
10661

10762
def test_none_result_is_not_cached(self):
10863
getter, calls = self._build_getter(returns=None)
10964
self.assertIsNone(getter())
11065
self.assertIsNone(getter())
11166
self.assertEqual(calls["n"], 2) # None recomputed each call (never stored)
112-
self.assertEqual(self.fake.sets, 0)
11367

114-
def test_invalidate_clears_l1_and_l2(self):
68+
def test_invalidate_recomputes(self):
11569
getter, calls = self._build_getter(returns=7)
11670
self.assertEqual(getter(), 7)
11771
invalidate_dojo_settings_cache("k")
11872
getter()
11973
self.assertEqual(calls["n"], 2) # recomputed after invalidation
12074

121-
@override_settings(SETTINGS_CACHE_L2_TTL=-1)
122-
def test_invalidate_skips_l2_when_disabled(self):
123-
# With L2 off the backend may be unreachable (no Redis in unit tests);
124-
# invalidation must not call cache.delete.
125-
invalidate_dojo_settings_cache("k")
126-
self.assertEqual(self.fake.deletes, 0)
127-
12875

12976
class ModelDictRoundTripTest(DojoTestCase):
13077

unittests/test_system_settings.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,15 @@ def test_post_request_initializes_form_with_finding_instance(self):
9393
self.assertIn(response.status_code, [200, 302])
9494

9595

96-
@override_settings(SETTINGS_CACHE_L1_TTL=30, SETTINGS_CACHE_L2_TTL=-1)
96+
@override_settings(SETTINGS_CACHE_L1_TTL=30)
9797
class TestSystemSettingsMiddlewareIntegration(DojoTestCase):
9898

9999
"""
100100
Integration tests for DojoSettingsManagerMiddleware + System_Settings_Manager.
101101
102-
Caching lives in dojo.caching (decorator); the middleware only resets the
103-
request-scoped L1 tier and surfaces a load error. These tests pin L1 on / L2
104-
off via override_settings so they don't depend on the compose env.
102+
Caching lives in dojo.caching (in-process L1 decorator); the middleware resets
103+
the request-scoped L1 tier and surfaces a load error. These tests pin L1 on via
104+
override_settings so they don't depend on the compose env.
105105
"""
106106

107107
def setUp(self):

0 commit comments

Comments
 (0)