Skip to content

Commit 88f7c64

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 e1c20b8 commit 88f7c64

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
@@ -1606,40 +1606,42 @@ public function getJudgeTasksAction(Request $request): array
16061606
// This is case 2.a) from above: start something new.
16071607
// This runs transactional to prevent a queue task being picked up twice.
16081608
$judgetasks = null;
1609-
$this->em->wrapInTransaction(function () use ($judgehost, $max_batchsize, &$judgetasks) {
1610-
$jobid = $this->em->createQueryBuilder()
1611-
->from(QueueTask::class, 'qt')
1612-
->innerJoin('qt.judging', 'j')
1613-
->select('j.judgingid')
1609+
$jobid = $this->em->createQueryBuilder()
1610+
->from(QueueTask::class, 'qt')
1611+
->innerJoin('qt.judging', 'j')
1612+
->select('j.judgingid')
1613+
->andWhere('qt.startTime IS NULL')
1614+
->addOrderBy('qt.priority')
1615+
->addOrderBy('qt.teamPriority')
1616+
->setMaxResults(1)
1617+
->getQuery()
1618+
->getOneOrNullResult(AbstractQuery::HYDRATE_SINGLE_SCALAR);
1619+
if ($jobid !== null) {
1620+
// Mark it as being worked on.
1621+
$result = $this->em->createQueryBuilder()
1622+
->update(QueueTask::class, 'qt')
1623+
->set('qt.startTime', Utils::now())
1624+
->andWhere('qt.judging = :jobid')
16141625
->andWhere('qt.startTime IS NULL')
1615-
->addOrderBy('qt.priority')
1616-
->addOrderBy('qt.teamPriority')
1617-
->setMaxResults(1)
1626+
->setParameter('jobid', $jobid)
16181627
->getQuery()
1619-
->getOneOrNullResult(AbstractQuery::HYDRATE_SINGLE_SCALAR);
1620-
if ($jobid === null) {
1621-
return;
1622-
}
1623-
$judgetasks = $this->getJudgetasks($jobid, $max_batchsize, $judgehost);
1624-
if (empty($judgetasks)) {
1625-
// Somehow we got ourselves in a situation that there was a queue task without remaining judge tasks.
1626-
// This should not happen, but if it does, we need to clean up. Each of the fetch-work calls will clean
1627-
// up one queue task. We need to signal to the judgehost that there might be more work to do.
1628+
->execute();
1629+
1630+
if ($result == 0) {
1631+
// Another judgehost beat us to it.
16281632
$judgetasks = [['type' => 'try_again']];
16291633
} else {
1630-
// Mark it as being worked on.
1631-
$this->em->createQueryBuilder()
1632-
->update(QueueTask::class, 'qt')
1633-
->set('qt.startTime', Utils::now())
1634-
->andWhere('qt.judging = :jobid')
1635-
->andWhere('qt.startTime IS NULL')
1636-
->setParameter('jobid', $jobid)
1637-
->getQuery()
1638-
->execute();
1634+
$judgetasks = $this->getJudgetasks($jobid, $max_batchsize, $judgehost);
1635+
if (empty($judgetasks)) {
1636+
// Somehow we got ourselves in a situation that there was a queue task without remaining judge tasks.
1637+
// This should not happen, but if it does, we need to clean up. Each of the fetch-work calls will clean
1638+
// up one queue task. We need to signal to the judgehost that there might be more work to do.
1639+
$judgetasks = [['type' => 'try_again']];
1640+
}
1641+
}
1642+
if (!empty($judgetasks)) {
1643+
return $judgetasks;
16391644
}
1640-
});
1641-
if (!empty($judgetasks)) {
1642-
return $judgetasks;
16431645
}
16441646

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

0 commit comments

Comments
 (0)