Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

N+1 Query in user.member_request_edit #751

Open
agdsn-sentry bot opened this issue Sep 18, 2024 · 4 comments
Open

N+1 Query in user.member_request_edit #751

agdsn-sentry bot opened this issue Sep 18, 2024 · 4 comments
Labels
enhancement 🔨 lib Things relating to the core logic / functionality 🔨 web/ui Things relating to Flask routes and Jinja templates

Comments

@agdsn-sentry
Copy link

agdsn-sentry bot commented Sep 18, 2024

This causes 41(!!) lazy fetches on every request, adding up to ~120ms.

Sentry Issue: PYCROFT-9D

Offending Spans db - SELECT building.site_id AS building_site_i...
@agdsn-sentry agdsn-sentry bot added 🔨 lib Things relating to the core logic / functionality 🔨 web/ui Things relating to Flask routes and Jinja templates enhancement labels Sep 18, 2024
@lukasjuhrich
Copy link
Collaborator

I'm really not sure why this happens. We have precisely 41 buildings, so my guess would be that it's the QuerySelectField. However, this basically only uses a Building.q.all() – so why would sqla lazy-load happen that fetches every building given the building id?

Needs investigation.

@lukasjuhrich
Copy link
Collaborator

relevant frontend endpoint:

@bp.route("member-request/<int:pre_member_id>/edit", methods=['GET', 'POST'])
def member_request_edit(pre_member_id: int) -> ResponseReturnValue:
prm = get_pre_member_or_404(pre_member_id)
form = PreMemberEditForm(
name=prm.name,
login=prm.login,
building=prm.room.building if prm.room else None,
level=prm.room.level if prm.room else None,
room_number=prm.room.number if prm.room else None,
email=prm.email,
move_in_date=prm.move_in_date,
birthdate=prm.birthdate,
person_id=prm.swdd_person_id,
)
if form.validate_on_submit():
old_email = prm.email
prm.name = form.name.data
prm.login = form.login.data
prm.email = form.email.data
prm.move_in_date = form.move_in_date.data
prm.birthdate = form.birthdate.data
if prm.swdd_person_id != form.person_id.data:
if form.person_id.data is None:
prm.swdd_person_id = form.person_id.data
else:
tenancy = Tenancy.q.filter_by(person_id=form.person_id.data).first()
if tenancy is not None:
prm.swdd_person_id = form.person_id.data
else:
form.person_id.errors.append("Zu der angegebenen Debitorennummer konnten keine Verträge gefunden werden!",)
room = Room.q.filter_by(building=form.building.data, level=form.level.data,
number=form.room_number.data).first()
prm.room = room
session.session.commit()
if old_email != prm.email:
send_confirmation_email(prm)
flash("Es wurde eine neue Bestätigungsmail verschickt!", "warning")
session.session.commit()
flash("Änderungen wurden gespeichert.", "success")
return render_template("user/member_request_edit.html",
page_title="Mitgliedschaftsanfrage Bearbeiten",
form=form,
prm=prm,
is_adult=prm.is_adult)

@lukasjuhrich
Copy link
Collaborator

It seems that the slow transactions are the POST ones, GET goes through rather quickly.

@lukasjuhrich
Copy link
Collaborator

I think validation might be the culprit, specifically https://github.com/wtforms/wtforms-sqlalchemy/blob/09b3d4745ec98d6d8f769f6794bc217c63d81946/wtforms_sqlalchemy/fields.py#L184-L193 (or the building_query it calls).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🔨 lib Things relating to the core logic / functionality 🔨 web/ui Things relating to Flask routes and Jinja templates
Projects
None yet
Development

No branches or pull requests

1 participant