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

Fix consecutive db queries #1364

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Fix consecutive db queries #1364

merged 1 commit into from
Jul 12, 2023

Conversation

actlikewill
Copy link
Contributor

@actlikewill actlikewill commented Jul 7, 2023

  • Some of the consecutive queries come from the taxonomies. This attempts to reduce them by getting using the get_root_nodes function.

fixes #1347

@actlikewill actlikewill changed the title Reduce db queries for taxonomies Fix db queries Jul 7, 2023
@actlikewill actlikewill changed the title Fix db queries Fix consecutive db queries Jul 7, 2023
@longhotsummer
Copy link
Contributor

Does this load them more efficiently?

@actlikewill
Copy link
Contributor Author

@longhotsummer Yes. get_tree() loads the entire tree which recursively calls the db for each node, get_root_nodes() uses depth=1. It saves some recursive database queries in the view function. The worse issue was calling get_children twice in the template. I've removed one of them using get_children_count

@actlikewill
Copy link
Contributor Author

@longhotsummer I'm checking the code and I can't find where its actually doing a recursive call. But checking query count shows that using get_root_nodes uses 2 fewer queries that get_tree 🤷

@actlikewill
Copy link
Contributor Author

@longhotsummer another tricky bit is this here:

@register.simple_tag
def build_taxonomy_url(item, prefix="taxonomy"):
    items = []
    root = item.get_root()
    if root != item:
        items.append(root.slug)
    items.append(item.slug)
    return f"/{prefix}/" + "/".join(items)


Calling item.get_root is causing that N+1 issue, though I dont see a way around it. We need to get the root slug to build the url properly

Copy link
Contributor

@longhotsummer longhotsummer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, let's see what the impact is

@actlikewill
Copy link
Contributor Author

@longhotsummer Looking through Sentry docs on consecutive db queries, it says they are caused by several db queries that are called in sequence which may not necessarily be related. Sentry says that they can be called in parallel to save on time. I think this happens because some queries trigger in the context_processor (loading settings), some happen in the view layer and some can happen in the template. I'm not sure how to fix some of them.

Sentry is also flagging many slow queries especially on the judgment listings. The recommended solutions are caching the data and adding more indexed columns. I haven't done that here but I'll keep investigating.

@actlikewill actlikewill marked this pull request as ready for review July 12, 2023 04:41
@actlikewill actlikewill merged commit 8024853 into main Jul 12, 2023
9 checks passed
@actlikewill actlikewill deleted the taxonomy-queries branch July 12, 2023 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consecutive DB Queries
2 participants