-
Notifications
You must be signed in to change notification settings - Fork 57
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 caching for dashboard stats #3263
Conversation
This comment has been minimized.
This comment has been minimized.
This pull request fixes 2 alerts when merging d356245 into 26d16c2 - view on LGTM.com fixed alerts:
|
I've also added some fixes to a problem that was recently discovered on Yale's site. Basically, we had the DerivedField dependent on ALL records, which meant we were updating the field for ALL sections for ALL programs every time any record was added. On a development server, this isn't a huge deal because we don't have that many sections, but on a production server (especially for an older chapter like Yale), it results in thousands of calculations which causes each record to take nearly a minute to save. The only real solution seems to be to remove the DerivedField entirely (RIP). We only used it in the jsondatamodule for calculating dashboard stats, which is now cached better in this PR, so now we just do the calculations in a for loop there. I tested the dashboard and the grid-based class changes pages and both seem to update checked-in numbers properly. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spectacular! Who knew the dashboard could be up to date!
* Initial docs for stable release 13 * Docs for #3116, #3117, and #3118 * Added docs about django upgrade * Docs for #3128 * Docs for #3129, #3133, #3134, and #3137 * Docs for #3156 and #3153 * Docs for #3174, #3163, and #3184 * Docs for #3139, #3140, and #3141 * Docs for #3143, #3150, #3154, #3160, #3162, and #3168 * Docs for #3171, #3185, #3186, and #3188 * Docs for #3131 and #3189 * Docs for #3149 and #3190 * Docs for #3193, #3194, #3195, #196, and #3197 * Clarification * Docs for #3192, #3201, #3209, and #2248 * Docs for #3204, #3212, #3214, #3205, 9fd073c, and #3226 * Docs for #3232, de5861c, #3231, #3233, #3234, #3237, #3238, and #3239 * Fix indent * Docs for #3227 and #3235 * Add missing word * spelling * Docs for e57581f, #3255, #3253, #3257, and #3249 * Docs for #3254, #3260, and #3262 * Docs for #3263, #3272, #3240, #3264, #3266, and #3270 * clarifications * Docs for #3283 and #3252 * Docs for #3288 and misc commits * Docs for #3292, #3311, #3286, #3289, and #3279 * Docs for a377f0d; move note * Docs for #3315, #3290, and #3322 * Docs for #3273 and #3317 * Final edits
The dashboard has notoriously been bad at showing up-to-date stats for a program. This is because the caching dependencies were never updated as more and more stats were added, meaning it would only update for a small number of database changes (or if the cache was flushed). Now, each component of the dashboard stats has its own cache dependencies, which should both speed up stat calculations overall and make sure the stats are always up-to-date. This should also help encourage similar cache dependency additions when new stats are added in the future.
For the reviewer, I've tested most of the dependencies on my dev server, but it would be good to have someone else do a second check and also to visually check if I've included all necessary cache dependency for each stat category. I didn't have every module enabled, so it's entirely possible I missed an important dependency for some list of teachers or students.
Fixes #827 and fixes #2082.