fix(FastAPI): voted/notified query efficiency

Previously, we were running a single ORM query for every single package
to check for its voted or notified states. Now, we perform a single
ORM query for each of the set of voted or notified packages in
relation with the request user.

This improves performance drastically at the expense of some
manual code additions and set-dependency; i.e. we add a bit
more complexity and roundabout way of getting our data.

Closes: https://gitlab.archlinux.org/archlinux/aurweb/-/issues/102

Signed-off-by: Kevin Morris <kevr@0cost.org>
This commit is contained in:
Kevin Morris 2021-09-18 23:30:51 -07:00
parent fd9b07c429
commit 4de18d8134
No known key found for this signature in database
GPG key ID: F7E46DED420788F3
5 changed files with 126 additions and 11 deletions

View file

@ -1,5 +1,6 @@
from collections import defaultdict
from http import HTTPStatus from http import HTTPStatus
from typing import List from typing import Dict, List
import orjson import orjson
@ -11,8 +12,11 @@ from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider
from aurweb.models.package import Package from aurweb.models.package import Package
from aurweb.models.package_base import PackageBase from aurweb.models.package_base import PackageBase
from aurweb.models.package_dependency import PackageDependency from aurweb.models.package_dependency import PackageDependency
from aurweb.models.package_notification import PackageNotification
from aurweb.models.package_relation import PackageRelation from aurweb.models.package_relation import PackageRelation
from aurweb.models.package_vote import PackageVote
from aurweb.models.relation_type import PROVIDES_ID, RelationType from aurweb.models.relation_type import PROVIDES_ID, RelationType
from aurweb.models.user import User
from aurweb.redis import redis_connection from aurweb.redis import redis_connection
from aurweb.templates import register_filter from aurweb.templates import register_filter
@ -164,3 +168,47 @@ def updated_packages(limit: int = 0, cache_ttl: int = 600) -> List[Package]:
# Return the deserialized list of packages. # Return the deserialized list of packages.
return packages return packages
def query_voted(query: List[Package], user: User) -> Dict[int, bool]:
""" Produce a dictionary of package base ID keys to boolean values,
which indicate whether or not the package base has a vote record
related to user.
:param query: A collection of Package models
:param user: The user that is being notified or not
:return: Vote state dict (PackageBase.ID: int -> bool)
"""
output = defaultdict(bool)
query_set = {pkg.PackageBase.ID for pkg in query}
voted = db.query(PackageVote).join(
PackageBase,
PackageBase.ID.in_(query_set)
).filter(
PackageVote.UsersID == user.ID
)
for vote in voted:
output[vote.PackageBase.ID] = True
return output
def query_notified(query: List[Package], user: User) -> Dict[int, bool]:
""" Produce a dictionary of package base ID keys to boolean values,
which indicate whether or not the package base has a notification
record related to user.
:param query: A collection of Package models
:param user: The user that is being notified or not
:return: Notification state dict (PackageBase.ID: int -> bool)
"""
output = defaultdict(bool)
query_set = {pkg.PackageBase.ID for pkg in query}
notified = db.query(PackageNotification).join(
PackageBase,
PackageBase.ID.in_(query_set)
).filter(
PackageNotification.UserID == user.ID
)
for notify in notified:
output[notify.PackageBase.ID] = True
return output

View file

@ -19,7 +19,7 @@ from aurweb.models.package_base import PackageBase
from aurweb.models.package_comaintainer import PackageComaintainer from aurweb.models.package_comaintainer import PackageComaintainer
from aurweb.models.package_request import PackageRequest from aurweb.models.package_request import PackageRequest
from aurweb.models.user import User from aurweb.models.user import User
from aurweb.packages.util import updated_packages from aurweb.packages.util import query_notified, query_voted, updated_packages
from aurweb.templates import make_context, render_template from aurweb.templates import make_context, render_template
router = APIRouter() router = APIRouter()
@ -145,14 +145,25 @@ async def index(request: Request):
PackageBase.MaintainerUID == request.user.ID PackageBase.MaintainerUID == request.user.ID
) )
# Packages maintained by the user that have been flagged.
context["flagged_packages"] = maintained.filter( context["flagged_packages"] = maintained.filter(
PackageBase.OutOfDateTS.isnot(None) PackageBase.OutOfDateTS.isnot(None)
).order_by( ).order_by(
PackageBase.ModifiedTS.desc(), Package.Name.asc() PackageBase.ModifiedTS.desc(), Package.Name.asc()
).limit(50).all() ).limit(50).all()
# Flagged packages that request.user has voted for.
context["flagged_packages_voted"] = query_voted(
context.get("flagged_packages"), request.user)
# Flagged packages that request.user is being notified about.
context["flagged_packages_notified"] = query_notified(
context.get("flagged_packages"), request.user)
archive_time = aurweb.config.getint('options', 'request_archive_time') archive_time = aurweb.config.getint('options', 'request_archive_time')
start = now - archive_time start = now - archive_time
# Package requests created by request.user.
context["package_requests"] = request.user.package_requests.filter( context["package_requests"] = request.user.package_requests.filter(
PackageRequest.RequestTS >= start PackageRequest.RequestTS >= start
).limit(50).all() ).limit(50).all()
@ -162,13 +173,31 @@ async def index(request: Request):
PackageBase.ModifiedTS.desc(), Package.Name.desc() PackageBase.ModifiedTS.desc(), Package.Name.desc()
).limit(50).all() ).limit(50).all()
# Packages that request.user has voted for.
context["packages_voted"] = query_voted(
context.get("packages"), request.user)
# Packages that request.user is being notified about.
context["packages_notified"] = query_notified(
context.get("packages"), request.user)
# Any packages that the request user comaintains. # Any packages that the request user comaintains.
context["comaintained"] = packages.join( context["comaintained"] = packages.join(
PackageComaintainer).filter( PackageComaintainer
PackageComaintainer.UsersID == request.user.ID).order_by( ).filter(
PackageComaintainer.UsersID == request.user.ID
).order_by(
PackageBase.ModifiedTS.desc(), Package.Name.desc() PackageBase.ModifiedTS.desc(), Package.Name.desc()
).limit(50).all() ).limit(50).all()
# Comaintained packages that request.user has voted for.
context["comaintained_voted"] = query_voted(
context.get("comaintained"), request.user)
# Comaintained packages that request.user is being notified about.
context["comaintained_notified"] = query_notified(
context.get("comaintained"), request.user)
return render_template(request, "index.html", context) return render_template(request, "index.html", context)

View file

@ -5,7 +5,11 @@
{% if not flagged_packages %} {% if not flagged_packages %}
<p>{{ "No packages matched your search criteria." | tr }}</p> <p>{{ "No packages matched your search criteria." | tr }}</p>
{% else %} {% else %}
{% with table_id = "flagged-packages", packages = flagged_packages %} {% with table_id = "flagged-packages",
packages = flagged_packages,
votes = flagged_packages_voted,
notified = flagged_packages_notified
%}
{% include 'partials/packages/results.html' %} {% include 'partials/packages/results.html' %}
{% endwith %} {% endwith %}
{% endif %} {% endif %}
@ -30,7 +34,10 @@
{% if not packages %} {% if not packages %}
<p>{{ "No packages matched your search criteria." | tr }}</p> <p>{{ "No packages matched your search criteria." | tr }}</p>
{% else %} {% else %}
{% with table_id = "my-packages" %} {% with table_id = "my-packages",
votes = packages_voted,
notified = packages_notified
%}
{% include 'partials/packages/results.html' %} {% include 'partials/packages/results.html' %}
{% endwith %} {% endwith %}
{% endif %} {% endif %}
@ -46,7 +53,11 @@
{% if not comaintained %} {% if not comaintained %}
<p>{{ "No packages matched your search criteria." | tr }}</p> <p>{{ "No packages matched your search criteria." | tr }}</p>
{% else %} {% else %}
{% with table_id = "comaintained-packages", packages = comaintained %} {% with table_id = "comaintained-packages",
packages = comaintained,
votes = comaintained_voted,
notified = comaintained_notified
%}
{% include 'partials/packages/results.html' %} {% include 'partials/packages/results.html' %}
{% endwith %} {% endwith %}
{% endif %} {% endif %}

View file

@ -28,14 +28,14 @@
<td>{{ pkg.PackageBase.Popularity | number_format(2) }}</td> <td>{{ pkg.PackageBase.Popularity | number_format(2) }}</td>
{% if request.user.is_authenticated() %} {% if request.user.is_authenticated() %}
<td> <td>
<!-- If I voted, display "Yes". --> {# If I voted, display "Yes". #}
{% if request.user.voted_for(pkg) %} {% if pkg.PackageBase.ID in votes %}
{{ "Yes" | tr }} {{ "Yes" | tr }}
{% endif %} {% endif %}
</td> </td>
<td> <td>
<!-- If I'm being notified, display "Yes". --> {# If I'm being notified, display "Yes". #}
{% if request.user.notified(pkg) %} {% if pkg.PackageBase.ID in notified %}
{{ "Yes" | tr }} {{ "Yes" | tr }}
{% endif %} {% endif %}
</td> </td>

View file

@ -1,3 +1,5 @@
from datetime import datetime
import pytest import pytest
from fastapi.testclient import TestClient from fastapi.testclient import TestClient
@ -7,6 +9,8 @@ from aurweb.models.account_type import USER_ID, AccountType
from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider from aurweb.models.official_provider import OFFICIAL_BASE, OfficialProvider
from aurweb.models.package import Package from aurweb.models.package import Package
from aurweb.models.package_base import PackageBase from aurweb.models.package_base import PackageBase
from aurweb.models.package_notification import PackageNotification
from aurweb.models.package_vote import PackageVote
from aurweb.models.user import User from aurweb.models.user import User
from aurweb.packages import util from aurweb.packages import util
from aurweb.redis import kill_redis from aurweb.redis import kill_redis
@ -19,6 +23,8 @@ def setup():
User.__tablename__, User.__tablename__,
Package.__tablename__, Package.__tablename__,
PackageBase.__tablename__, PackageBase.__tablename__,
PackageVote.__tablename__,
PackageNotification.__tablename__,
OfficialProvider.__tablename__ OfficialProvider.__tablename__
) )
@ -71,3 +77,24 @@ def test_updated_packages(maintainer: User, package: Package):
assert util.updated_packages(1, 0) == [expected] assert util.updated_packages(1, 0) == [expected]
assert util.updated_packages(1, 600) == [expected] assert util.updated_packages(1, 600) == [expected]
kill_redis() # Kill it again, in case other tests use a real instance. kill_redis() # Kill it again, in case other tests use a real instance.
def test_query_voted(maintainer: User, package: Package):
now = int(datetime.utcnow().timestamp())
with db.begin():
db.create(PackageVote, User=maintainer, VoteTS=now,
PackageBase=package.PackageBase)
query = db.query(Package).filter(Package.ID == package.ID).all()
query_voted = util.query_voted(query, maintainer)
assert query_voted[package.PackageBase.ID]
def test_query_notified(maintainer: User, package: Package):
with db.begin():
db.create(PackageNotification, User=maintainer,
PackageBase=package.PackageBase)
query = db.query(Package).filter(Package.ID == package.ID).all()
query_notified = util.query_notified(query, maintainer)
assert query_notified[package.PackageBase.ID]