MDL-63891 assignfeedback_editpdf: remove queued jobs that keep failing

Without this, its possible to get a document in the editpdf conversion queue
that keeps failing - and in some cases completely crashing the PHP script
despite exception handling - which ends up blocking the processing of the queue
entirely.

This change allows for a configurable number of attempts the conversion
task will perform before removing the item from the queue.

Backport of MDL-57610.

config-dist.php

@@ -588,6 +588,14 @@ $CFG->admin = 'admin';
 //
 //      $CFG->disablelogintoken = true;
 //
+// Document conversion limit
+//
+// How many times the background task should attempt to convert a given attempt
+// before removing it from the queue. Currently this limit is only used by the
+// mod_assign conversion task.
+//
+//      $CFG->conversionattemptlimit = 3;
+//
 //=========================================================================
 // 7. SETTINGS FOR DEVELOPMENT SERVERS - not intended for production use!!!
 //=========================================================================

mod/assign/feedback/editpdf/classes/event/observer.php

@@ -35,17 +35,7 @@ class observer {
      * @param \mod_assign\event\submission_created $event
      */
     public static function submission_created(\mod_assign\event\submission_created $event) {
-        global $DB;
-
-        $submissionid = $event->other['submissionid'];
-        $submissionattempt = $event->other['submissionattempt'];
-        $fields = array( 'submissionid' => $submissionid, 'submissionattempt' => $submissionattempt);
-        $record = (object) $fields;
-
-        $exists = $DB->get_records('assignfeedback_editpdf_queue', $fields);
-        if (!$exists) {
-            $DB->insert_record('assignfeedback_editpdf_queue', $record);
-        }
+        self::queue_conversion($event);
     }
 
     /**
@@ -53,6 +43,14 @@ class observer {
      * @param \mod_assign\event\submission_updated $event
      */
     public static function submission_updated(\mod_assign\event\submission_updated $event) {
+        self::queue_conversion($event);
+    }
+
+    /**
+     * Queue the submission for processing.
+     * @param $event The submission created/updated event.
+     */
+    protected static function queue_conversion($event) {
         global $DB;
 
         $submissionid = $event->other['submissionid'];
@@ -60,9 +58,13 @@ class observer {
         $fields = array( 'submissionid' => $submissionid, 'submissionattempt' => $submissionattempt);
         $record = (object) $fields;
 
-        $exists = $DB->get_records('assignfeedback_editpdf_queue', $fields);
+        $exists = $DB->get_record('assignfeedback_editpdf_queue', $fields);
         if (!$exists) {
             $DB->insert_record('assignfeedback_editpdf_queue', $record);
+        } else {
+            // This submission attempt was already queued, so just reset the existing failure counter to ensure it gets processed.
+            $exists->attemptedconversions = 0;
+            $DB->update_record('assignfeedback_editpdf_queue', $exists);
         }
     }
 }

mod/assign/feedback/editpdf/classes/task/convert_submissions.php

@@ -58,15 +58,21 @@ class convert_submissions extends scheduled_task {
 
         $assignmentcache = array();
 
+        $conversionattemptlimit = !empty($CFG->conversionattemptlimit) ? $CFG->conversionattemptlimit : 3;
         foreach ($records as $record) {
             $submissionid = $record->submissionid;
             $submission = $DB->get_record('assign_submission', array('id' => $submissionid), '*', IGNORE_MISSING);
-            if (!$submission) {
-                // Submission no longer exists.
+            if (!$submission || $record->attemptedconversions >= $conversionattemptlimit) {
+                // Submission no longer exists; or we've exceeded the conversion attempt limit.
                 $DB->delete_records('assignfeedback_editpdf_queue', array('id' => $record->id));
                 continue;
             }
 
+            // Record that we're attempting the conversion ahead of time.
+            // We can't do this afterwards as its possible for the conversion process to crash the script entirely.
+            $record->attemptedconversions++;
+            $DB->update_record('assignfeedback_editpdf_queue', $record);
+
             $assignmentid = $submission->assignment;
             $attemptnumber = $record->submissionattempt;
 

mod/assign/feedback/editpdf/db/install.xml

@@ -64,6 +64,7 @@
         <FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
         <FIELD NAME="submissionid" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
         <FIELD NAME="submissionattempt" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
+        <FIELD NAME="attemptedconversions" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
       </FIELDS>
       <KEYS>
         <KEY NAME="primary" TYPE="primary" FIELDS="id"/>

mod/assign/feedback/editpdf/db/upgrade.php

@@ -62,5 +62,38 @@ function xmldb_assignfeedback_editpdf_upgrade($oldversion) {
     // Automatically generated Moodle v3.5.0 release upgrade line.
     // Put any upgrade step following this.
 
+    if ($oldversion < 2018051401) {
+        $table = new xmldb_table('assignfeedback_editpdf_queue');
+        $field = new xmldb_field('attemptedconversions', XMLDB_TYPE_INTEGER, '10', null,
+            XMLDB_NOTNULL, null, 0, 'submissionattempt');
+
+        if (!$dbman->field_exists($table, $field)) {
+            $dbman->add_field($table, $field);
+        }
+
+        // Attempts are removed from the queue after being processed, a duplicate row won't achieve anything productive.
+        // So look for any duplicates and remove them so we can add a unique key.
+        $sql = "SELECT MIN(id) as minid, submissionid, submissionattempt
+                FROM {assignfeedback_editpdf_queue}
+                GROUP BY submissionid, submissionattempt
+                HAVING COUNT(id) > 1";
+
+        if ($duplicatedrows = $DB->get_recordset_sql($sql)) {
+            foreach ($duplicatedrows as $row) {
+                $DB->delete_records_select('assignfeedback_editpdf_queue',
+                    'submissionid = :submissionid AND submissionattempt = :submissionattempt AND id <> :minid', (array)$row);
+            }
+        }
+        $duplicatedrows->close();
+
+        // Define key submissionid-submissionattempt to be added to assignfeedback_editpdf_queue.
+        $table = new xmldb_table('assignfeedback_editpdf_queue');
+        $key = new xmldb_key('submissionid-submissionattempt', XMLDB_KEY_UNIQUE, ['submissionid', 'submissionattempt']);
+
+        $dbman->add_key($table, $key);
+
+        upgrade_plugin_savepoint(true, 2018051401, 'assignfeedback', 'editpdf');
+    }
+
     return true;
 }

mod/assign/feedback/editpdf/version.php

@@ -24,7 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2018051400;
+$plugin->version   = 2018051401;
 $plugin->requires  = 2018050800;
 $plugin->component = 'assignfeedback_editpdf';
-