Differential D21622 Diff 51468 src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
Changeset View
Changeset View
Standalone View
Standalone View
src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
| Show First 20 Lines • Show All 87 Lines • ▼ Show 20 Lines | if (!$issuable) { | ||||
| return; | return; | ||||
| } | } | ||||
| $actor = $this->getActor(); | $actor = $this->getActor(); | ||||
| if (!$build->canIssueCommand($actor, $command)) { | if (!$build->canIssueCommand($actor, $command)) { | ||||
| return; | return; | ||||
| } | } | ||||
| id(new HarbormasterBuildCommand()) | // establish separate connection to avoid HarbormasterBuildCommand | ||||
| // being created in transaction which already is happening, otherwise | |||||
| // Worker can be started before HarbormasterBuildCommand is committed | |||||
| $conn = id(new HarbormasterBuildCommand()) | |||||
| ->establishConnection('w', $force_new = true); | |||||
| $build_command = id(new HarbormasterBuildCommand()) | |||||
| ->setForcedConnection($conn) | |||||
| ->setAuthorPHID($xaction->getAuthorPHID()) | ->setAuthorPHID($xaction->getAuthorPHID()) | ||||
| ->setTargetPHID($build->getPHID()) | ->setTargetPHID($build->getPHID()) | ||||
| ->setCommand($command) | ->setCommand($command) | ||||
| ->save(); | ->save(); | ||||
| try { | |||||
| PhabricatorWorker::scheduleTask( | PhabricatorWorker::scheduleTask( | ||||
| 'HarbormasterBuildWorker', | 'HarbormasterBuildWorker', | ||||
| array( | array( | ||||
| 'buildID' => $build->getID(), | 'buildID' => $build->getID(), | ||||
| ), | ), | ||||
| array( | array( | ||||
| 'objectPHID' => $build->getPHID(), | 'objectPHID' => $build->getPHID(), | ||||
| )); | )); | ||||
artms: Leaking of commands and "no longer" restartable test job can be reproduced by adding sleep here. | |||||
| } catch (Exception $e) { | |||||
| // if we can't schedule task, let's delete command | |||||
| $build_command->delete(); | |||||
| throw $e; | |||||
| } catch (Throwable $e) { | |||||
| // if we can't schedule task, let's delete command | |||||
| $build_command->delete(); | |||||
| throw $e; | |||||
| } | |||||
| } | } | ||||
| protected function applyCustomExternalTransaction( | protected function applyCustomExternalTransaction( | ||||
| PhabricatorLiskDAO $object, | PhabricatorLiskDAO $object, | ||||
| PhabricatorApplicationTransaction $xaction) { | PhabricatorApplicationTransaction $xaction) { | ||||
| switch ($xaction->getTransactionType()) { | switch ($xaction->getTransactionType()) { | ||||
| case HarbormasterBuildTransaction::TYPE_CREATE: | case HarbormasterBuildTransaction::TYPE_CREATE: | ||||
| case HarbormasterBuildTransaction::TYPE_COMMAND: | case HarbormasterBuildTransaction::TYPE_COMMAND: | ||||
| return; | return; | ||||
| } | } | ||||
| return parent::applyCustomExternalTransaction($object, $xaction); | return parent::applyCustomExternalTransaction($object, $xaction); | ||||
| } | } | ||||
| } | } | ||||
Leaking of commands and "no longer" restartable test job can be reproduced by adding sleep here.
And after you restart job you'll see that HarbormasterBuildWorker is created in worker_activetask but build command is not created in harbomaster_buildcommand table. This causes HarbormasterBuildWorker to bail out quite soon (no command to execute) and once transaction is fully commited - build command is created which essentially locks build in 'restarting' phase which can only be fixed by adding new diff to revision