MyFinances
MyFinances copied to clipboard
TreyWW
performance bug: Causing extra user queries
What version of MyFinances are you currently using?
latest (main)
What device type are you currently facing the issue on?
Desktop
Describe the bug
At the moment we're using 2 queries for each request due to our custom user manager
I think the fix will be to get rid of CustomUserManager and access the required data where needed. It's not a small refactoring...
class CustomUserManager(UserManager):
def get_queryset(self):
return (
super()
.get_queryset()
.select_related("user_profile", "logged_in_as_team")
.annotate(notification_count=((Count("user_notifications"))))
)
The reason I added that to the usermanager is because that data is needed for EVERY page as it's in the topbar. I feel like that'll cause more issues taking it away (we'd just have to end up moving it to a middleware or something anyways). I believe we're using a CustomUserManager aswell as the default one, so we need to just get rid of the original one maybe? I'm not too sure, we can probably see more by using the DebugToolbar and going into each request individually (i could check this later)
@TreyWW hmm basically it's all caused by CustomUserMiddleware
if request.user.is_authenticated:
request.user = User.objects.get(pk=request.user.pk)
is causing 2nd query because AuthenticationMiddleware is already handling that.
Removing "backend.models.CustomUserMiddleware" from MIDDLEWARE in settings.py and from models.py seams to fix the issue with double queries. I just noticed strange behavior while creating receipts, at some point creation window after hitting Save haven't disappeared and you had to click somewhere outside the window. At the moment I haven't been able to reproduce that bug again so I don't know what causing it.
@TreyWW hmm basically it's all caused by
CustomUserMiddlewareif request.user.is_authenticated: request.user = User.objects.get(pk=request.user.pk)is causing 2nd query because
AuthenticationMiddlewareis already handling that.
Yeah that's the part I'm aware of, that's the good query, I want Django to use that one only though - rather than doing its own somewhere else
What's the reason for that ? Both of them have the same query as far as I remember (can't check it now) and from what I've checked functionality is the same with are without it, user name is displayed as it should be etc.
If we would want to keep it I guess we need to find a way then to get rid off AuthenticationMiddleware without breaking functionality.
Our one also prefetches things to do with profile info. It makes sense to do it all in one query rather than a second query
@TreyWW
I'm thinking of removing then "django.contrib.auth.middleware.AuthenticationMiddleware" from middleware in settings/settings.py and replacing it with "backend.middleware.CustomUserMiddleware". Modifying CustomUserMiddleware like this (in this form query take around 0.10ms):
class CustomUserMiddleware(MiddlewareMixin):
def process_request(self, request):
user = get_user(request)
# Replace request.user with CustomUser instance if authenticated
if user.is_authenticated:
request.user = User.objects.get(pk=user.pk)
else:
# If user is not authenticated, set request.user to AnonymousUser
request.user = AnonymousUser()
and moving it to backend/middleware.py. As I tested everything on the website works fine and we don't query User every single time when changing a tab (not to mention double query) we just use data stored in session.