Details

    • Affected Branches:
      MOODLE_22_STABLE
    • Rank:
      56

      Description

      Please check MDL-25432 for more information on orphan modules in course.
      There is no way to clean orphan nodes and hence it gets messy while creating backups.

      It will be nice to have a orphan module cleaner under /admin/tool/

      Also, please update message in backup_factory::get_backup_activity_task and backupo_activity_task::construct to reflect that you should use this tool to clean orphan modules.

        Issue Links

          Activity

          Rajesh Taneja created issue -
          Rajesh Taneja made changes -
          Field Original Value New Value
          Link This issue QA test addition/change MDL-25432 [ MDL-25432 ]
          Rajesh Taneja made changes -
          Priority Minor [ 4 ] Critical [ 2 ]
          Hide
          Michael de Raadt added a comment -

          We'll have to find some time for you to work on this if you want to.

          Show
          Michael de Raadt added a comment - We'll have to find some time for you to work on this if you want to.
          Michael de Raadt made changes -
          Fix Version/s DEV backlog [ 10464 ]
          Labels triaged
          Assignee moodle.com [ moodle.com ] Rajesh Taneja [ rajeshtaneja ]
          Hide
          Rajesh Taneja added a comment -

          It will be nice to have this in next sprint.

          Show
          Rajesh Taneja added a comment - It will be nice to have this in next sprint.
          Hide
          Rajesh Taneja added a comment -

          This should also provide hook for interactive backup, so user can take decision on how they want to proceed with orphan module cleanup (as and when they encounter orphan modules)

          Show
          Rajesh Taneja added a comment - This should also provide hook for interactive backup, so user can take decision on how they want to proceed with orphan module cleanup (as and when they encounter orphan modules)
          Hide
          Mark Ward added a comment -

          I have implemented a rough but functional version of this as a fix for MDL-25432.

          As stated on that issue, this runs through all courses and identifies orphaned module entries in mdl_course_modules and identifies those in a dialogue to the user.

          The user then has an option to clean up all the identified entries. If they select this option the orphaned entries will be removed from the mdl_course_modules table.

          A good expansion to this would be to allow users to choose which orphaned entries they wish to remove by listing each with a tick box. This may be overwhelming on sites with large amounts of orphaned entries, however.

          Show
          Mark Ward added a comment - I have implemented a rough but functional version of this as a fix for MDL-25432 . As stated on that issue, this runs through all courses and identifies orphaned module entries in mdl_course_modules and identifies those in a dialogue to the user. The user then has an option to clean up all the identified entries. If they select this option the orphaned entries will be removed from the mdl_course_modules table. A good expansion to this would be to allow users to choose which orphaned entries they wish to remove by listing each with a tick box. This may be overwhelming on sites with large amounts of orphaned entries, however.
          Mark Ward made changes -
          Attachment fixbackups_2.zip [ 26727 ]
          Chris Follin made changes -
          Labels triaged moodlerooms partner triaged
          Hide
          Rajesh Taneja added a comment -

          Thanks for the patch Mark, will try to look at this today

          Show
          Rajesh Taneja added a comment - Thanks for the patch Mark, will try to look at this today
          Hide
          Rajesh Taneja added a comment - - edited

          Hello Mark,
          I looked at your patch and it looks great. Although, I have few questions for you, before I can look further on this:

          1. You are just checking for course module instance in appropriate module table. Is it possible that it exists, but not linked to any section? I mean course_module->instance = {$module_name}- >id, but course_module->section != course_section->id.
          2. Is it possible that module doesn't exist, but we have a module instance? I mean, course->module != module->id.

          I am just trying to get all possible cases to make sure we don't miss anything. As, i am not sure which/what data is actually getting corrupt.

          To summarize, we should check for:

          1. course_module->instance exists in {$module_name}->id (which you are checking)
          2. course_module->section exists in course_section->id ?
          3. course_module->module exists in modules->id ?

          Also, do we need to check for module blocks? Is it possible that they exists for orphan modules?

          Show
          Rajesh Taneja added a comment - - edited Hello Mark, I looked at your patch and it looks great. Although, I have few questions for you, before I can look further on this: You are just checking for course module instance in appropriate module table. Is it possible that it exists, but not linked to any section? I mean course_module->instance = {$module_name}- >id, but course_module->section != course_section->id. Is it possible that module doesn't exist, but we have a module instance? I mean, course->module != module->id. I am just trying to get all possible cases to make sure we don't miss anything. As, i am not sure which/what data is actually getting corrupt. To summarize, we should check for: course_module->instance exists in {$module_name}->id (which you are checking) course_module->section exists in course_section->id ? course_module->module exists in modules->id ? Also, do we need to check for module blocks? Is it possible that they exists for orphan modules?
          Rajesh Taneja made changes -
          Status Open [ 1 ] Development in progress [ 3 ]
          Hide
          Michael de Raadt added a comment -

          It would be good to see this able to be run from cron and from the CLI.

          Show
          Michael de Raadt added a comment - It would be good to see this able to be run from cron and from the CLI.
          Hide
          Rajesh Taneja added a comment -

          Adding Eloy, to have some inputs from him.

          Show
          Rajesh Taneja added a comment - Adding Eloy, to have some inputs from him.
          Hide
          Mark Ward added a comment -

          Hi Rajesh.

          In terms of what was stopping the backups from working we were looking for orphaned instances, which explains why my attempt was a bit single-minded .

          I've altered my code to check for instances of (2) and (3) on my test server. I found that there are indeed course_modules which are orphaned from sections, although I don't understand what has caused them. Given that these are linked to real instances of modules we don't want to just remove them, do we? Maybe we could move them to section 0 in the same course if they are a real instances, or would that just be more confusing?

          I didn't find any examples of course_modules which have been orphaned from their module, but in fairness I haven't removed any activities or resource plugins from my server. I had always assumed this would be cleaned up by Moodle when the module was uninstalled though?

          Many thanks for taking a look!

          Mark

          Show
          Mark Ward added a comment - Hi Rajesh. In terms of what was stopping the backups from working we were looking for orphaned instances, which explains why my attempt was a bit single-minded . I've altered my code to check for instances of (2) and (3) on my test server. I found that there are indeed course_modules which are orphaned from sections, although I don't understand what has caused them. Given that these are linked to real instances of modules we don't want to just remove them, do we? Maybe we could move them to section 0 in the same course if they are a real instances, or would that just be more confusing? I didn't find any examples of course_modules which have been orphaned from their module, but in fairness I haven't removed any activities or resource plugins from my server. I had always assumed this would be cleaned up by Moodle when the module was uninstalled though? Many thanks for taking a look! Mark
          Ankit Agarwal made changes -
          Link This issue has a non-specific relationship to MDL-28963 [ MDL-28963 ]
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback Mark,

          I am not sure of the cause of this issue and is it a good idea to delete modules which are not linked to section. Eloy is the best person to answer this, so will wait for him to respond.

          IMO, this should not happen in first place. So in addition to orphan cleaner, it would be nice to fix the real cause of database corruption.

          Show
          Rajesh Taneja added a comment - Thanks for the feedback Mark, I am not sure of the cause of this issue and is it a good idea to delete modules which are not linked to section. Eloy is the best person to answer this, so will wait for him to respond. IMO, this should not happen in first place. So in addition to orphan cleaner, it would be nice to fix the real cause of database corruption.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          1) I think that deleting course_modules not having activity instances is ok, as far as they only can lead to problems here and there. And there is nothing we can recover there.

          2) Surely we could do also the inverse: look for instances not having course_modules, and perhaps offer to "revive" them.

          3) And surely we could also, look for correct modules missing sections, and perhaps offer to "revive" them too.

          But, for sure, only in 1) we can delete (cannot recover anything). IMO both in 2) and 3) we should not delete, there is information to recover.

          Also, it would be worth knowing how/where the system arrives to any of the 1), 2), 3) situations above. They simply should not happen.

          Perhaps... the section ones (3)... are about course which number of sections has been reduced in the course config page or so? Just guessing...

          But, for sure, delete only when there is nothing to lose (1). Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - 1) I think that deleting course_modules not having activity instances is ok, as far as they only can lead to problems here and there. And there is nothing we can recover there. 2) Surely we could do also the inverse: look for instances not having course_modules, and perhaps offer to "revive" them. 3) And surely we could also, look for correct modules missing sections, and perhaps offer to "revive" them too. But, for sure, only in 1) we can delete (cannot recover anything). IMO both in 2) and 3) we should not delete, there is information to recover. Also, it would be worth knowing how/where the system arrives to any of the 1), 2), 3) situations above. They simply should not happen. Perhaps... the section ones (3)... are about course which number of sections has been reduced in the course config page or so? Just guessing... But, for sure, delete only when there is nothing to lose (1). Ciao
          Rajesh Taneja made changes -
          Fix Version/s STABLE Sprint 18 [ 11650 ]
          Fix Version/s DEV backlog [ 10464 ]
          Hide
          Mark Ward added a comment - - edited

          My working theory for (1) is that it occurs if someone clicks "add a resource" or "add an activity" and then quickly clicks back on the browser. If this is the case the it would be best handled as part of the cron job.

          I will do some testing and see how accurate this is... :LATER: Well I have tried a few ideas of what might happen during normal operation but cannot recreate the orphaned instances .

          I do think we need to understand what is causing these orphans before anything is implemented to resolve them.

          Show
          Mark Ward added a comment - - edited My working theory for (1) is that it occurs if someone clicks "add a resource" or "add an activity" and then quickly clicks back on the browser. If this is the case the it would be best handled as part of the cron job. I will do some testing and see how accurate this is... :LATER: Well I have tried a few ideas of what might happen during normal operation but cannot recreate the orphaned instances . I do think we need to understand what is causing these orphans before anything is implemented to resolve them.
          Hide
          Rajesh Taneja added a comment -

          Moving it to next sprint, will try finalize this in this sprint

          Show
          Rajesh Taneja added a comment - Moving it to next sprint, will try finalize this in this sprint
          Rajesh Taneja made changes -
          Fix Version/s STABLE Sprint 19 [ 11951 ]
          Fix Version/s STABLE Sprint 18 [ 11650 ]
          Hide
          Rajesh Taneja added a comment -

          I tried recreating orphan modules, but was unsuccessful. Will try stopping backup or running cron with some magic to reproduce it. I agree, without knowing the real cause of this issue, there won't be any nice solution.

          Next week, I will open a discussion, to find some answers on this.

          Show
          Rajesh Taneja added a comment - I tried recreating orphan modules, but was unsuccessful. Will try stopping backup or running cron with some magic to reproduce it. I agree, without knowing the real cause of this issue, there won't be any nice solution. Next week, I will open a discussion, to find some answers on this.
          Hide
          Rajesh Taneja added a comment -

          Had a word with Petr about this, and the right way to go about this is to create /admin/tool/database_cleaner, which should allow admin to have a look at orphans and clean them.
          It seems like a dev job and can't be done in a sprint, hence moving it to Dev backlog. In the mean while, will try to see how we can avoid orphans and provide a patch for the same.

          Show
          Rajesh Taneja added a comment - Had a word with Petr about this, and the right way to go about this is to create /admin/tool/database_cleaner, which should allow admin to have a look at orphans and clean them. It seems like a dev job and can't be done in a sprint, hence moving it to Dev backlog. In the mean while, will try to see how we can avoid orphans and provide a patch for the same.
          Rajesh Taneja made changes -
          Fix Version/s DEV backlog [ 10464 ]
          Fix Version/s STABLE Sprint 19 [ 11951 ]
          Hide
          Rajesh Taneja added a comment -

          Assigning this to moodle, so that someone from dev team can look at this.

          Show
          Rajesh Taneja added a comment - Assigning this to moodle, so that someone from dev team can look at this.
          Rajesh Taneja made changes -
          Assignee Rajesh Taneja [ rajeshtaneja ] moodle.com [ moodle.com ]
          Ankit Agarwal made changes -
          Link This issue QA test addition/change MDL-31671 [ MDL-31671 ]
          Ankit Agarwal made changes -
          Link This issue will help resolve MDL-36506 [ MDL-36506 ]
          Michael de Raadt made changes -
          Fix Version/s BACKEND [ 12582 ]
          Michael de Raadt made changes -
          Component/s Other [ 10063 ]
          Michael de Raadt made changes -
          Component/s Administration [ 10050 ]
          Hide
          Marina Glancy added a comment -

          Hi. Some tools already exist:

          MDL-37028
          In 2.6 there is a CLI tool that restores orphaned modules: /admin/cli/fix_course_sequence.php
          https://github.com/moodle/moodle/blob/MOODLE_26_STABLE/admin/cli/fix_course_sequence.php

          MDL-38228
          In 2.5 and 2.6 there is an upgrade script that finds orphan modules and restores them in the course (marked as hidden):
          https://github.com/moodle/moodle/commit/47644f7d6a0595d94e72d92ca7ce74b8e0e98ee2

          Some situations that those tools do not cover:
          course_module links to non-existing course
          entry in course_module exists and in the corresponding module table not (forum, assign, etc.)

          Show
          Marina Glancy added a comment - Hi. Some tools already exist: MDL-37028 In 2.6 there is a CLI tool that restores orphaned modules: /admin/cli/fix_course_sequence.php https://github.com/moodle/moodle/blob/MOODLE_26_STABLE/admin/cli/fix_course_sequence.php MDL-38228 In 2.5 and 2.6 there is an upgrade script that finds orphan modules and restores them in the course (marked as hidden): https://github.com/moodle/moodle/commit/47644f7d6a0595d94e72d92ca7ce74b8e0e98ee2 Some situations that those tools do not cover: course_module links to non-existing course entry in course_module exists and in the corresponding module table not (forum, assign, etc.)
          Marina Glancy made changes -
          Link This issue will be (partly) resolved by MDL-38228 [ MDL-38228 ]
          Marina Glancy made changes -
          Link This issue will be (partly) resolved by MDL-37028 [ MDL-37028 ]
          Martin Dougiamas made changes -
          Rank Ranked higher

            People

            • Votes:
              6 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated: