Skip to content

Commit 808cb02

Browse files
authored
Fix cache layer todo (#363)
1 parent 1991d04 commit 808cb02

1 file changed

Lines changed: 27 additions & 14 deletions

File tree

hive/server/db.py

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@
1616

1717
CACHE_NAMESPACE = "hivemind"
1818

19+
# Sentinel value to represent 'record not found' in cache.
20+
# Using a string marker that can be easily serialized/deserialized.
21+
# This allows us to distinguish between:
22+
# - Cache miss (None from cache.get) → query DB
23+
# - Record doesn't exist (sentinel in cache) → return None (cached)
24+
# - Record exists (value in cache) → return value (cached)
25+
_CACHE_NOT_FOUND = "__CACHE_NOT_FOUND__"
26+
1927
def sqltimer(function):
2028
"""Decorator for DB query methods which tracks timing."""
2129
async def _wrapper(*args, **kwargs):
@@ -37,27 +45,32 @@ async def _wrapper(*args, **kwargs):
3745
if Stats._db.DEBUG_SQL:
3846
log.debug("[CACHE-DEBUG] cache_key: %s, value: %s", kwargs["cache_key"], v)
3947
if v is None:
40-
# Get from DB and set to cache, when miss cache
48+
# Cache miss: Get from DB and set to cache
4149
v = await func(*args, **kwargs)
4250
if Stats._db.DEBUG_SQL:
4351
log.debug("[CACHE-DEBUG] Not fit cache, cache_key: %s, Get from DB, value: %s", kwargs["cache_key"], v)
44-
if v is None:
45-
"""
46-
TODO:
47-
* Hit no cache from Redis, we will get None
48-
* Get no record from DB, we will get None
49-
These two conditions are conflict.
50-
If we don't cache the DB result None, every request will get in DB.
51-
If we cache the DB result None, we cannot charge whether cache exist or not.
52-
"""
53-
log.warning("[CACHE-LAYER-TODO] [%s] (%s)", args, kwargs)
54-
return None
5552
if "cache_ttl" in kwargs:
5653
ttl = kwargs['cache_ttl']
5754
else:
5855
ttl = 5*60
59-
await args[0].redis_cache.set(kwargs['cache_key'], v, ttl=ttl, namespace=CACHE_NAMESPACE)
60-
return v
56+
# Use sentinel value to cache "not found" results only for query_one
57+
# For query_col and query_all, empty list [] is a valid result and should be cached as-is
58+
# For query_one, None means "record doesn't exist" and needs sentinel to distinguish from cache miss
59+
func_name = func.__name__
60+
if func_name == 'query_one' and v is None:
61+
# Only use sentinel for query_one when result is None
62+
cache_value = _CACHE_NOT_FOUND
63+
else:
64+
# For query_col, query_all, query_row: cache the actual result (including empty lists)
65+
cache_value = v
66+
await args[0].redis_cache.set(kwargs['cache_key'], cache_value, ttl=ttl, namespace=CACHE_NAMESPACE)
67+
return v
68+
elif v == _CACHE_NOT_FOUND:
69+
# Cache hit with sentinel: record doesn't exist (cached) - only for query_one
70+
return None
71+
else:
72+
# Cache hit with value: record exists (cached) or empty list for query_col/query_all
73+
return v
6174
else:
6275
return await func(*args, **kwargs)
6376
return _wrapper

0 commit comments

Comments
 (0)