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

Resources Accessed view times out and does not load the bar chart in some courses #1466

Closed
jennlove-um opened this issue Dec 21, 2022 · 7 comments · Fixed by #1488
Closed
Assignees
Labels

Comments

@jennlove-um
Copy link
Contributor

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

Resources Accessed view loads a bar chart showing the percent of students who viewed resources during the selected timeframe.

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

For certain courses, the view does not load the bar chart. Developer tools shows a 504 gateway timeout.

Steps to Reproduce :

  1. Go to MyLA for course 556396
  2. Click on Resources Accessed
  3. The bar chart does not load and a 504 shows up in the developer tools

Additional context (Add any other context about the problem here) :

This is a large course with 400+ students in it.

@jennlove-um jennlove-um added this to To do in MyLA-2023.01.01 via automation Dec 21, 2022
@jonespm
Copy link
Member

jonespm commented Dec 22, 2022

It looks like this site has nearly 400 resources that are loaded.
image

This view gets slow in the back and front with that many resources. Many of these have low <10% accesssed.

I think the only way we'll improve this would be

  • Change the view to include another filter, for instance "Show up to 50 most accessed" or "Show up to 50 least accessed". This might be able to go to 200, we'd have to test it. But really anything over 25 isn't possible to read.
    • This would apply to the other filter options such as start and end weeks and types
    • Maybe it should also say somewhere "X resources were not shown?"
  • Redesign this view to somehow be able to handle more resources.

This may all need some work. I don't know if there's a quick fix other than putting a hardcoded limit with no user option.

@jennlove-um
Copy link
Contributor Author

Time out has been increased in beta and test and is ready for testing.

This works in test. The bar chart loads.

@jennlove-um
Copy link
Contributor Author

For the next release, limit the number of files that display to 100. Make this configurable for admins.

Add the ability to filter resources I've viewed and resources I have not viewed.

@jennlove-um jennlove-um removed this from To do in MyLA-2023.01.01 Jan 12, 2023
@jennlove-um jennlove-um added this to To do in MyLA-2023.01.02 via automation Jan 12, 2023
@jonespm
Copy link
Member

jonespm commented Mar 3, 2023

So this is proving to be a little tricky because the calculations for percent and total_percent as well as the filter are done in the code rather than in the SQL.

I've so far for this course been able to reduce it from about 30 seconds to 10 on my local PC if I sort and cut at the end, which is going to be slower on the server. I'm not sure how to fix this much without a rewrite to move this logic into the SQL.

And depending on where I take the head, the total_percent is changing. I'm not sure why yet but still trying to figure this out. If I do the limit at least earlier it's faster but the values are different.

I guess this speed improvement is better than nothing but perhaps it's possible for this percent calculation to be in the SQL.

@jonespm
Copy link
Member

jonespm commented Mar 3, 2023

I kind of feel like this might be the best we can easily do here, just let it do all the computations and filter at the very end. This at least does save a lot of time on the client and makes it seem like it's faster. This is where most of the slowdown is.

I think optimizing the client and server are separate independent problems.

Nevermind, it looks like the slowdown locally might have just because I had debugging on. This seems pretty fast on the server, it's all just slow on the client. I'm going to remove the debug so it's not accidentally run.

@jonespm
Copy link
Member

jonespm commented Mar 3, 2023

Test Plan:

  • Load data for a course with a lot of resources like 556396
  • Go to resources accessed in that view, by default it should have a message that it was limited to 100
  • Go to Admin -> Constances (Config) and increase the limit to like 1000
  • Go back to this course it should now be unlimited

Verify that there are more results in the unlimited case
Verify that the results are similar to the values in production before the patch

jonespm added a commit to jonespm/student-dashboard-django that referenced this issue Mar 3, 2023
jonespm added a commit to jonespm/student-dashboard-django that referenced this issue Mar 3, 2023
@jonespm jonespm moved this from To do to In progress in MyLA-2023.01.02 Mar 6, 2023
@ssciolla ssciolla added the 🪳 bug Something isn't working label Mar 9, 2023
MyLA-2023.01.02 automation moved this from In progress to Review/QA Mar 10, 2023
jonespm added a commit that referenced this issue Mar 10, 2023
)

Co-authored-by: Sam Sciolla <35741256+ssciolla@users.noreply.github.com>
@jennlove-um
Copy link
Contributor Author

This looks good in testing. I can change the default for how many resources display for courses. The message on the page also displays correctly depending on whether there is limit or all resources are displaying.

@jennlove-um jennlove-um moved this from Review/QA to Review/QA - DEV in MyLA-2023.01.02 Mar 29, 2023
@jennlove-um jennlove-um moved this from Review/QA - DEV to Done in MyLA-2023.01.02 Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants