Skip to content

Commit 13f9de0

Browse files
lmignonlembregtse
authored andcommitted
[IMP] fastapi: Improves app cache lifecycle
This commit improves the lifecycle of the fastapi app cache. It first ensures that the cache is effectively invalidated when changes are made to the app configuration even if theses changes occur into an other server instance. It also remove the use of a locking mechanism put in place to ensure a thread safe access to a value into the cache to avoid potential concurrency issue when a default value is set to the cache at access time. This lock could lead to unnecessary contention and reduce the performance benefits of queue.Queue's fine-grained internal synchronization for a questionable gain. The only expected gain was to avoid the useless creation of a queue.Queue instance that would never be used since at the time of puting the value into the cache we are sure that a value is already present into the dictionary.
1 parent ca1aee6 commit 13f9de0

3 files changed

Lines changed: 91 additions & 13 deletions

File tree

fastapi/fastapi_dispatcher.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def dispatch(self, endpoint, args):
2525
root_path = "/" + environ["PATH_INFO"].split("/")[1]
2626
# TODO store the env into contextvar to be used by the odoo_env
2727
# depends method
28-
with fastapi_app_pool.get_app(root_path, request.env) as app:
28+
with fastapi_app_pool.get_app(env=request.env, root_path=root_path) as app:
2929
uid = request.env["fastapi.endpoint"].sudo().get_uid(root_path)
3030
data = BytesIO()
3131
with self._manage_odoo_env(uid):

fastapi/models/fastapi_endpoint.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
from .. import dependencies
1818
from ..middleware import ASGIMiddleware
19-
from ..pools import fastapi_app_pool
2019

2120
_logger = logging.getLogger(__name__)
2221

@@ -122,10 +121,10 @@ def _registered_endpoint_rule_keys(self):
122121
return tuple(res)
123122

124123
@api.model
125-
def _routing_impacting_fields(self) -> tuple[str]:
124+
def _routing_impacting_fields(self) -> Tuple[str, ...]:
126125
"""The list of fields requiring to refresh the mount point of the pp
127126
into odoo if modified"""
128-
return ("root_path",)
127+
return ("root_path", "save_http_session")
129128

130129
#
131130
# end of endpoint.route.sync.mixin methods implementation
@@ -200,7 +199,17 @@ def _endpoint_registry_route_unique_key(self, routing: dict[str, Any]):
200199
return f"{self._name}:{self.id}:{path}"
201200

202201
def _reset_app(self):
203-
fastapi_app_pool.invalidate(self.root_path, self.env)
202+
self._reset_app_cache_marker.clear_cache(self)
203+
204+
@tools.ormcache()
205+
def _reset_app_cache_marker(self):
206+
"""This methos is used to get a way to mark the orm cache as dirty
207+
when the app is reset. By marking the cache as dirty, the system
208+
will signal to others instances that the cache is not up to date
209+
and that they should invalidate their cache as well. This is required
210+
to ensure that any change requiring a reset of the app is propagated
211+
to all the running instances.
212+
"""
204213

205214
@api.model
206215
def get_app(self, root_path):

fastapi/pools/fastapi_app.py

Lines changed: 77 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Copyright 2025 ACSONE SA/NV
22
# License LGPL-3.0 or later (http://www.gnu.org/licenses/LGPL).
3-
3+
import logging
44
import queue
55
import threading
66
from collections import defaultdict
@@ -11,19 +11,65 @@
1111

1212
from fastapi import FastAPI
1313

14+
_logger = logging.getLogger(__name__)
15+
1416

1517
class FastApiAppPool:
18+
"""Pool of FastAPI apps.
19+
20+
This class manages a pool of FastAPI apps. The pool is organized by database name
21+
and root path. Each pool is a queue of FastAPI apps.
22+
23+
The pool is used to reuse FastAPI apps across multiple requests. This is useful
24+
to avoid the overhead of creating a new FastAPI app for each request. The pool
25+
ensures that only one request at a time uses an app.
26+
27+
The proper way to use the pool is to use the get_app method as a context manager.
28+
This ensures that the app is returned to the pool after the context manager exits.
29+
The get_app method is designed to ensure that the app made available to the
30+
caller is unique and not used by another caller at the same time.
31+
32+
.. code-block:: python
33+
34+
with fastapi_app_pool.get_app(env=request.env, root_path=root_path) as app:
35+
# use the app
36+
37+
The pool is invalidated when the cache registry is updated. This ensures that
38+
the pool is always up-to-date with the latest app configuration. It also
39+
ensures that the invalidation is done even in the case of a modification occurring
40+
in a different worker process or thread or server instance. This mechanism
41+
works because every time an attribute of the fastapi.endpoint model is modified
42+
and this attribute is part of the list returned by the `_fastapi_app_fields`,
43+
or `_routing_impacting_fields` methods, we reset the cache of a marker method
44+
`_reset_app_cache_marker`. As side effect, the cache registry is marked to be
45+
updated by the increment of the `cache_sequence` SQL sequence. This cache sequence
46+
on the registry is reloaded from the DB on each request made to a specific database.
47+
When an app is retrieved from the pool, we always compare the cache sequence of
48+
the pool with the cache sequence of the registry. If the two sequences are different,
49+
we invalidate the pool and save the new cache sequence on the pool.
50+
51+
The cache is based on a defaultdict of defaultdict of queue.Queue. We are cautious
52+
that the use of defaultdict is not thread-safe for operations that modify the
53+
dictionary. However the only operation that modifies the dictionary is the
54+
first access to a new key. If two threads access the same key at the same time,
55+
the two threads will create two different queues. This is not a problem since
56+
at the time of returning an app to the pool, we are sure that a queue exists
57+
for the key into the cache and all the created apps are returned to the same
58+
valid queue. And the end, the lack of thread-safety for the defaultdict could
59+
only lead to a negligible overhead of creating a new queue that will never be
60+
used. This is why we consider that the use of defaultdict is safe in this context.
61+
"""
62+
1663
def __init__(self):
1764
self._queue_by_db_by_root_path: dict[
1865
str, dict[str, queue.Queue[FastAPI]]
1966
] = defaultdict(lambda: defaultdict(queue.Queue))
67+
self.__cache_sequence = 0
2068
self._lock = threading.Lock()
2169

2270
def __get_pool(self, env: Environment, root_path: str) -> queue.Queue[FastAPI]:
2371
db_name = env.cr.dbname
24-
with self._lock:
25-
# default dict is not thread safe but the use
26-
return self._queue_by_db_by_root_path[db_name][root_path]
72+
return self._queue_by_db_by_root_path[db_name][root_path]
2773

2874
def __get_app(self, env: Environment, root_path: str) -> FastAPI:
2975
pool = self.__get_pool(env, root_path)
@@ -39,7 +85,7 @@ def __return_app(self, env: Environment, app: FastAPI, root_path: str) -> None:
3985

4086
@contextmanager
4187
def get_app(
42-
self, root_path: str, env: Environment
88+
self, env: Environment, root_path: str
4389
) -> Generator[FastAPI, None, None]:
4490
"""Return a FastAPI app to be used in a context manager.
4591
@@ -49,13 +95,36 @@ def get_app(
4995
When used into the FastApiDispatcher class this ensures that the app is reused
5096
across multiple requests but only one request at a time uses an app.
5197
"""
98+
self._check_cache(env)
5299
app = self.__get_app(env, root_path)
53100
try:
54101
yield app
55102
finally:
56103
self.__return_app(env, app, root_path)
57104

58-
def invalidate(self, root_path: str, env: Environment) -> None:
59-
with self._lock:
60-
db_name = env.cr.dbname
105+
@property
106+
def cache_sequence(self) -> int:
107+
return self.__cache_sequence
108+
109+
@cache_sequence.setter
110+
def cache_sequence(self, value: int) -> None:
111+
if value != self.__cache_sequence:
112+
with self._lock:
113+
self.__cache_sequence = value
114+
115+
def _check_cache(self, env: Environment) -> None:
116+
cache_sequence = env.registry.cache_sequence
117+
if cache_sequence != self.cache_sequence and self.cache_sequence != 0:
118+
_logger.info(
119+
"Cache registry updated, reset fastapi_app pool for the current "
120+
"database"
121+
)
122+
self.invalidate(env)
123+
self.cache_sequence = cache_sequence
124+
125+
def invalidate(self, env: Environment, root_path: str | None = None) -> None:
126+
db_name = env.cr.dbname
127+
if root_path:
61128
self._queue_by_db_by_root_path[db_name][root_path] = queue.Queue()
129+
elif db_name in self._queue_by_db_by_root_path:
130+
del self._queue_by_db_by_root_path[db_name]

0 commit comments

Comments
 (0)