Implement batch scheduling bridge call #1901
Conversation
|
Did you know that you can edit the description? |
Ninjaclasher
force-pushed
the
batch-bridge
branch
5 times, most recently
from
825d4db
to
41806c7
Compare
|
I scheduled 56 submissions for rejudging and nothing broke 👍 . |
| @@ -160,7 +160,6 @@ def judge(self, request, queryset): | |||
| self.message_user(request, gettext('You do not have the permission to rejudge submissions.'), | |||
| level=messages.ERROR) | |||
| return | |||
| queryset = queryset.order_by('id') | |||
There was a problem hiding this comment.
Is this unrelated to the main focus of this PR? Just curious.
judge/models/submission.py
Outdated
|
|
||
| judge.alters_data = True | ||
|
|
||
| @classmethod | ||
| def batch_judge(cls, submissions, *, rejudge=False, force_judge=False, rejudge_user=None, chunk_size=100, **kwargs): |
There was a problem hiding this comment.
Would it be less clunky to put this on the QuerySet given how you are using it?
| } | ||
|
|
||
| with transaction.atomic(): | ||
| submission_queryset = Submission.objects.filter(id__in=map(attrgetter('id'), submissions)) \ |
There was a problem hiding this comment.
If the argument is a queryset, then you wouldn't need to do this hack and could just do .exclude on the queryset directly. And then you can just load this queryset to get the submissions instead of filtering in Python.
There was a problem hiding this comment.
If the argument is a queryset, that means it will be lazy. We would be doing the splicing at the database layer, so the slice may change between batches. For example, consider we were trying to rejudge Submission.objects.exclude(status='QU'). As we're processing each batch, [0:batch_size] would always contain a not judged queryset since the previous batch's submissions would become QU (and thus filtered out). We would only process the [0:batch_size] splice once, and so we would be missing some submissions.
From what I could tell, there's 2 solutions:
- Use a single batch. This wouldn't handle large querysets well, so I opted not to do this.
- Evaluate the queryset so we have a fixed set of submissions to judge in each batch, which is what is currently done via
.iterator().
Other solutions are welcome.
| ids = set(submission_queryset.values_list('id', flat=True)) | ||
| # Do the filtering using the new set of IDs rather than submission_queryset | ||
| # because submission_queryset itself takes a list of IDs anyways. | ||
| SubmissionTestCase.objects.filter(submission_id__in=ids).delete() |
There was a problem hiding this comment.
And then you can just pass submission_queryset as the argument here.
judge/judgeapi.py
Outdated
| # It's not worth further complicating this code by separating contest submissions and | ||
| # normal submissions as that case is not even being used. | ||
| # For the moment, let's just set all submissions to have "DEFAULT_PRIORITY" if we're not rejudging. | ||
| 'priority': BATCH_REJUDGE_PRIORITY if rejudge else DEFAULT_PRIORITY, |
There was a problem hiding this comment.
Why not just move the priority into the list? Then you can set per-submission priorities. In fact priority is a per-submission priority and should be in there anyway...
Ninjaclasher
force-pushed
the
batch-bridge
branch
from
41806c7
to
2a15ef1
Compare
Closes #1318.