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

[Enhancement]: Optimize SQL Queries in SQLAlchemy to Eliminate N+1 Problem #1902

Merged
merged 11 commits into from
Aug 29, 2024

Conversation

aybruhm
Copy link
Member

@aybruhm aybruhm commented Jul 16, 2024

Description

This PR refactors SQL queries in the backend to enhance performance by resolving the N+1 query problem. The updates involve selectively loading only necessary fields and utilizing join-loading for related tables to reduce excessive queries.

Related Issue

Closes AGE-430

Changes

  • Optimized SQL queries in SQLAlchemy to load only essential fields.
  • Implemented join-loading for related tables to resolve the N+1 query problem.
  • Updated relevant backend functions to leverage the optimized queries.

What to QA

  • Evaluation Run:
    • Run an evaluation and ensure it completes successfully without triggering additional N+1 issues in Sentry.
    • Verify that evaluation results remain accurate and consistent across multiple runs.

Acceptance Tests

  1. Create Apps:
    • Create an app using the CLI AND from the UI.
  2. Create an App Variant from the Default Variant:
    • Create a new variant derived from the default variant.
  3. Run Automatic Evaluations:
    • Perform at least four (4) automatic evaluations using the created app.
  4. View and Compare Evaluation Results:
    • After running evaluations, review and compare the results across different runs.
  5. Delete Apps:
    • Delete the apps after completing the evaluations.

Note: The N+1 query issue is a common performance bottleneck in SQL databases. This PR addresses the identified N+1 issue observed during evaluations. If Sentry reports additional N+1 issues, please tag me in the notification.

…ion and evaluator config from DB in evaluate task function
…ssary fields and join-loading required table(s) to eliminate N+1 queries
Copy link

vercel bot commented Jul 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
agenta ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2024 4:07pm
agenta-documentation ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 23, 2024 4:07pm

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 16, 2024
@dosubot dosubot bot added Backend enhancement New feature or request labels Jul 16, 2024
Copy link
Contributor

@jp-agenta jp-agenta left a comment

Choose a reason for hiding this comment

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

See non-blocking comments.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 18, 2024
@mmabrouk
Copy link
Member

Thanks for the PR @aybruhm and for the review @jp-agenta

Just wanted to check, has this been throughly QA'ed, i.e. all the modified endpoints / actions have been smoke tested

@aybruhm aybruhm marked this pull request as draft August 16, 2024 14:15
@aybruhm aybruhm marked this pull request as draft August 16, 2024 14:15
Copy link
Contributor

@jp-agenta jp-agenta left a comment

Choose a reason for hiding this comment

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

Thanks @aybruhm !
More than code review, this one requires proper (automated) QA -- @aybruhm @mmabrouk @aakrem

@aybruhm
Copy link
Member Author

aybruhm commented Aug 29, 2024

@aybruhm aybruhm merged commit c77beb0 into main Aug 29, 2024
10 checks passed
@aybruhm aybruhm deleted the enhancement/n+1_query-resolve branch August 29, 2024 08:23
@zenUnicorn
Copy link
Contributor

QA done and everything works fine!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backend enhancement New feature or request lgtm This PR has been approved by a maintainer refactoring size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants