Moodle
  1. Moodle
  2. MDL-34451

Flaw in attempt page update, breaks some attempts, huge performance hit

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.3.2
    • Component/s: Quiz
    • Labels:
    • Testing Instructions:
      Hide

      1. Attempt a multi-page quiz as a student.

      2. From some page in the middle, click 'Finish attempt...'

      3. Then click 'Return to attempt' and make sure you go back to the page you were on.

      4. If you can be bothered, look directly in the DB, and verify that quiz_attempts.currentpage is only being changed for the current attempt, not for all attempts!

      Show
      1. Attempt a multi-page quiz as a student. 2. From some page in the middle, click 'Finish attempt...' 3. Then click 'Return to attempt' and make sure you go back to the page you were on. 4. If you can be bothered, look directly in the DB, and verify that quiz_attempts.currentpage is only being changed for the current attempt, not for all attempts!
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
    • Rank:
      42855

      Description

      New logic was added in 2.3 that sets the attempt page if it doesn't match, but the updates happens unconditionally, so all attempt pages are set to the page of the current attempt, which then means every other attempt will do this too when visited, causing a vicious cycle.

      On large sites, this means that you have a very large update whenever and attempt is done. On postgres, this causes tons of dead records (we had over 20 million, they were being made faster than vaccum could keep up). The updates would cause the attemps to timeout, because it was updating so many records. The fix is to include the missing parameters for the set_field call in mod/quiz/attempt.php

        Issue Links

          Activity

          Hide
          Eric Merrill added a comment -

          I believe that MDL-34212 was caused by the incorrect setting of pages for all attempts as reported in MLD-34451. I'm not sure if the attempts will fix themselves after the patch in MDL-34212, of if we need to actively correct the already broken ones.

          Show
          Eric Merrill added a comment - I believe that MDL-34212 was caused by the incorrect setting of pages for all attempts as reported in MLD-34451. I'm not sure if the attempts will fix themselves after the patch in MDL-34212 , of if we need to actively correct the already broken ones.
          Hide
          Tim Hunt added a comment -

          Wow! how did I let that one slip through peer review. Thank you for spotting this. Submitting for integration.

          Show
          Tim Hunt added a comment - Wow! how did I let that one slip through peer review. Thank you for spotting this. Submitting for integration.
          Hide
          Dan Poltawski added a comment -

          Thanks Eric/Tim i've integrated this now.

          Please could you add some regression testing instructions.

          Show
          Dan Poltawski added a comment - Thanks Eric/Tim i've integrated this now. Please could you add some regression testing instructions.
          Hide
          Rajesh Taneja added a comment -

          Works Great,
          Thanks for fixing this, Tim.

          Show
          Rajesh Taneja added a comment - Works Great, Thanks for fixing this, Tim.
          Hide
          Aparup Banerjee added a comment -

          yay, it works!

          This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

          Thank you all for taking the time to get us here.

          cheers!

          Show
          Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!
          Hide
          Mark van Hoek added a comment -

          Thanks you guys - much appreciated.

          Show
          Mark van Hoek added a comment - Thanks you guys - much appreciated.
          Hide
          Jean-Luc Delghust added a comment -

          Thanks!

          Show
          Jean-Luc Delghust added a comment - Thanks!
          Hide
          Dominik Jeni added a comment -

          "This issue will help resolve: MDL-34212 Cannot continue Quiz - get "No questions found" error"

          --> Integrating this fix manually, it didn't solve the problem. Sometimes when re-attempting a quiz, it jumps automatically to "page=58" (which is not existing).

          After testing the fix MDL-34599, this bug is gone.. BUT then, I can't finish a quiz (missing parameter)!

          Show
          Dominik Jeni added a comment - "This issue will help resolve: MDL-34212 Cannot continue Quiz - get "No questions found" error" --> Integrating this fix manually, it didn't solve the problem. Sometimes when re-attempting a quiz, it jumps automatically to "page=58" (which is not existing). After testing the fix MDL-34599 , this bug is gone.. BUT then, I can't finish a quiz (missing parameter)!
          Hide
          Eric Merrill added a comment -

          Comment over in MDL-34212 so we can get it re-opened. That was I just marked it as related, not a duplicate. I think this bug caused the original problem, but wouldn't necessarily fix certain already broken records.

          Show
          Eric Merrill added a comment - Comment over in MDL-34212 so we can get it re-opened. That was I just marked it as related, not a duplicate. I think this bug caused the original problem, but wouldn't necessarily fix certain already broken records.
          Hide
          Tim Hunt added a comment -

          MDL-34599 will fix the original problem, and it has been integrated and will be part of this week's release.

          Show
          Tim Hunt added a comment - MDL-34599 will fix the original problem, and it has been integrated and will be part of this week's release.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: