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

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

          Attachments

            Issue Links

              Activity

              Hide
              cfollin 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
              cfollin 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
              mblake Michael Blake added a comment -

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

              Show
              mblake Michael Blake added a comment - This issue has been reported as causing an issue for a MP. Please give it priority.
              Hide
              nebgor 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
              nebgor 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
              nebgor Aparup Banerjee added a comment -

              linking bug about cleaning up zero instances after restore.

              Show
              nebgor Aparup Banerjee added a comment - linking bug about cleaning up zero instances after restore.
              Hide
              nebgor 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
              nebgor 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
              mudrd8mz David Mudrák 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
              mudrd8mz David Mudrák 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
              nebgor 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
              nebgor 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
              cfollin 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
              cfollin 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
              mudrd8mz David Mudrák added a comment -

              Looking good and safe for me now.

              Show
              mudrd8mz David Mudrák added a comment - Looking good and safe for me now.
              Hide
              nebgor 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
              nebgor 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
              samhemelryk Sam Hemelryk added a comment -

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

              Cheers
              Sam

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Apu, this has been integrated now. Please add some testing instructions. Cheers Sam
              Hide
              andyjdavis 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
              andyjdavis 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
              nebgor Aparup Banerjee added a comment -

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

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

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

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

              Sent upstream and closing, many thanks!

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

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    10/Oct/11