Add limit for maximum number of retries for automatic regrading, and …

…rotate the submission to be sent.

Fixes apluslms#1147

aplus/celery.py

@@ -5,6 +5,7 @@
 import logging
 from dateutil.relativedelta import relativedelta
 from time import sleep
+from random import choice
 
 from django.conf import settings
 
@@ -49,13 +50,23 @@ def retry_submissions():
     from exercise.submission_models import PendingSubmission
 
     # Recovery state: only send one grading request to probe the state of grader
-    # We pick the one with most attempts, so that total_retries comes down more quickly
-    # when things are back to normal, and we can return to normal state
     if not PendingSubmission.objects.is_grader_stable():
-        pending = PendingSubmission.objects.order_by('-num_retries').first()
-        # pylint: disable-next=logging-fstring-interpolation
-        logging.info(f"Recovery state: retrying expired submission {pending.submission}")
-        pending.submission.exercise.grade(pending.submission)
+        # Get ids of all pending submissions and randomly load one to be retried
+        # (do not load all the submissions objects to save memory)
+        submission_ids = PendingSubmission.objects.values_list('id',flat=True)
+        random_choice = choice(submission_ids)
+        pending = PendingSubmission.objects.get(pk=random_choice)
+        if pending.num_retries >= settings.SUBMISSION_RETRY_LIMIT and settings.SUBMISSION_RETRY_LIMIT > 0:
+            # pylint: disable-next=logging-fstring-interpolation
+            logging.info(f"Recovery state: submission retry limit exceeded for submission {pending.submission} - removing from pending")
+            pending.submission.set_error()
+            pending.submission.save()
+            pending.delete()
+        else:
+            if pending.submission.exercise.can_regrade:
+                # pylint: disable-next=logging-fstring-interpolation
+                logging.info(f"Recovery state: retrying expired submission {pending.submission} (retries: {pending.num_retries})")
+                pending.submission.exercise.grade(pending.submission)
         return
 
     # Stable state: retry all expired submissions
@@ -66,7 +77,18 @@ def retry_submissions():
 
     for pending in expired:
         if pending.submission.exercise.can_regrade:
-            # pylint: disable-next=logging-fstring-interpolation
-            logger.info(f"Retrying expired submission {pending.submission}")
-            pending.submission.exercise.grade(pending.submission)
-            sleep(0.5)  # Delay 500 ms to avoid choking grader
+            # Do not retry submission until SUBMISSION_EXPIRY_TIMEOUT * num_retries has passed
+            pending_timelimit = datetime.datetime.now(datetime.timezone.utc) - relativedelta(seconds=settings.SUBMISSION_EXPIRY_TIMEOUT*pending.num_retries)
+            if pending.num_retries < settings.SUBMISSION_RETRY_LIMIT:
+                if pending.submission_time < pending_timelimit:
+                    # pylint: disable-next=logging-fstring-interpolation
+                    logging.info(f"Retrying expired submission {pending.submission} (retries: {pending.num_retries})")
+                    pending.submission.exercise.grade(pending.submission)
+                    sleep(0.5)  # Delay 500 ms to avoid choking grader
+                else:
+                    logging.info(f"Not yet retrying submission {pending.submission} (retries: {pending.num_retries})")
+            else:
+                logging.info(f"Could not grade submission {pending.submission} (maximum retries exceeded).")
+                pending.submission.set_error()
+                pending.submission.save()
+                pending.delete()

aplus/settings.py

@@ -280,6 +280,9 @@
 # Network location is sufficient, e.g. "localhost:8080" or "grader.cs.aalto.fi"
 SUBMISSION_RETRY_SERVICES = []
 
+# Maximum number of retries to automatically grade a given submission
+SUBMISSION_RETRY_LIMIT = 3
+
 # Number of unresponded retries beyond which we move to recovery state.
 # In recovery state there likely is more persistent problem with the grader
 # or network that needs fixing.

exercise/templates/exercise/submission_plain.html

@@ -94,7 +94,14 @@
 		</table>
 
 		<div id="exercise-all">
-
+		{% if pending.num_retries > 0 %}
+		<div class="alert alert-warning">
+			{% blocktranslate trimmed with num_retries=pending.num_retries max_retries=pending.max_retries %}
+				SUBMISSION_GRADING_RETRIED -- {{ num_retries }}, {{ max_retries }}
+			{% endblocktranslate %}
+		</div>
+		{% endif %}
+
 		{% if feedback_revealed and submission.assistant_feedback %}
 		<h4>{% translate "ASSISTANT_FEEDBACK" %}</h4>
 		<blockquote>{{ submission.assistant_feedback|safe }}</blockquote>

exercise/views.py

@@ -12,6 +12,7 @@
 from django.views.decorators.clickjacking import xframe_options_exempt
 from django.views.decorators.csrf import csrf_exempt
 from django.db import DatabaseError
+from django.conf import settings
 
 from authorization.permissions import ACCESS
 from course.models import CourseModule, SubmissionTag
@@ -23,7 +24,7 @@
 from .cache.points import ExercisePoints
 from .models import BaseExercise, LearningObject, LearningObjectDisplay
 from .protocol.exercise_page import ExercisePage
-from .submission_models import SubmittedFile, Submission, SubmissionTagging
+from .submission_models import SubmittedFile, Submission, SubmissionTagging, PendingSubmission
 from .viewbase import (
     ExerciseBaseView,
     SubmissionBaseView,
@@ -478,6 +479,16 @@ def get_common_objects(self):
         super().get_common_objects()
         self.page = { "is_wait": "wait" in self.request.GET }
         self.note("page")
+        # If the submission is not in 'ready' state, check if there is a pendingSubmission
+        # object for this submission and fetch the number of retries from it, so the info
+        # can be displayed for the user. Also display the maximum retries from settings.
+        if self.submission.status != 'ready':
+            try:
+                pending = PendingSubmission.objects.get(submission__id=self.submission.id)
+                self.pending = { "num_retries": pending.num_retries, "max_retries": settings.SUBMISSION_RETRY_LIMIT }
+                self.note("pending")
+            except:
+                pass
 
     def get(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
         if not self.feedback_revealed:

locale/en/LC_MESSAGES/django.po

@@ -4868,6 +4868,14 @@ msgstr "Submission"
 msgid "FILES_IN_SUBMISSION"
 msgstr "Files in this submission"
 
+#: exercise/templates/exercise/submission_plain.html
+#, python-format
+msgid "SUBMISSION_GRADING_RETRIED -- %(num_retries)s, %(max_retries)s"
+msgstr ""
+"Grading this submission did not finish in time, and it has been "
+"retried %(num_retries)s time(s). Retries continue until successful, "
+"or until the limit of %(max_retries)s retries is reached."
+
 #: exercise/templates/exercise/submission.html
 #: exercise/templates/exercise/submission_plain.html
 msgid "NO_GRADER_FEEDBACK_FOR_SUBMISSION"

locale/fi/LC_MESSAGES/django.po

@@ -4882,6 +4882,14 @@ msgstr "Palautus"
 msgid "FILES_IN_SUBMISSION"
 msgstr "Tiedostot tässä palautuksessa"
 
+#: exercise/templates/exercise/submission_plain.html
+#, python-format
+msgid "SUBMISSION_GRADING_RETRIED -- %(num_retries)s, %(max_retries)s"
+msgstr ""
+"Palautuksen arvostelu ei valmistunut määräajassa, ja sitä on "
+"yritetty uudelleen %(num_retries)s kertaa. Palautusta yritetään "
+"automaattisesti enintään %(max_retries)s kertaa."
+
 #: exercise/templates/exercise/submission.html
 #: exercise/templates/exercise/submission_plain.html
 msgid "NO_GRADER_FEEDBACK_FOR_SUBMISSION"