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 SQL performance issue on the student reports page #6134

Merged
merged 1 commit into from
Nov 24, 2022

Conversation

m1r0
Copy link
Member

@m1r0 m1r0 commented Nov 17, 2022

Fixes #6122

Changes proposed in this Pull Request

The fix hints to the SQL optimizer to use the sensei_comment_type_user_id index. This is the default behavior on newer versions of MySQL/MariaDB but older versions use the woo_idx_comment_type index which leads to major performance issues.

The problem was spotted on a website using MariaDB 10.4.25.

Testing instructions

  • Make sure the Reports -> Students screen works as before and there are no performance issues.

The lovely folks at Team 51 confirmed that this fixes the issue on their client's site. For more info, check this thread below.

Context

p1668540147192489-slack-C02P7FHLVR9

The fix hints to the SQL optimizer to use the "sensei_comment_type_user_id" index. This is needed on some older MySQL/MariaDB versions. The issue was spotted on MariaDB 10.4.25.
@m1r0 m1r0 requested a review from a team November 17, 2022 17:51
@m1r0 m1r0 self-assigned this Nov 17, 2022
@codecov
Copy link

codecov bot commented Nov 17, 2022

Codecov Report

Merging #6134 (dd354a6) into trunk (fa8bbe2) will increase coverage by 0.82%.
The diff coverage is 78.04%.

Impacted file tree graph

@@             Coverage Diff              @@
##              trunk    #6134      +/-   ##
============================================
+ Coverage     44.97%   45.80%   +0.82%     
- Complexity     9455     9472      +17     
============================================
  Files           476      476              
  Lines         33483    33543      +60     
  Branches        272      272              
============================================
+ Hits          15059    15363     +304     
+ Misses        18222    17978     -244     
  Partials        202      202              
Impacted Files Coverage Δ
includes/class-sensei-posttypes.php 11.13% <ø> (+0.04%) ⬆️
includes/class-sensei-question.php 12.02% <0.00%> (-0.08%) ⬇️
...sensei-reports-overview-data-provider-students.php 80.76% <ø> (ø)
...t-api/class-sensei-rest-api-lessons-controller.php 7.69% <ø> (-2.61%) ⬇️
includes/class-sensei-lesson.php 37.11% <50.00%> (ø)
includes/class-sensei-modules.php 38.23% <86.20%> (+2.63%) ⬆️
includes/class-sensei-quiz.php 60.45% <100.00%> (+0.07%) ⬆️
includes/class-sensei-usage-tracking.php 25.00% <100.00%> (-0.75%) ⬇️
...api/class-sensei-rest-api-questions-controller.php 84.15% <100.00%> (ø)
.../class-sensei-learners-admin-bulk-actions-view.php 54.28% <0.00%> (-0.26%) ⬇️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ada5ee6...dd354a6. Read the comment docs.

@m1r0 m1r0 added this to the 4.8.2 milestone Nov 17, 2022
@m1r0 m1r0 changed the title Fix a performance issue with an SQL query on the student reports page Fix SQL performance issue on the student reports page Nov 24, 2022
@m1r0 m1r0 merged commit 5503ef6 into trunk Nov 24, 2022
@m1r0 m1r0 deleted the fix/student-reports-performance-issue branch November 24, 2022 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The students report screen timeouts on sites with many users
2 participants