Skip to content

Commit b12e92e

Browse files
author
epriestley
committed
Add timing information for commit hooks to push logs
Summary: Depends on D19779. Ref T13216. The push logs currently record the "hostWait", which is roughly "locking + subprocess cost". We also record locking separately, so we can figure out "subprocess cost" alone by subtracting the lock costs. However, the subprocess (normally `git receive-pack`) runs hooks, and we don't have an easy way to figure out how much time was spent doing actual `git` stuff vs spent doing commit hook processing. This would have been useful in diagnosing at least one recent issue. Track at least a rough hook cost and record it in the push logs. Test Plan: Pushed to a repository, saw a reasonable hook cost appear in the database table. Reviewers: amckinley Reviewed By: amckinley Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13216 Differential Revision: https://secure.phabricator.com/D19780
1 parent 966db4d commit b12e92e

File tree

4 files changed

+23
-2
lines changed

4 files changed

+23
-2
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE {$NAMESPACE}_repository.repository_pushevent
2+
ADD hookWait BIGINT UNSIGNED;

scripts/repository/commit_hook.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
// subclasses of PhabricatorConfigSiteSource to read it and build an instance
1818
// environment.
1919

20+
$hook_start = microtime(true);
21+
2022
if ($argc > 1) {
2123
$context = $argv[1];
2224
$context = explode(':', $context, 2);
@@ -35,7 +37,8 @@
3537
throw new Exception(pht('usage: commit-hook <repository>'));
3638
}
3739

38-
$engine = new DiffusionCommitHookEngine();
40+
$engine = id(new DiffusionCommitHookEngine())
41+
->setStartTime($hook_start);
3942

4043
$repository = id(new PhabricatorRepositoryQuery())
4144
->setViewer(PhabricatorUser::getOmnipotentUser())

src/applications/diffusion/engine/DiffusionCommitHookEngine.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ final class DiffusionCommitHookEngine extends Phobject {
3131
private $mercurialHook;
3232
private $mercurialCommits = array();
3333
private $gitCommits = array();
34+
private $startTime;
3435

3536
private $heraldViewerProjects;
3637
private $rejectCode = PhabricatorRepositoryPushLog::REJECT_BROKEN;
@@ -70,6 +71,15 @@ public function getRequestIdentifier() {
7071
return $this->requestIdentifier;
7172
}
7273

74+
public function setStartTime($start_time) {
75+
$this->startTime = $start_time;
76+
return $this;
77+
}
78+
79+
public function getStartTime() {
80+
return $this->startTime;
81+
}
82+
7383
public function setSubversionTransactionInfo($transaction, $repository) {
7484
$this->subversionTransaction = $transaction;
7585
$this->subversionRepository = $repository;
@@ -1102,11 +1112,15 @@ private function newPushLog() {
11021112
private function newPushEvent() {
11031113
$viewer = $this->getViewer();
11041114

1115+
$hook_start = $this->getStartTime();
1116+
$hook_end = microtime(true);
1117+
11051118
$event = PhabricatorRepositoryPushEvent::initializeNewEvent($viewer)
11061119
->setRepositoryPHID($this->getRepository()->getPHID())
11071120
->setRemoteAddress($this->getRemoteAddress())
11081121
->setRemoteProtocol($this->getRemoteProtocol())
1109-
->setEpoch(PhabricatorTime::getNow());
1122+
->setEpoch(PhabricatorTime::getNow())
1123+
->setHookWait((int)(1000000 * ($hook_end - $hook_start)));
11101124

11111125
$identifier = $this->getRequestIdentifier();
11121126
if (strlen($identifier)) {

src/applications/repository/storage/PhabricatorRepositoryPushEvent.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ final class PhabricatorRepositoryPushEvent
1919
protected $writeWait;
2020
protected $readWait;
2121
protected $hostWait;
22+
protected $hookWait;
2223

2324
private $repository = self::ATTACHABLE;
2425
private $logs = self::ATTACHABLE;
@@ -41,6 +42,7 @@ protected function getConfiguration() {
4142
'writeWait' => 'uint64?',
4243
'readWait' => 'uint64?',
4344
'hostWait' => 'uint64?',
45+
'hookWait' => 'uint64?',
4446
),
4547
self::CONFIG_KEY_SCHEMA => array(
4648
'key_repository' => array(

0 commit comments

Comments
 (0)