Skip to content

Commit 5263bdb

Browse files
tuupketuupke
committed
Lock, and transaction-free entry_point update
1 parent d4762d2 commit 5263bdb

File tree

1 file changed

+99
-18
lines changed

1 file changed

+99
-18
lines changed

‎webapp/src/Controller/API/JudgehostController.php

Lines changed: 99 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
use Doctrine\DBAL\ArrayParameterType;
3333
use Doctrine\DBAL\Exception;
3434
use Doctrine\DBAL\Exception as DBALException;
35+
use Doctrine\DBAL\TransactionIsolationLevel;
3536
use Doctrine\ORM\AbstractQuery;
3637
use Doctrine\ORM\EntityManager;
3738
use Doctrine\ORM\EntityManagerInterface;
@@ -313,33 +314,89 @@ public function updateJudgingAction(
313314
}
314315

315316
if ($request->request->has('output_compile')) {
317+
$output_compile = base64_decode($request->request->get('output_compile'));
318+
316319
// Note: we use ->get here instead of ->has since entry_point can be the empty string and then we do not
317320
// want to update the submission or send out an update event
318321
if ($request->request->get('entry_point')) {
319-
$this->em->wrapInTransaction(function () use ($query, $request, &$judging) {
320-
$submission = $judging->getSubmission();
321-
if ($submission->getEntryPoint() === $request->request->get('entry_point')) {
322-
return;
322+
// Lock-free setting of, and detection of mismatched entry_point.
323+
$submission = $judging->getSubmission();
324+
325+
// Retrieve, and update the current entrypoint.
326+
$oldEntryPoint = $submission->getEntryPoint();
327+
$newEntryPoint = $request->request->get('entry_point');
328+
329+
330+
if ($oldEntryPoint === $newEntryPoint) {
331+
// Nothing to do
332+
} elseif (!empty($oldEntryPoint)) {
333+
// Conflict detected disable the judgehost.
334+
$disabled = [
335+
'kind' => 'judgehost',
336+
'hostname' => $judgehost->getHostname(),
337+
];
338+
$error = new InternalError();
339+
$error
340+
->setJudging($judging)
341+
->setContest($judging->getContest())
342+
->setDescription('Reported EntryPoint conflict difference for j' . $judging->getJudgingid().'. Expected: "' . $oldEntryPoint. '", received: "' . $newEntryPoint . '".')
343+
->setJudgehostlog(base64_encode('New compilation output: ' . $output_compile))
344+
->setTime(Utils::now())
345+
->setDisabled($disabled);
346+
$this->em->persist($error);
347+
} else {
348+
// Update needed. Note, conflicts might still be possible.
349+
350+
$rowsAffected = $this->em->createQueryBuilder()
351+
->update(Submission::class, 's')
352+
->set('s.entry_point', ':entrypoint')
353+
->andWhere('s.submitid = :id')
354+
->andWhere('s.entry_point IS NULL')
355+
->setParameter('entrypoint', $newEntryPoint)
356+
->setParameter('id', $submission->getSubmitid())
357+
->getQuery()
358+
->execute();
359+
360+
if ($rowsAffected == 0) {
361+
// There is a potential conflict, two options.
362+
// The new entry point is either the same (no issue) or different (conflict).
363+
// Read the entrypoint and check.
364+
$this->em->clear();
365+
$currentEntryPoint = $query->getOneOrNullResult()->getSubmission()->getEntryPoint();
366+
if ($newEntryPoint !== $currentEntryPoint) {
367+
// Conflict detected disable the judgehost.
368+
$disabled = [
369+
'kind' => 'judgehost',
370+
'hostname' => $judgehost->getHostname(),
371+
];
372+
$error = new InternalError();
373+
$error
374+
->setJudging($judging)
375+
->setContest($judging->getContest())
376+
->setDescription('Reported EntryPoint conflict difference for j' . $judging->getJudgingid().'. Expected: "' . $oldEntryPoint. '", received: "' . $newEntryPoint . '".')
377+
->setJudgehostlog(base64_encode('New compilation output: ' . $output_compile))
378+
->setTime(Utils::now())
379+
->setDisabled($disabled);
380+
$this->em->persist($error);
381+
}
382+
} else {
383+
$submissionId = $submission->getSubmitid();
384+
$contestId = $submission->getContest()->getCid();
385+
$this->eventLogService->log('submission', $submissionId,
386+
EventLogService::ACTION_UPDATE, $contestId);
323387
}
324-
$submission->setEntryPoint($request->request->get('entry_point'));
325-
$this->em->flush();
326-
$submissionId = $submission->getSubmitid();
327-
$contestId = $submission->getContest()->getCid();
328-
$this->eventLogService->log('submission', $submissionId,
329-
EventLogService::ACTION_UPDATE, $contestId);
330388

331-
// As EventLogService::log() will clear the entity manager, so the judging has
332-
// now become detached. We will have to reload it.
389+
// As EventLogService::log() will clear the entity manager, both branches clear the entity manager.
390+
// The judging is now detached, reload it.
333391
/** @var Judging $judging */
334392
$judging = $query->getOneOrNullResult();
335-
});
393+
}
336394
}
337395

338396
// Reload judgehost just in case it got cleared above.
339397
/** @var Judgehost $judgehost */
340398
$judgehost = $this->em->getRepository(Judgehost::class)->findOneBy(['hostname' => $hostname]);
341399

342-
$output_compile = base64_decode($request->request->get('output_compile'));
343400
if ($request->request->getBoolean('compile_success')) {
344401
if ($judging->getOutputCompile() === null) {
345402
$judging
@@ -1346,6 +1403,28 @@ public function checkVersions(Request $request, string $judgetaskid): array
13461403
if ($request->request->has('runner')) {
13471404
$reportedVersions['runner'] = base64_decode($request->request->get('runner'));
13481405
}
1406+
1407+
// We want the version update to be both lock-free manner while keeping one latest version exposed.
1408+
// To ensure this is the case a READ_UNCOMMITTED transaction is used. This is not ideal, but does keep both
1409+
// constraints. The updating threads might see multiple versions, but only the newest one is exposed.
1410+
//
1411+
// Keep in mind that there might be multiple concurrent updates while reading the explanation.
1412+
// - First insert a new version assuming it will be the latest version.
1413+
// - Retrieve all 'new versions', not newest 'new versions' stop processing.
1414+
// - The newest 'new version' assumes it is the latest version and updates all "older" 'new versions' to
1415+
// not be the latest.
1416+
// Age is determined by `judgetaskid`. The greatest `judgetaskid` is the newest. It might be possible that yet
1417+
// another thread is inserting versions. Though unfortunate there is no way to prevent this.
1418+
// Runs in a READ_UNCOMMITTED transaction to make sure concurrent threads see, and can update,
1419+
// required values.
1420+
$this->em->wrapInTransaction(function ($em) {
1421+
$isoLevel = $this->em->getConnection()->getTransactionIsolation();
1422+
$em->getConnection()->setTransactionIsolation(TransactionIsolationLevel::READ_UNCOMMITTED);
1423+
1424+
// Restore the isolation level.
1425+
$em->getConnection()->setTransactionIsolation($isoLevel);
1426+
});
1427+
13491428
$this->em->wrapInTransaction(function () use (
13501429
$judgehost,
13511430
$reportedVersions,
@@ -1355,10 +1434,12 @@ public function checkVersions(Request $request, string $judgetaskid): array
13551434
$activeVersion = $this->em->getRepository(Version::class)
13561435
->findOneBy(['language' => $language, 'judgehost' => $judgehost, 'active' => true]);
13571436

1358-
$isNewVersion = false;
1359-
if (!$activeVersion) {
1360-
$isNewVersion = true;
1361-
} else {
1437+
$isNewVersion = !$activeVersion ||
1438+
$activeVersion->getCompilerVersion() !== @$reportedVersions['compiler'] ||
1439+
$activeVersion->getRunnerVersion() !== @$reportedVersions['runner'];
1440+
1441+
$isNewVersion = !$activeVersion;
1442+
if (!$isNewVersion) {
13621443
$reportedCompilerVersion = $reportedVersions['compiler'] ?? null;
13631444
if ($activeVersion->getCompilerVersion() !== $reportedCompilerVersion) {
13641445
$isNewVersion = true;

0 commit comments

Comments
 (0)