Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-66782

Export function adds extra DB call to forum/lib, impacting performance.

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Deferred
    • Affects Version/s: 3.8
    • Fix Version/s: None
    • Component/s: Forum
    • Labels:
    • Testing Instructions:
      Hide
      Setup
      • You need a course with at least a teacher and a student.
      • A forum activity (no discussions or posts needed, we just need to test the Export link visibility
      Regression test
      1. Log in as admin.
      2. Go to Site administration > Users > Permissions > Capability overview
      3. Search for exportforum and click Get the overview button.
        • Make sure that capability is only allowed to Manager, Teacher and Non-editing teacher.
      4. Please note Student role is marked as Not set, click on the Student role link to edit that role.
      5. In the role edit page, click Edit button.
      6. Again, search for mod/forum:exportforum and check the Allow checkbox and click Save changes
      7. On a different window log in as student and go to the forum.
      8. Click on the cog.
      9. Make sure the Export option is now available for the student.
      10. Click on the Export link.
      11. Make sure the Export page was rendered.
      12. Click on Export button.
      13. Make sure the forum content gets exported.
      14. On window 2 as admin, edit student role again.
      15. Unassign that capability mod/forum:exportforum from student role and save.
      16. On window 1, refresh the page.
      17. Make sure you get an error: You cannot export this forum
      Performance
      1. Clone a new Moodle instance, but don't install it yet.
      2. Checkout the following hash : c3122dfcf57d55
      3. Install the site
      4. Create a forum and a few discussions in it
      5. Now, view the forum (you should see the list of discussions)
      6. In the footer, check "DB reads/writes:", in particular the reads.
      7. Refresh the page a few times and the reads with stabilise to a number, remember this number.
      8. Now, checkout master

        git checkout master
        

      9. Upgrade the site
      10. View the forum again
      11. Again, refresh the page a few times until the read number stabilises, and remember this number.
      12. Verify that the second read number (after upgrade - the one from the step above) is less than the first number, by 1.

       git checkout c3122dfcf57d55

      Show
      Setup You need a course with at least a teacher and a student. A forum activity (no discussions or posts needed, we just need to test the Export link visibility Regression test Log in as admin . Go to Site administration > Users > Permissions > Capability overview Search for exportforum and click Get the overview button. Make sure that capability is only allowed to Manager , Teacher and Non-editing teacher . Please note Student role is marked as Not set , click on the Student role link to edit that role. In the role edit page, click Edit button. Again, search for mod/forum:exportforum and check the Allow checkbox and click Save changes On a different window log in as student and go to the forum. Click on the cog . Make sure the Export option is now available for the student. Click on the Export link. Make sure the Export page was rendered. Click on Export button. Make sure the forum content gets exported. On window 2 as admin, edit student role again. Unassign that capability mod/forum:exportforum from student role and save. On window 1 , refresh the page. Make sure you get an error: You cannot export this forum Performance Clone a new Moodle instance, but don't install it yet. Checkout the following hash : c3122dfcf57d55 Install the site Create a forum and a few discussions in it Now, view the forum (you should see the list of discussions) In the footer, check "DB reads/writes:", in particular the reads. Refresh the page a few times and the reads with stabilise to a number, remember this number. Now, checkout master git checkout master Upgrade the site View the forum again Again, refresh the page a few times until the read number stabilises, and remember this number. Verify that the second read number (after upgrade - the one from the step above) is less than the first number, by 1. git checkout c3122dfcf57d55
    • Affected Branches:
      MOODLE_38_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-66782-master

      Description

      MDL-66075 added the export feature, controlled by a cap.

      The changes to lib here:
      https://github.com/lameze/moodle/commit/75af28c73fa19174cdd3f8a9ff1c77f10b7096af#diff-968b624e975ad3df52446c206926d60aR5251

      Add an extra DB call as it fetches the forum (entities/forum object) from the repository. The entity is used by the capability manager later on here:
      https://github.com/lameze/moodle/commit/75af28c73fa19174cdd3f8a9ff1c77f10b7096af#diff-968b624e975ad3df52446c206926d60aR5389

      Given, there's a call to fetch a forumobject (a stdClass) right at the start of this method, and then the vault/repo code fetches from the DB AGAIN, I think this needs improving. Perhaps we can ONLY use the entity and just refactor the method code to suit - I'm not sure.

      Just using has_cap instead of the manager work (reduces reads), but this isn't ideal. We want to be using the cap manager.

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                jaked Jake Dallimore
                Reporter:
                jaked Jake Dallimore
                Participants:
                Component watchers:
                Andrew Nicols, Mathew May, Michael Hawkins, Shamim Rezaie, Simey Lameze
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 43 minutes
                  43m