fix: add threading lock to graph cache, expand docstring
Address review feedback from @bensig: 1. Wrap cache reads/writes in threading.Lock for thread safety 2. Promote the col-arg caveat from inline comment to docstring Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -18,6 +18,7 @@ No external graph DB needed — built from ChromaDB metadata.
|
||||
import hashlib
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
import time
|
||||
from collections import Counter, defaultdict
|
||||
from datetime import datetime, timezone
|
||||
@@ -28,6 +29,7 @@ from .palace import mine_lock
|
||||
|
||||
# Module-level graph cache with TTL and write-invalidation.
|
||||
# Warm cache serves build_graph() in O(1); invalidate_graph_cache() clears on writes.
|
||||
_graph_cache_lock = threading.Lock()
|
||||
_graph_cache_nodes = None
|
||||
_graph_cache_edges = None
|
||||
_graph_cache_time = 0.0
|
||||
@@ -37,9 +39,10 @@ _GRAPH_CACHE_TTL = 60.0 # seconds — graph changes less often than metadata
|
||||
def invalidate_graph_cache():
|
||||
"""Clear the graph cache. Called from mcp_server.py on writes."""
|
||||
global _graph_cache_nodes, _graph_cache_edges, _graph_cache_time
|
||||
_graph_cache_nodes = None
|
||||
_graph_cache_edges = None
|
||||
_graph_cache_time = 0.0
|
||||
with _graph_cache_lock:
|
||||
_graph_cache_nodes = None
|
||||
_graph_cache_edges = None
|
||||
_graph_cache_time = 0.0
|
||||
|
||||
|
||||
def _get_collection(config=None):
|
||||
@@ -59,7 +62,11 @@ def build_graph(col=None, config=None):
|
||||
Build the palace graph from ChromaDB metadata.
|
||||
|
||||
Returns cached result if fresh (within TTL). Cache is invalidated
|
||||
on writes via invalidate_graph_cache().
|
||||
on writes via invalidate_graph_cache(). Thread-safe via _graph_cache_lock.
|
||||
|
||||
Note: warm cache ignores ``col`` and ``config`` arguments — this is
|
||||
intentional for the MCP server's single-palace use case. Callers
|
||||
switching collections should call ``invalidate_graph_cache()`` first.
|
||||
|
||||
Returns:
|
||||
nodes: dict of {room: {wings: set, halls: set, count: int}}
|
||||
@@ -69,8 +76,9 @@ def build_graph(col=None, config=None):
|
||||
now = time.time()
|
||||
# NOTE: warm cache ignores col/config args — intentional for the MCP server's
|
||||
# single-palace use case. Callers switching collections must invalidate first.
|
||||
if _graph_cache_nodes is not None and (now - _graph_cache_time) < _GRAPH_CACHE_TTL:
|
||||
return _graph_cache_nodes, _graph_cache_edges
|
||||
with _graph_cache_lock:
|
||||
if _graph_cache_nodes is not None and (now - _graph_cache_time) < _GRAPH_CACHE_TTL:
|
||||
return _graph_cache_nodes, _graph_cache_edges
|
||||
|
||||
if col is None:
|
||||
col = _get_collection(config)
|
||||
@@ -130,9 +138,10 @@ def build_graph(col=None, config=None):
|
||||
# Only cache non-empty graphs so new data is picked up immediately
|
||||
# when the palace is first populated.
|
||||
if nodes:
|
||||
_graph_cache_nodes = nodes
|
||||
_graph_cache_edges = edges
|
||||
_graph_cache_time = time.time()
|
||||
with _graph_cache_lock:
|
||||
_graph_cache_nodes = nodes
|
||||
_graph_cache_edges = edges
|
||||
_graph_cache_time = time.time()
|
||||
|
||||
return nodes, edges
|
||||
|
||||
|
||||
Reference in New Issue
Block a user