From 80aa7d4c06f8e1d4f2284a21564f29fbc51b02ab Mon Sep 17 00:00:00 2001 From: Stepan Strelets Date: Tue, 4 Apr 2017 23:12:20 +0300 Subject: [PATCH] Fix build execute - in some cases one build executed with many process --- src/PHPCensor/Builder.php | 10 ++++++- src/PHPCensor/BuilderException.php | 8 +++++ src/PHPCensor/Command/RunCommand.php | 44 ++++++++++++++++++---------- src/PHPCensor/Model/Build.php | 21 +++++++++++++ src/PHPCensor/Store/BuildStore.php | 20 +++++++++++++ src/PHPCensor/Worker/BuildWorker.php | 37 ++++++++++++++++------- 6 files changed, 114 insertions(+), 26 deletions(-) create mode 100644 src/PHPCensor/BuilderException.php diff --git a/src/PHPCensor/Builder.php b/src/PHPCensor/Builder.php index 63113baf..7ac1ec10 100644 --- a/src/PHPCensor/Builder.php +++ b/src/PHPCensor/Builder.php @@ -175,8 +175,16 @@ class Builder implements LoggerAwareInterface */ public function execute() { + // check current status + if ($this->build->getStatus() != Build::STATUS_PENDING) { + throw new BuilderException('Can`t build - status is not pending', BuilderException::FAIL_START); + } + // set status only if current status pending + if (!$this->build->setStatusSync(Build::STATUS_RUNNING)) { + throw new BuilderException('Can`t build - unable change status to running', BuilderException::FAIL_START); + } + // Update the build in the database, ping any external services. - $this->build->setStatus(Build::STATUS_RUNNING); $this->build->setStarted(new \DateTime()); $this->store->save($this->build); $this->build->sendStatusPostback(); diff --git a/src/PHPCensor/BuilderException.php b/src/PHPCensor/BuilderException.php new file mode 100644 index 00000000..04dedccd --- /dev/null +++ b/src/PHPCensor/BuilderException.php @@ -0,0 +1,8 @@ +getProjectId(), $running)) { + if (!empty($running[$build->getProjectId()])) { $this->logger->addInfo(sprintf('Skipping Build %d - Project build already in progress.', $build->getId())); - $result['items'][] = $build; - - // Re-run build validator: - $running = $this->validateRunningBuilds(); continue; } $builds++; - try { - // Logging relevant to this build should be stored - // against the build itself. - $buildDbLog = new BuildDBLogHandler($build, Logger::INFO); - $this->logger->pushHandler($buildDbLog); + // Logging relevant to this build should be stored + // against the build itself. + $buildDbLog = new BuildDBLogHandler($build, Logger::INFO); + $this->logger->pushHandler($buildDbLog); + try { $builder = new Builder($build, $this->logger); $builder->execute(); - // After execution we no longer want to record the information - // back to this specific build so the handler should be removed. - $this->logger->popHandler(); - // destructor implicitly call flush - unset($buildDbLog); + } catch (BuilderException $ex) { + $this->logger->addError($ex->getMessage()); + switch($ex->getCode()) { + case BuilderException::FAIL_START: + // non fatal + break; + default: + $build->setStatus(Build::STATUS_FAILED); + $build->setFinished(new \DateTime()); + $build->setLog($build->getLog() . PHP_EOL . PHP_EOL . $ex->getMessage()); + $store->save($build); + break; + } + } catch (\Exception $ex) { $build->setStatus(Build::STATUS_FAILED); $build->setFinished(new \DateTime()); @@ -127,6 +133,14 @@ class RunCommand extends Command $store->save($build); } + // After execution we no longer want to record the information + // back to this specific build so the handler should be removed. + $this->logger->popHandler(); + // destructor implicitly call flush + unset($buildDbLog); + + // Re-run build validator: + $running = $this->validateRunningBuilds(); } $this->logger->addInfo('Finished processing builds.'); diff --git a/src/PHPCensor/Model/Build.php b/src/PHPCensor/Model/Build.php index 0847847c..a4b6aced 100644 --- a/src/PHPCensor/Model/Build.php +++ b/src/PHPCensor/Model/Build.php @@ -410,6 +410,27 @@ class Build extends Model $this->setModified('status'); } + /** + * Set the value of Status / status only if it synced with db. Must not be null. + * + * @param $value int + * @return bool + */ + public function setStatusSync($value) + { + $this->validateNotNull('Status', $value); + $this->validateInt('Status', $value); + + if ($this->data['status'] !== $value) { + $store = Factory::getStore('Build'); + if ($store->updateStatusSync($this, $value)) { + $this->data['status'] = $value; + return true; + } + } + return false; + } + /** * Set the value of Log / log. * diff --git a/src/PHPCensor/Store/BuildStore.php b/src/PHPCensor/Store/BuildStore.php index ed593eac..793d9a5f 100644 --- a/src/PHPCensor/Store/BuildStore.php +++ b/src/PHPCensor/Store/BuildStore.php @@ -310,4 +310,24 @@ class BuildStore extends Store return false; } } + + /** + * Update status only if it synced with db + * @param Build $build + * @param int $status + * @return bool + */ + public function updateStatusSync($build, $status) + { + try { + $query = 'UPDATE {{build}} SET status = :status_new WHERE {{id}} = :id AND {{status}} = :status_current'; + $stmt = Database::getConnection('write')->prepareCommon($query); + $stmt->bindValue(':id', $build->getId(), \PDO::PARAM_INT); + $stmt->bindValue(':status_current', $build->getStatus(), \PDO::PARAM_INT); + $stmt->bindValue(':status_new', $status, \PDO::PARAM_INT); + return ($stmt->execute() and ($stmt->rowCount() == 1)); + } catch (\Exception $e) { + return false; + } + } } diff --git a/src/PHPCensor/Worker/BuildWorker.php b/src/PHPCensor/Worker/BuildWorker.php index 326b3258..3b492513 100644 --- a/src/PHPCensor/Worker/BuildWorker.php +++ b/src/PHPCensor/Worker/BuildWorker.php @@ -9,6 +9,7 @@ use Monolog\Logger; use Pheanstalk\Job; use Pheanstalk\Pheanstalk; use PHPCensor\Builder; +use PHPCensor\BuilderException; use PHPCensor\BuildFactory; use PHPCensor\Logging\BuildDBLogHandler; use PHPCensor\Model\Build; @@ -109,20 +110,30 @@ class BuildWorker $this->pheanstalk->delete($job); } - try { - // Logging relevant to this build should be stored - // against the build itself. - $buildDbLog = new BuildDBLogHandler($build, Logger::INFO); - $this->logger->pushHandler($buildDbLog); + // Logging relevant to this build should be stored + // against the build itself. + $buildDbLog = new BuildDBLogHandler($build, Logger::INFO); + $this->logger->pushHandler($buildDbLog); + try { $builder = new Builder($build, $this->logger); $builder->execute(); - // After execution we no longer want to record the information - // back to this specific build so the handler should be removed. - $this->logger->popHandler(); - // destructor implicitly call flush - unset($buildDbLog); + } catch (BuilderException $ex) { + $this->logger->addError($ex->getMessage()); + switch($ex->getCode()) { + case BuilderException::FAIL_START: + // non fatal + $this->pheanstalk->release($job); + break; + default: + $build->setStatus(Build::STATUS_FAILED); + $build->setFinished(new \DateTime()); + $build->setLog($build->getLog() . PHP_EOL . PHP_EOL . $ex->getMessage()); + $store->save($build); + $build->sendStatusPostback(); + break; + } } catch (\PDOException $ex) { // If we've caught a PDO Exception, it is probably not the fault of the build, but of a failed // connection or similar. Release the job and kill the worker. @@ -136,6 +147,12 @@ class BuildWorker $build->sendStatusPostback(); } + // After execution we no longer want to record the information + // back to this specific build so the handler should be removed. + $this->logger->popHandler(); + // destructor implicitly call flush + unset($buildDbLog); + // Reset the config back to how it was prior to running this job: if (!empty($currentConfig)) { Database::reset();