Skip to content

Commit 7ce48c3

Browse files
committed
Remove transaction from fetch work API.
We have seen the transaction to fail, resulting in exceptions/500s. Part of fixing #2848. There is also no need to have a transaction at all. We now do check after the update whether we won instead and if not, tell the judgehost to try again. Before this, I could with 4 judgedaemons on my laptop reliably reproduce the error by just judging the example problems, seeing it ~5 times for all ~100 submissions. Afterwards, I ran this 10 times and didn't encounter any error.
1 parent 228b1fa commit 7ce48c3

File tree

1 file changed

+31
-29
lines changed

1 file changed

+31
-29
lines changed

webapp/src/Controller/API/JudgehostController.php

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,40 +1600,42 @@ public function getJudgeTasksAction(Request $request): array
16001600
// This is case 2.a) from above: start something new.
16011601
// This runs transactional to prevent a queue task being picked up twice.
16021602
$judgetasks = null;
1603-
$this->em->wrapInTransaction(function () use ($judgehost, $max_batchsize, &$judgetasks) {
1604-
$jobid = $this->em->createQueryBuilder()
1605-
->from(QueueTask::class, 'qt')
1606-
->innerJoin('qt.judging', 'j')
1607-
->select('j.judgingid')
1603+
$jobid = $this->em->createQueryBuilder()
1604+
->from(QueueTask::class, 'qt')
1605+
->innerJoin('qt.judging', 'j')
1606+
->select('j.judgingid')
1607+
->andWhere('qt.startTime IS NULL')
1608+
->addOrderBy('qt.priority')
1609+
->addOrderBy('qt.teamPriority')
1610+
->setMaxResults(1)
1611+
->getQuery()
1612+
->getOneOrNullResult(AbstractQuery::HYDRATE_SINGLE_SCALAR);
1613+
if ($jobid !== null) {
1614+
// Mark it as being worked on.
1615+
$result = $this->em->createQueryBuilder()
1616+
->update(QueueTask::class, 'qt')
1617+
->set('qt.startTime', Utils::now())
1618+
->andWhere('qt.judging = :jobid')
16081619
->andWhere('qt.startTime IS NULL')
1609-
->addOrderBy('qt.priority')
1610-
->addOrderBy('qt.teamPriority')
1611-
->setMaxResults(1)
1620+
->setParameter('jobid', $jobid)
16121621
->getQuery()
1613-
->getOneOrNullResult(AbstractQuery::HYDRATE_SINGLE_SCALAR);
1614-
if ($jobid === null) {
1615-
return;
1616-
}
1617-
$judgetasks = $this->getJudgetasks($jobid, $max_batchsize, $judgehost);
1618-
if (empty($judgetasks)) {
1619-
// Somehow we got ourselves in a situation that there was a queue task without remaining judge tasks.
1620-
// This should not happen, but if it does, we need to clean up. Each of the fetch-work calls will clean
1621-
// up one queue task. We need to signal to the judgehost that there might be more work to do.
1622+
->execute();
1623+
1624+
if ($result == 0) {
1625+
// Another judgehost beat us to it.
16221626
$judgetasks = [['type' => 'try_again']];
16231627
} else {
1624-
// Mark it as being worked on.
1625-
$this->em->createQueryBuilder()
1626-
->update(QueueTask::class, 'qt')
1627-
->set('qt.startTime', Utils::now())
1628-
->andWhere('qt.judging = :jobid')
1629-
->andWhere('qt.startTime IS NULL')
1630-
->setParameter('jobid', $jobid)
1631-
->getQuery()
1632-
->execute();
1628+
$judgetasks = $this->getJudgetasks($jobid, $max_batchsize, $judgehost);
1629+
if (empty($judgetasks)) {
1630+
// Somehow we got ourselves in a situation that there was a queue task without remaining judge tasks.
1631+
// This should not happen, but if it does, we need to clean up. Each of the fetch-work calls will clean
1632+
// up one queue task. We need to signal to the judgehost that there might be more work to do.
1633+
$judgetasks = [['type' => 'try_again']];
1634+
}
1635+
}
1636+
if (!empty($judgetasks)) {
1637+
return $judgetasks;
16331638
}
1634-
});
1635-
if (!empty($judgetasks)) {
1636-
return $judgetasks;
16371639
}
16381640

16391641
if ($this->config->get('enable_parallel_judging')) {

0 commit comments

Comments
 (0)