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.
This commit is contained in:
Eelco Dolstra 2015-06-09 15:03:20 +02:00
parent 61d4060522
commit c93aa92563

View file

@ -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, 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) 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(); 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<int>() + 1; int stepNr = res[0][0].is_null() ? 1 : res[0][0].as<int>() + 1;
@ -892,13 +896,13 @@ void State::doBuildStep(std::shared_ptr<StoreAPI> store, Step::ptr step,
} else { } else {
/* Create failed build steps for every build that depends /* Create failed build steps for every build that depends
on this. */ on this. */
finishBuildStep(txn, result.startTime, result.stopTime, build->id, stepNr, machine->sshName, bssFailed, result.errorMsg);
for (auto build2 : dependents) { for (auto build2 : dependents) {
if (build == build2) continue; if (build == build2) continue;
createBuildStep(txn, result.stopTime, build2, step, machine->sshName, bssFailed, result.errorMsg, build->id); 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. */ /* Mark all builds that depend on this derivation as failed. */
for (auto build2 : dependents) { for (auto build2 : dependents) {
printMsg(lvlError, format("marking build %1% as failed") % build2->id); printMsg(lvlError, format("marking build %1% as failed") % build2->id);