Page MenuHomePhabricator
Files F15436910

D16131.id38807.diff
No OneTemporary
Actions

D16131.id38807.diff
View Options

diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php
@@ -514,10 +514,16 @@
$repository->getID(),
$commit_identifier);
if ($conn_w->getAffectedRows()) {
+ $commit = $commit->loadOneWhere(
+ 'repositoryID = %d AND commitIdentifier = %s',
+ $repository->getID(),
+ $commit_identifier);
+
+ // After reviving a commit, schedule new daemons for it.
+ $this->didDiscoverCommit($repository, $commit, $epoch);
return;
}
-
$commit->setRepositoryID($repository->getID());
$commit->setCommitIdentifier($commit_identifier);
$commit->setEpoch($epoch);
@@ -575,21 +581,7 @@
}
$commit->saveTransaction();
- $this->insertTask($repository, $commit);
-
- queryfx(
- $conn_w,
- 'INSERT INTO %T (repositoryID, size, lastCommitID, epoch)
- VALUES (%d, 1, %d, %d)
- ON DUPLICATE KEY UPDATE
- size = size + 1,
- lastCommitID =
- IF(VALUES(epoch) > epoch, VALUES(lastCommitID), lastCommitID),
- epoch = IF(VALUES(epoch) > epoch, VALUES(epoch), epoch)',
- PhabricatorRepository::TABLE_SUMMARY,
- $repository->getID(),
- $commit->getID(),
- $epoch);
+ $this->didDiscoverCommit($repository, $commit, $epoch);
if ($this->repairMode) {
// Normally, the query should throw a duplicate key exception. If we
@@ -614,6 +606,29 @@
}
}
+ private function didDiscoverCommit(
+ PhabricatorRepository $repository,
+ PhabricatorRepositoryCommit $commit,
+ $epoch) {
+
+ $this->insertTask($repository, $commit);
+
+ // Update the repository summary table.
+ queryfx(
+ $commit->establishConnection('w'),
+ 'INSERT INTO %T (repositoryID, size, lastCommitID, epoch)
+ VALUES (%d, 1, %d, %d)
+ ON DUPLICATE KEY UPDATE
+ size = size + 1,
+ lastCommitID =
+ IF(VALUES(epoch) > epoch, VALUES(lastCommitID), lastCommitID),
+ epoch = IF(VALUES(epoch) > epoch, VALUES(epoch), epoch)',
+ PhabricatorRepository::TABLE_SUMMARY,
+ $repository->getID(),
+ $commit->getID(),
+ $epoch);
+ }
+
private function didDiscoverRefs(array $refs) {
foreach ($refs as $ref) {
$this->workingSet[$ref->getIdentifier()] = true;
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php
@@ -3,6 +3,10 @@
final class PhabricatorRepositoryCommitHeraldWorker
extends PhabricatorRepositoryCommitParserWorker {
+ protected function getImportStepFlag() {
+ return PhabricatorRepositoryCommit::IMPORTED_HERALD;
+ }
+
public function getRequiredLeaseTime() {
// Herald rules may take a long time to process.
return phutil_units('4 hours in seconds');
@@ -12,6 +16,12 @@
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
+ if ($this->shouldSkipImportStep()) {
+ // This worker has no followup tasks, so we can just bail out
+ // right away without queueing anything.
+ return;
+ }
+
// Reload the commit to pull commit data and audit requests.
$commit = id(new DiffusionCommitQuery())
->setViewer(PhabricatorUser::getOmnipotentUser())
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php
@@ -3,14 +3,18 @@
final class PhabricatorRepositoryCommitOwnersWorker
extends PhabricatorRepositoryCommitParserWorker {
+ protected function getImportStepFlag() {
+ return PhabricatorRepositoryCommit::IMPORTED_OWNERS;
+ }
+
protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
- $this->triggerOwnerAudits($repository, $commit);
-
- $commit->writeImportStatusFlag(
- PhabricatorRepositoryCommit::IMPORTED_OWNERS);
+ if (!$this->shouldSkipImportStep()) {
+ $this->triggerOwnerAudits($repository, $commit);
+ $commit->writeImportStatusFlag($this->getImportStepFlag());
+ }
if ($this->shouldQueueFollowupTasks()) {
$this->queueTask(
diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
--- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
+++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php
@@ -26,6 +26,14 @@
pht('Commit "%s" does not exist.', $commit_id));
}
+ if ($commit->isUnreachable()) {
+ throw new PhabricatorWorkerPermanentFailureException(
+ pht(
+ 'Commit "%s" has been deleted: it is no longer reachable from '.
+ 'any ref.',
+ $commit_id));
+ }
+
$this->commit = $commit;
return $commit;
@@ -44,6 +52,42 @@
return !idx($this->getTaskData(), 'only');
}
+ protected function getImportStepFlag() {
+ return null;
+ }
+
+ final protected function shouldSkipImportStep() {
+ // If this step has already been performed and this is a "natural" task
+ // which was queued by the normal daemons, decline to do the work again.
+ // This mitigates races if commits are rapidly deleted and revived.
+ $flag = $this->getImportStepFlag();
+ if (!$flag) {
+ // This step doesn't have an associated flag.
+ return false;
+ }
+
+ $commit = $this->commit;
+ if (!$commit->isPartiallyImported($flag)) {
+ // This commit doesn't have the flag set yet.
+ return false;
+ }
+
+
+ if (!$this->shouldQueueFollowupTasks()) {
+ // This task was queued by administrative tools, so do the work even
+ // if it duplicates existing work.
+ return false;
+ }
+
+ $this->log(
+ "%s\n",
+ pht(
+ 'Skipping import step; this step was previously completed for '.
+ 'this commit.'));
+
+ return true;
+ }
+
abstract protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit);
diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
--- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
+++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php
@@ -3,6 +3,10 @@
abstract class PhabricatorRepositoryCommitChangeParserWorker
extends PhabricatorRepositoryCommitParserWorker {
+ protected function getImportStepFlag() {
+ return PhabricatorRepositoryCommit::IMPORTED_CHANGE;
+ }
+
public function getRequiredLeaseTime() {
// It can take a very long time to parse commits; some commits in the
// Facebook repository affect many millions of paths. Acquire 24h leases.
@@ -23,9 +27,15 @@
return;
}
- $results = $this->parseCommitChanges($repository, $commit);
- if ($results) {
- $this->writeCommitChanges($repository, $commit, $results);
+ if (!$this->shouldSkipImportStep()) {
+ $results = $this->parseCommitChanges($repository, $commit);
+ if ($results) {
+ $this->writeCommitChanges($repository, $commit, $results);
+ }
+
+ $commit->writeImportStatusFlag($this->getImportStepFlag());
+
+ PhabricatorSearchWorker::queueDocumentForIndexing($commit->getPHID());
}
$this->finishParse();
@@ -85,12 +95,6 @@
protected function finishParse() {
$commit = $this->commit;
-
- $commit->writeImportStatusFlag(
- PhabricatorRepositoryCommit::IMPORTED_CHANGE);
-
- PhabricatorSearchWorker::queueDocumentForIndexing($commit->getPHID());
-
if ($this->shouldQueueFollowupTasks()) {
$this->queueTask(
'PhabricatorRepositoryCommitOwnersWorker',
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php
@@ -3,42 +3,52 @@
abstract class PhabricatorRepositoryCommitMessageParserWorker
extends PhabricatorRepositoryCommitParserWorker {
- abstract protected function parseCommitWithRef(
- PhabricatorRepository $repository,
- PhabricatorRepositoryCommit $commit,
- DiffusionCommitRef $ref);
+ protected function getImportStepFlag() {
+ return PhabricatorRepositoryCommit::IMPORTED_MESSAGE;
+ }
+
+ abstract protected function getFollowupTaskClass();
final protected function parseCommit(
PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit) {
- $viewer = PhabricatorUser::getOmnipotentUser();
+ if (!$this->shouldSkipImportStep()) {
+ $viewer = PhabricatorUser::getOmnipotentUser();
- $refs_raw = DiffusionQuery::callConduitWithDiffusionRequest(
- $viewer,
- DiffusionRequest::newFromDictionary(
+ $refs_raw = DiffusionQuery::callConduitWithDiffusionRequest(
+ $viewer,
+ DiffusionRequest::newFromDictionary(
+ array(
+ 'repository' => $repository,
+ 'user' => $viewer,
+ )),
+ 'diffusion.querycommits',
array(
- 'repository' => $repository,
- 'user' => $viewer,
- )),
- 'diffusion.querycommits',
- array(
- 'repositoryPHID' => $repository->getPHID(),
- 'phids' => array($commit->getPHID()),
- 'bypassCache' => true,
- 'needMessages' => true,
- ));
-
- if (empty($refs_raw['data'])) {
- throw new Exception(
- pht(
- 'Unable to retrieve details for commit "%s"!',
- $commit->getPHID()));
- }
+ 'repositoryPHID' => $repository->getPHID(),
+ 'phids' => array($commit->getPHID()),
+ 'bypassCache' => true,
+ 'needMessages' => true,
+ ));
+
+ if (empty($refs_raw['data'])) {
+ throw new Exception(
+ pht(
+ 'Unable to retrieve details for commit "%s"!',
+ $commit->getPHID()));
+ }
- $ref = DiffusionCommitRef::newFromConduitResult(head($refs_raw['data']));
+ $ref = DiffusionCommitRef::newFromConduitResult(head($refs_raw['data']));
+ $this->updateCommitData($ref);
+ }
- $this->parseCommitWithRef($repository, $commit, $ref);
+ if ($this->shouldQueueFollowupTasks()) {
+ $this->queueTask(
+ $this->getFollowupTaskClass(),
+ array(
+ 'commitID' => $commit->getID(),
+ ));
+ }
}
final protected function updateCommitData(DiffusionCommitRef $ref) {
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php
@@ -3,20 +3,8 @@
final class PhabricatorRepositoryGitCommitMessageParserWorker
extends PhabricatorRepositoryCommitMessageParserWorker {
- protected function parseCommitWithRef(
- PhabricatorRepository $repository,
- PhabricatorRepositoryCommit $commit,
- DiffusionCommitRef $ref) {
-
- $this->updateCommitData($ref);
-
- if ($this->shouldQueueFollowupTasks()) {
- $this->queueTask(
- 'PhabricatorRepositoryGitCommitChangeParserWorker',
- array(
- 'commitID' => $commit->getID(),
- ));
- }
+ protected function getFollowupTaskClass() {
+ return 'PhabricatorRepositoryGitCommitChangeParserWorker';
}
}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryMercurialCommitMessageParserWorker.php
@@ -3,20 +3,8 @@
final class PhabricatorRepositoryMercurialCommitMessageParserWorker
extends PhabricatorRepositoryCommitMessageParserWorker {
- protected function parseCommitWithRef(
- PhabricatorRepository $repository,
- PhabricatorRepositoryCommit $commit,
- DiffusionCommitRef $ref) {
-
- $this->updateCommitData($ref);
-
- if ($this->shouldQueueFollowupTasks()) {
- $this->queueTask(
- 'PhabricatorRepositoryMercurialCommitChangeParserWorker',
- array(
- 'commitID' => $commit->getID(),
- ));
- }
+ protected function getFollowupTaskClass() {
+ return 'PhabricatorRepositoryMercurialCommitChangeParserWorker';
}
}
diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
--- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
+++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositorySvnCommitMessageParserWorker.php
@@ -3,20 +3,8 @@
final class PhabricatorRepositorySvnCommitMessageParserWorker
extends PhabricatorRepositoryCommitMessageParserWorker {
- protected function parseCommitWithRef(
- PhabricatorRepository $repository,
- PhabricatorRepositoryCommit $commit,
- DiffusionCommitRef $ref) {
-
- $this->updateCommitData($ref);
-
- if ($this->shouldQueueFollowupTasks()) {
- $this->queueTask(
- 'PhabricatorRepositorySvnCommitChangeParserWorker',
- array(
- 'commitID' => $commit->getID(),
- ));
- }
+ protected function getFollowupTaskClass() {
+ return 'PhabricatorRepositorySvnCommitChangeParserWorker';
}
}

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 26, 4:46 PM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7321648
Default Alt Text
D16131.id38807.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility