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

in cron job, use batch select for artifacts across all courses, instead of doing one course at a time #827

Closed
zqian opened this issue Dec 12, 2019 · 2 comments · Fixed by #1423

Comments

@zqian
Copy link
Member

zqian commented Dec 12, 2019

Expected behavior (A clear and concise description of what you expected to happen) :

There is still room for improvement for database select in cron.py

Describe the bug (Tell us what happens instead of the expected behavior) :

It looks like the util_function for executing database query is called for each course separately, for various objects.

For example, https://github.com/tl-its-umich-edu/my-learning-analytics/blob/master/dashboard/cron.py line 370 is executed for every course to get the course assignment data.

We could do a batch select for all registered courses

@zqian zqian added the 🪳 bug Something isn't working label Dec 12, 2019
@zqian zqian self-assigned this Dec 12, 2019
@jonespm jonespm added this to To do in MyLA-Default-Project Mar 10, 2020
@jonespm
Copy link
Member

jonespm commented Sep 2, 2022

The objects that this could be improved in are

However looks like the only slow process in the cron currently is the update_submission process which currently takes 13 minutes (more than half of the time of a typical 23 minute run run). That might be the only part to focus on. Though this seems like low priority.

  • assignment and co.lms_int_id = '{course_id}'
  • assignment_groups and co.lms_int_id = '{course_id}'
  • assignment_weight and co_km.lms_int_id = '{course_id}'
  • course WHERE co2.lms_int_id = '{course_id}'
  • submission co.lms_int_id = '{course_id}'
  • user co.lms_int_id ='{course_id}'

It's possible changing some of these would require more processing in the backend job.

It looks like the resource query is already does this efficiently with and co_km.lms_int_id in %(course_ids)s

zqian added a commit to zqian/my-learning-analytics that referenced this issue Sep 6, 2022
…mode, instead of retrieve info course by course
@jonespm jonespm linked a pull request Sep 6, 2022 that will close this issue
@jonespm jonespm added this to In progress in MyLA-2022.02.01 Sep 7, 2022
MyLA-Default-Project automation moved this from To do to Review/QA Sep 9, 2022
MyLA-2022.02.01 automation moved this from In progress to Review/QA Sep 9, 2022
zqian added a commit that referenced this issue Sep 9, 2022
Co-authored-by: Code Hugger (Matthew Jones) <jonespm@umich.edu>
@jennlove-um jennlove-um moved this from Review/QA to Review/QA - DEV in MyLA-2022.02.01 Sep 19, 2022
@pushyamig
Copy link
Contributor

I will QA this

@jennlove-um jennlove-um moved this from Review/QA - DEV to Done in MyLA-2022.02.01 Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
MyLA-Default-Project
  
Review/QA
Development

Successfully merging a pull request may close this issue.

4 participants