Skip to content

Commit fca8b5a

Browse files
author
epriestley
committed
Improve UX for repository updates
Summary: Fixes T5926. Fixes T5830. Ref T4767. Users currently sometimes have a hard time understanding repository update frequencies. This is compounded by aggressive backoff and incorrect backoff while importing repositories. - Don't back off while importing repositories. This prevents us from hanging at 99.99% for inactive repositories while waiting for the next update. - Back off less aggressively in general, and even more gradually during the first 3 days. This should make behavior around weekends better. - Show update frequency in the UI. - Provide an explicit "update now" button to call `diffusion.looksoon` in a more user-friendly way. - Document how backoff policies work and how to adjust behavior. Test Plan: - Ran `bin/phd debug pulllocal` and verified backoff worked correctly from debugging output. - Clicked "Update Now" to get a hint, reloaded page to see it update. - Read documentation. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4767, T5830, T5926 Differential Revision: https://secure.phabricator.com/D10323
1 parent d122d9e commit fca8b5a

File tree

7 files changed

+297
-49
lines changed

7 files changed

+297
-49
lines changed

‎src/__phutil_library_map__.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@
475475
'DiffusionRepositoryEditLocalController' => 'applications/diffusion/controller/DiffusionRepositoryEditLocalController.php',
476476
'DiffusionRepositoryEditMainController' => 'applications/diffusion/controller/DiffusionRepositoryEditMainController.php',
477477
'DiffusionRepositoryEditSubversionController' => 'applications/diffusion/controller/DiffusionRepositoryEditSubversionController.php',
478+
'DiffusionRepositoryEditUpdateController' => 'applications/diffusion/controller/DiffusionRepositoryEditUpdateController.php',
478479
'DiffusionRepositoryListController' => 'applications/diffusion/controller/DiffusionRepositoryListController.php',
479480
'DiffusionRepositoryNewController' => 'applications/diffusion/controller/DiffusionRepositoryNewController.php',
480481
'DiffusionRepositoryPath' => 'applications/diffusion/data/DiffusionRepositoryPath.php',
@@ -3223,6 +3224,7 @@
32233224
'DiffusionRepositoryEditLocalController' => 'DiffusionRepositoryEditController',
32243225
'DiffusionRepositoryEditMainController' => 'DiffusionRepositoryEditController',
32253226
'DiffusionRepositoryEditSubversionController' => 'DiffusionRepositoryEditController',
3227+
'DiffusionRepositoryEditUpdateController' => 'DiffusionRepositoryEditController',
32263228
'DiffusionRepositoryListController' => 'DiffusionController',
32273229
'DiffusionRepositoryNewController' => 'DiffusionController',
32283230
'DiffusionRepositoryRef' => 'Phobject',

‎src/applications/diffusion/application/PhabricatorDiffusionApplication.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ public function getRoutes() {
9292
'delete/' => 'DiffusionRepositoryEditDeleteController',
9393
'hosting/' => 'DiffusionRepositoryEditHostingController',
9494
'(?P<serve>serve)/' => 'DiffusionRepositoryEditHostingController',
95+
'update/' => 'DiffusionRepositoryEditUpdateController',
9596
),
9697
'pathtree/(?P<dblob>.*)' => 'DiffusionPathTreeController',
9798
'mirror/' => array(

‎src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ private function buildBasicActions(PhabricatorRepository $repository) {
207207
->setHref($this->getRepositoryControllerURI($repository, 'edit/basic/'));
208208
$view->addAction($edit);
209209

210+
$edit = id(new PhabricatorActionView())
211+
->setIcon('fa-refresh')
212+
->setName(pht('Update Now'))
213+
->setWorkflow(true)
214+
->setHref(
215+
$this->getRepositoryControllerURI($repository, 'edit/update/'));
216+
$view->addAction($edit);
217+
210218
$activate = id(new PhabricatorActionView())
211219
->setHref(
212220
$this->getRepositoryControllerURI($repository, 'edit/activate/'))
@@ -280,6 +288,10 @@ private function buildBasicProperties(
280288
pht('Status'),
281289
$this->buildRepositoryStatus($repository));
282290

291+
$view->addProperty(
292+
pht('Update Frequency'),
293+
$this->buildRepositoryUpdateInterval($repository));
294+
283295
$description = $repository->getDetail('description');
284296
$view->addSectionHeader(pht('Description'));
285297
if (!strlen($description)) {
@@ -942,14 +954,16 @@ private function buildRepositoryStatus(
942954
->setNote($message->getParameter('message')));
943955
return $view;
944956
case PhabricatorRepositoryStatusMessage::CODE_OKAY:
957+
$ago = (PhabricatorTime::getNow() - $message->getEpoch());
945958
$view->addItem(
946959
id(new PHUIStatusItemView())
947960
->setIcon(PHUIStatusItemView::ICON_ACCEPT, 'green')
948961
->setTarget(pht('Updates OK'))
949962
->setNote(
950963
pht(
951-
'Last updated %s.',
952-
phabricator_datetime($message->getEpoch(), $viewer))));
964+
'Last updated %s (%s ago).',
965+
phabricator_datetime($message->getEpoch(), $viewer),
966+
phutil_format_relative_time_detailed($ago))));
953967
break;
954968
}
955969
} else {
@@ -1020,12 +1034,34 @@ private function buildRepositoryStatus(
10201034
id(new PHUIStatusItemView())
10211035
->setIcon(PHUIStatusItemView::ICON_UP, 'indigo')
10221036
->setTarget(pht('Prioritized'))
1023-
->setNote(pht('This repository will be updated soon.')));
1037+
->setNote(pht('This repository will be updated soon!')));
10241038
}
10251039

10261040
return $view;
10271041
}
10281042

1043+
private function buildRepositoryUpdateInterval(
1044+
PhabricatorRepository $repository) {
1045+
1046+
$smart_wait = $repository->loadUpdateInterval();
1047+
1048+
$doc_href = PhabricatorEnv::getDoclink(
1049+
'Diffusion User Guide: Repository Updates');
1050+
1051+
return array(
1052+
phutil_format_relative_time_detailed($smart_wait),
1053+
" \xC2\xB7 ",
1054+
phutil_tag(
1055+
'a',
1056+
array(
1057+
'href' => $doc_href,
1058+
'target' => '_blank',
1059+
),
1060+
pht('Learn More')),
1061+
);
1062+
}
1063+
1064+
10291065
private function buildMirrorActions(
10301066
PhabricatorRepository $repository) {
10311067

‎src/applications/diffusion/controller/DiffusionRepositoryEditUpdateController.php‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
3+
final class DiffusionRepositoryEditUpdateController
4+
extends DiffusionRepositoryEditController {
5+
6+
public function processRequest() {
7+
$request = $this->getRequest();
8+
$viewer = $request->getUser();
9+
$drequest = $this->diffusionRequest;
10+
$repository = $drequest->getRepository();
11+
12+
$repository = id(new PhabricatorRepositoryQuery())
13+
->setViewer($viewer)
14+
->requireCapabilities(
15+
array(
16+
PhabricatorPolicyCapability::CAN_VIEW,
17+
PhabricatorPolicyCapability::CAN_EDIT,
18+
))
19+
->withIDs(array($repository->getID()))
20+
->executeOne();
21+
if (!$repository) {
22+
return new Aphront404Response();
23+
}
24+
25+
$edit_uri = $this->getRepositoryControllerURI($repository, 'edit/');
26+
27+
if ($request->isFormPost()) {
28+
$params = array(
29+
'callsigns' => array(
30+
$repository->getCallsign(),
31+
),
32+
);
33+
34+
id(new ConduitCall('diffusion.looksoon', $params))
35+
->setUser($viewer)
36+
->execute();
37+
38+
return id(new AphrontRedirectResponse())->setURI($edit_uri);
39+
}
40+
41+
$doc_name = 'Diffusion User Guide: Repository Updates';
42+
$doc_href = PhabricatorEnv::getDoclink($doc_name);
43+
$doc_link = phutil_tag(
44+
'a',
45+
array(
46+
'href' => $doc_href,
47+
'target' => '_blank',
48+
),
49+
$doc_name);
50+
51+
return $this->newDialog()
52+
->setTitle(pht('Update Repository Now'))
53+
->appendParagraph(
54+
pht(
55+
'Normally, Phabricator automatically updates repositories '.
56+
'based on how much time has elapsed since the last commit. '.
57+
'This helps reduce load if you have a large number of mostly '.
58+
'inactive repositories, which is common.'))
59+
->appendParagraph(
60+
pht(
61+
'You can manually schedule an update for this repository. The '.
62+
'daemons will perform the update as soon as possible. This may '.
63+
'be helpful if you have just made a commit to a rarely used '.
64+
'repository.'))
65+
->appendParagraph(
66+
pht(
67+
'To learn more about how Phabricator updates repositories, '.
68+
'read %s in the documentation.',
69+
$doc_link))
70+
->addCancelButton($edit_uri)
71+
->addSubmitButton(pht('Schedule Update'));
72+
}
73+
74+
75+
}

‎src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php‎

Lines changed: 14 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -345,55 +345,23 @@ private function resolveUpdateFuture(
345345
phlog($stderr_msg);
346346
}
347347

348-
$sleep_for = (int)$repository->getDetail('pull-frequency', $min_sleep);
349-
350-
// Smart wait: pull rarely used repositories less frequently. Find the
351-
// most recent commit which is older than the current time (this keeps us
352-
// from spinning on repositories with a silly commit post-dated to some time
353-
// in 2037), and adjust how frequently we pull based on how frequently this
354-
// repository updates.
355-
356-
$table = id(new PhabricatorRepositoryCommit());
357-
$last_commit = queryfx_one(
358-
$table->establishConnection('w'),
359-
'SELECT epoch FROM %T
360-
WHERE repositoryID = %d AND epoch <= %d
361-
ORDER BY epoch DESC LIMIT 1',
362-
$table->getTableName(),
363-
$repository->getID(),
364-
time() + $min_sleep);
365-
if ($last_commit) {
366-
$time_since_commit = (time() + $min_sleep) - $last_commit['epoch'];
367-
368-
// Wait 0.5% of the time since the last commit before we pull. This gives
369-
// us these wait times:
370-
//
371-
// 50 minutes or less: 15 seconds
372-
// about 3 hours: 1 minute
373-
// about 16 hours: 5 minutes
374-
// about 2 days: 15 minutes
375-
// 50 days or more: 6 hours
376-
377-
$smart_wait = ($time_since_commit / 200);
378-
$smart_wait = min($smart_wait, phutil_units('6 hours in seconds'));
348+
// For now, continue respecting this deprecated setting for raising the
349+
// minimum pull frequency.
350+
// TODO: Remove this some day once this code has been completely stable
351+
// for a while.
352+
$sleep_for = (int)$repository->getDetail('pull-frequency');
353+
$min_sleep = max($sleep_for, $min_sleep);
379354

380-
$this->log(
381-
pht(
382-
'Last commit to repository "%s" was %s seconds ago; considering '.
383-
'a wait of %s seconds before update.',
384-
$repository->getMonogram(),
385-
new PhutilNumber($time_since_commit),
386-
new PhutilNumber($smart_wait)));
387-
388-
$smart_wait = max(15, $smart_wait);
389-
$sleep_for = max($smart_wait, $sleep_for);
390-
}
355+
$smart_wait = $repository->loadUpdateInterval($min_sleep);
391356

392-
if ($sleep_for < $min_sleep) {
393-
$sleep_for = $min_sleep;
394-
}
357+
$this->log(
358+
pht(
359+
'Based on activity in repository "%s", considering a wait of %s '.
360+
'seconds before update.',
361+
$repository->getMonogram(),
362+
new PhutilNumber($smart_wait)));
395363

396-
return time() + $sleep_for;
364+
return time() + $smart_wait;
397365
}
398366

399367

‎src/applications/repository/storage/PhabricatorRepository.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1319,6 +1319,69 @@ public static function assertValidRemoteURI($uri) {
13191319
}
13201320

13211321

1322+
/**
1323+
* Load the pull frequency for this repository, based on the time since the
1324+
* last activity.
1325+
*
1326+
* We pull rarely used repositories less frequently. This finds the most
1327+
* recent commit which is older than the current time (which prevents us from
1328+
* spinning on repositories with a silly commit post-dated to some time in
1329+
* 2037). We adjust the pull frequency based on when the most recent commit
1330+
* occurred.
1331+
*
1332+
* @param int The minimum update interval to use, in seconds.
1333+
* @return int Repository update interval, in seconds.
1334+
*/
1335+
public function loadUpdateInterval($minimum = 15) {
1336+
// If a repository is still importing, always pull it as frequently as
1337+
// possible. This prevents us from hanging for a long time at 99.9% when
1338+
// importing an inactive repository.
1339+
if ($this->isImporting()) {
1340+
return $minimum;
1341+
}
1342+
1343+
$window_start = (PhabricatorTime::getNow() + $minimum);
1344+
1345+
$table = id(new PhabricatorRepositoryCommit());
1346+
$last_commit = queryfx_one(
1347+
$table->establishConnection('r'),
1348+
'SELECT epoch FROM %T
1349+
WHERE repositoryID = %d AND epoch <= %d
1350+
ORDER BY epoch DESC LIMIT 1',
1351+
$table->getTableName(),
1352+
$this->getID(),
1353+
$window_start);
1354+
if ($last_commit) {
1355+
$time_since_commit = ($window_start - $last_commit['epoch']);
1356+
1357+
$last_few_days = phutil_units('3 days in seconds');
1358+
1359+
if ($time_since_commit <= $last_few_days) {
1360+
// For repositories with activity in the recent past, we wait one
1361+
// extra second for every 10 minutes since the last commit. This
1362+
// shorter backoff is intended to handle weekends and other short
1363+
// breaks from development.
1364+
$smart_wait = ($time_since_commit / 600);
1365+
} else {
1366+
// For repositories without recent activity, we wait one extra second
1367+
// for every 4 minutes since the last commit. This longer backoff
1368+
// handles rarely used repositories, up to the maximum.
1369+
$smart_wait = ($time_since_commit / 240);
1370+
}
1371+
1372+
// We'll never wait more than 6 hours to pull a repository.
1373+
$longest_wait = phutil_units('6 hours in seconds');
1374+
$smart_wait = min($smart_wait, $longest_wait);
1375+
1376+
$smart_wait = max($minimum, $smart_wait);
1377+
} else {
1378+
$smart_wait = $minimum;
1379+
}
1380+
1381+
return $smart_wait;
1382+
}
1383+
1384+
13221385
/* -( PhabricatorPolicyInterface )----------------------------------------- */
13231386

13241387

0 commit comments

Comments
 (0)