Cache the results of is_admin_or_superuser_check #146
Conversation
bmtcril
force-pushed
the
bmtcril/cache_superuser_check
branch
from
934807f to
d8a03a4
Compare
| if auto_load_policy_interval > 0: | ||
| cls.configure_enforcer_auto_loading(auto_load_policy_interval) | ||
| else: | ||
| logger.warning("CASBIN_AUTO_LOAD_POLICY_INTERVAL is not set or zero; auto-load is disabled.") |
There was a problem hiding this comment.
This warning was firing several times in a request, I don't think we need it anymore?
|
|
||
| compile-requirements: ## compile the requirements/*.txt files with the latest packages satisfying requirements/*.in | ||
| pip install -qr requirements/pip-tools.txt | ||
| pip install -qr requirements/pip.txt |
There was a problem hiding this comment.
This is the fix we've been using elsewhere to deal with the current pip-tools compatibility issues with pip.
| -c https://raw.githubusercontent.com/edx/edx-lint/master/edx_lint/files/common_constraints.txt | ||
|
|
||
| # Different packages want different versions of click, we force the most compatible one here | ||
| click==8.3.0 |
There was a problem hiding this comment.
This is another pip-tools issue that was blocking make upgrade . pip-tools wanted 8.3.1, everything else wanted 8.3.0 so I've pinned it here for now.
bmtcril
force-pushed
the
bmtcril/cache_superuser_check
branch
from
dcf6472 to
3611811
Compare
mariajgrimaldi
left a comment
mariajgrimaldi
left a comment
There was a problem hiding this comment.
LGTM! Thank you so much for working on this :)
bmtcril
changed the title
is_admin_or_superuser_check is being called once per policy when checking enforcement, creating a potential performance issue with numerous calls to the database. This adds a brief cache to offload some of the burden, but we will need a better fix long term.
By using the RequestCache instead of the Django cache we are able to have a thread-local memory copy of the user's superuser / staff state that exists only for the length of the request. This will save a large number of round trips to the cache backend.
bmtcril
force-pushed
the
bmtcril/cache_superuser_check
branch
from
3611811 to
a8350d6
Compare
|
This has been tested on the dev sandbox and provides a substantial performance improvement, no errors were noted. Mean response time under load for the validate/me call went from ~470ms to ~109ms. Auth User selects went from 171 to 2. There is room for future improvement by reducing the columns selected to just |
The
is_admin_or_superuser_checkis being called once per policy when checking enforcement, creating a potential performance issue with numerous calls to the database. This adds a RequestCache to use an in-thread memory cache for these repeated calls.Also fixes an ongoing issue with requirements upgrades.
Merge checklist:
Check off if complete or not applicable: