From c93aa925631484dca9a0fadf494596f35af04105 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 9 Jun 2015 15:03:20 +0200 Subject: [PATCH] Create BuildSteps race-free If multiple threads create a step for the same build, they could get the same "max(stepnr)" and allocate conflicting new step numbers. So lock the BuildSteps table while doing this. We could use a different isolation level, but this is easier. --- src/hydra-queue-runner/hydra-queue-runner.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/hydra-queue-runner/hydra-queue-runner.cc b/src/hydra-queue-runner/hydra-queue-runner.cc index 1c3ab547..4f1fc48c 100644 --- a/src/hydra-queue-runner/hydra-queue-runner.cc +++ b/src/hydra-queue-runner/hydra-queue-runner.cc @@ -358,6 +358,10 @@ void State::clearBusy(time_t stopTime) int State::createBuildStep(pqxx::work & txn, time_t startTime, Build::ptr build, Step::ptr step, const std::string & machine, BuildStepStatus status, const std::string & errorMsg, BuildID propagatedFrom) { + /* Acquire an exclusive lock on BuildSteps to ensure that we don't + race with other threads creating a step of the same build. */ + txn.exec("lock table BuildSteps in exclusive mode"); + auto res = txn.parameterized("select max(stepnr) from BuildSteps where build = $1")(build->id).exec(); int stepNr = res[0][0].is_null() ? 1 : res[0][0].as() + 1; @@ -892,13 +896,13 @@ void State::doBuildStep(std::shared_ptr store, Step::ptr step, } else { /* Create failed build steps for every build that depends on this. */ - finishBuildStep(txn, result.startTime, result.stopTime, build->id, stepNr, machine->sshName, bssFailed, result.errorMsg); - for (auto build2 : dependents) { if (build == build2) continue; createBuildStep(txn, result.stopTime, build2, step, machine->sshName, bssFailed, result.errorMsg, build->id); } + finishBuildStep(txn, result.startTime, result.stopTime, build->id, stepNr, machine->sshName, bssFailed, result.errorMsg); + /* Mark all builds that depend on this derivation as failed. */ for (auto build2 : dependents) { printMsg(lvlError, format("marking build %1% as failed") % build2->id);