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

Assignment print course overview query performance

          Details

          • Testing Instructions:
            Hide

            This is a performance improvement, so not really testable - just check for regressions.

            1. In a course create 3 assignments with file submissions enabled
            2. Login as a student and upload a file submission to each assignment
            3. View the "My home" page for the student (has the course_overview block by default).
            4. Verify there are no reported errors
            5. Login as a teacher for the same course
            6. View the "My home" page for the teacher.
            7. Verify there are no reported errors
            Show
            This is a performance improvement, so not really testable - just check for regressions. In a course create 3 assignments with file submissions enabled Login as a student and upload a file submission to each assignment View the "My home" page for the student (has the course_overview block by default). Verify there are no reported errors Login as a teacher for the same course View the "My home" page for the teacher. Verify there are no reported errors
          • Affected Branches:
            MOODLE_23_STABLE
          • Fixed Branches:
            MOODLE_23_STABLE, MOODLE_24_STABLE
          • Pull from Repository:
            git@github.com:damyon/moodle.git
          • Pull Master Branch:
            MDL-37527-master
          • Pull Master Diff URL:

            Description

            The query in question is the one to get all user submissions. Please see attached fix, without it indexes are not used.

              Gliffy Diagrams

                Attachments

                  Issue Links

                  caused a regression

                  Bug - A problem which impairs or prevents Moodle from functioning correctly. MDL-37619 Failed unit tests for mod/assign/lib.php

                  • Critical - Crashes server, loss of data, severe memory leak
                  • Closed

                    Activity

                    Ascending order - Click to sort in descending order
                    Hide
                    Permalink
                    damyon Damyon Wiese added a comment -

                    Thanks for reporting this - the query currently will return results for all assignment submissions for the current user - not just the ones in the current course - (due to the condition being ANDed with a LEFT JOIN) - the extra results are not used and this does not cause any display errors (but it is good this patch fixes this).

                    Show
                    damyon Damyon Wiese added a comment - Thanks for reporting this - the query currently will return results for all assignment submissions for the current user - not just the ones in the current course - (due to the condition being ANDed with a LEFT JOIN) - the extra results are not used and this does not cause any display errors (but it is good this patch fixes this).
                    Hide
                    Permalink
                    kstokking Kris Stokking added a comment - - edited

                    This bug affects the course overview block, and has severe performance implications on a site with a large amounts of assignments and a large amount of users accessing MyMoodle. It is highly recommended that the fix be expedited.

                    Show
                    kstokking Kris Stokking added a comment - - edited This bug affects the course overview block, and has severe performance implications on a site with a large amounts of assignments and a large amount of users accessing MyMoodle. It is highly recommended that the fix be expedited.
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

                    TIA and ciao

                    Show
                    poltawski Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
                    Hide
                    Permalink
                    poltawski Dan Poltawski added a comment -

                    Integrated to 23, 24 and master, thanks Mark/Damyon

                    Show
                    poltawski Dan Poltawski added a comment - Integrated to 23, 24 and master, thanks Mark/Damyon
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    Note that the new tests introduced by MDL-37547 are saying this is buggy. So Damyon fixed it @ MDL-37619 ...

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - Note that the new tests introduced by MDL-37547 are saying this is buggy. So Damyon fixed it @ MDL-37619 ...
                    Hide
                    Permalink
                    damyon Damyon Wiese added a comment -

                    So make sure you pull from integration before testing (then you'll get MDL-37619).

                    Show
                    damyon Damyon Wiese added a comment - So make sure you pull from integration before testing (then you'll get MDL-37619 ).
                    Hide
                    Permalink
                    ankit_frenz Ankit Agarwal added a comment -

                    No regression was found.
                    Passing
                    Thanks

                    Show
                    ankit_frenz Ankit Agarwal added a comment - No regression was found. Passing Thanks
                    Hide
                    Permalink
                    stronk7 Eloy Lafuente (stronk7) added a comment -

                    Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

                    Closing as fixed, ciao

                    Show
                    stronk7 Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

                      People

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

                        Dates

                        • Created:
                          Updated:
                          Resolved:
                          Fix Release Date:
                          11/Mar/13