Moodle
  1. Moodle
  2. MDL-27174

Automated Backups Fail When Assignment Course Module Has Instance of Zero (0)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.9.10
    • Fix Version/s: 1.9.14
    • Component/s: Backup
    • Labels:

      Description

      We have multiple client sites with automated backups that do not complete. Error messages point to the assignment module trying to require a file that doesn't exist.

      PHP Warning: require_once(<moodle_root>/mod/assignment/type//assignment.class.php): failed to open stream: No such file or directory in <moodle_root>/mod/assignment/backuplib.php on line 79
      PHP Fatal error: require_once(): Failed opening required '<moodle_root>/mod/assignment/type//assignment.class.php' (include_path='<moodle_root>/lib/pear:.:/usr/share/php:/usr/share/pear') in <moodle_root>/mod/assignment/backuplib.php on line 79

      The middle of the require_once where it has a double slash should have been populated with the assignment type. It's not populated because the course_module record for the assignment has an instance of 0 so Moodle doesn't know what kind of assignment it is. The instance=0 records show up after a course is restored from backup, although not every restore has this issue and it is not limited to backups from different servers.

      We created a patch that allows the assignment backup to skip assignments that have an instance of 0 and removes such records when restoring.

        Gliffy Diagrams

        1. assignmentbackup_part2.patch
          0.5 kB
          Chris Follin
        2. assignmentbackup.patch
          2 kB
          Chris Follin

          Issue Links

            Activity

            Hide
            Chris Follin added a comment -

            We still had some issues with this after the original patch, although the original did help. This second patch is in addition to, not in place of, the first patch.

            Show
            Chris Follin added a comment - We still had some issues with this after the original patch, although the original did help. This second patch is in addition to, not in place of, the first patch.
            Hide
            Michael Blake added a comment -

            This issue has been reported as causing an issue for a MP. Please give it priority.

            Show
            Michael Blake added a comment - This issue has been reported as causing an issue for a MP. Please give it priority.
            Hide
            Aparup Banerjee added a comment - - edited

            Hi Chris,
            is it possible to provide a sample backup that is causing this?

            i've been wondering if the assignment sub-type was installed prior to restoring the course. Could that have caused the instance=0 record?

            if so, we need to improve the zero instances cleanup (ah your patch does do that) or imo preferably not create these zero instance records.

            cheers,
            Aparup

            Show
            Aparup Banerjee added a comment - - edited Hi Chris, is it possible to provide a sample backup that is causing this? i've been wondering if the assignment sub-type was installed prior to restoring the course. Could that have caused the instance=0 record? if so, we need to improve the zero instances cleanup (ah your patch does do that) or imo preferably not create these zero instance records. cheers, Aparup
            Hide
            Aparup Banerjee added a comment -

            linking bug about cleaning up zero instances after restore.

            Show
            Aparup Banerjee added a comment - linking bug about cleaning up zero instances after restore.
            Hide
            Aparup Banerjee added a comment -

            This will need proper review and testing. The suggested patch seems to make sense , just not sure about the improved zero instance cleaning up.

            Surely there will be some regressions or else why was the original solution to zero instances cleanup more strict? but i'm not sure - hopefully peer reviewing this will shed some light.

            Show
            Aparup Banerjee added a comment - This will need proper review and testing. The suggested patch seems to make sense , just not sure about the improved zero instance cleaning up. Surely there will be some regressions or else why was the original solution to zero instances cleanup more strict? but i'm not sure - hopefully peer reviewing this will shed some light.
            Hide
            David Mudrak added a comment -

            Well I believe there is a good reason why the original cleaner selected only the course modules it was interested in: we can't delete all course modules with instance=0 in the given course as there can be some script executed in parallel that just created them and is going to update the field soon...

            Show
            David Mudrak added a comment - Well I believe there is a good reason why the original cleaner selected only the course modules it was interested in: we can't delete all course modules with instance=0 in the given course as there can be some script executed in parallel that just created them and is going to update the field soon...
            Hide
            Aparup Banerjee added a comment -

            Thanks David. I believe you're right. also just realised that field is zero by default.

            Perhaps we should not have to restore such modules in the first place but 1.9 backup/restore is so fantastic

            in any case, that first patch does make sense - i've updated my repo. David, if you can have a look to peer review it again that would be great.

            perhaps cleanup can be improved as an improvement issue?

            Show
            Aparup Banerjee added a comment - Thanks David. I believe you're right. also just realised that field is zero by default. Perhaps we should not have to restore such modules in the first place but 1.9 backup/restore is so fantastic in any case, that first patch does make sense - i've updated my repo. David, if you can have a look to peer review it again that would be great. perhaps cleanup can be improved as an improvement issue?
            Hide
            Chris Follin added a comment -

            Hi Aparup,

            Sorry it has taken me a few days to get back to you. I don't have a sample backup handy at the moment but I might be able to find one if you still need it. It would not seem to be an issue of the assignment type not existing because we had this problem when restoring a course on the exact same site on which the backup was made. The assignment type existed when the backup was made and still existed when the backup was attempted to be restored. That said, I unfortunately still do not have a better idea, or any idea for that matter, of how these 0 instances came to be.

            Show
            Chris Follin added a comment - Hi Aparup, Sorry it has taken me a few days to get back to you. I don't have a sample backup handy at the moment but I might be able to find one if you still need it. It would not seem to be an issue of the assignment type not existing because we had this problem when restoring a course on the exact same site on which the backup was made. The assignment type existed when the backup was made and still existed when the backup was attempted to be restored. That said, I unfortunately still do not have a better idea, or any idea for that matter, of how these 0 instances came to be.
            Hide
            David Mudrak added a comment -

            Looking good and safe for me now.

            Show
            David Mudrak added a comment - Looking good and safe for me now.
            Hide
            Aparup Banerjee added a comment -

            Thanks David,
            Sending this up for integration as it fixes the fatal error.

            Chris,
            Thanks for your fix!

            If the clean up improvements do still need to be done, please open up a separate issue linked to this

            Show
            Aparup Banerjee added a comment - Thanks David, Sending this up for integration as it fixes the fatal error. Chris, Thanks for your fix! If the clean up improvements do still need to be done, please open up a separate issue linked to this
            Hide
            Sam Hemelryk added a comment -

            Thanks Apu, this has been integrated now.
            Please add some testing instructions.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Thanks Apu, this has been integrated now. Please add some testing instructions. Cheers Sam
            Hide
            Andrew Davis added a comment -

            Hi Aparup. Could you please expand those testing instructions a bit. Do you mean an assignment that has several subtypes? Can you provide a link to such a thing? And just generally clean up the English

            Show
            Andrew Davis added a comment - Hi Aparup. Could you please expand those testing instructions a bit. Do you mean an assignment that has several subtypes? Can you provide a link to such a thing? And just generally clean up the English
            Hide
            Aparup Banerjee added a comment -

            Hi Andrew,
            I've clarified it a bit more. Links added too

            Show
            Aparup Banerjee added a comment - Hi Andrew, I've clarified it a bit more. Links added too
            Hide
            Rajesh Taneja added a comment -

            Works as expected...
            Thanks Chris and Aparup for fixing this issue

            Show
            Rajesh Taneja added a comment - Works as expected... Thanks Chris and Aparup for fixing this issue
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Sent upstream and closing, many thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Sent upstream and closing, many thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: