mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-18 04:42:40 +01:00
When repositories hit pull errors, stop updating them as frequently
Summary: Ref T11665. Currently, when a repository hits an error, we retry it after 15s. This is correct if the error was temporary/transient/config-related (e.g., bad network or administrator setting up credentials) but not so great if the error is long-lasting (completely bad authentication, invalid URI, etc), as it can pile up to a meaningful amount of unnecessary load over time. Instead, record how many times in a row we've hit an error and adjust backoff behavior: first error is 15s, then 30s, 45s, etc. Additionally, when computing the backoff for an empty repository, use the repository creation time as though it was the most recent commit. This is a good proxy which gives us reasonable backoff behavior. This required removing the `CODE_WORKING` messages, since they would have reset the error count. We could restore them (as a different type of message), but I think they aren't particularly useful since cloning usually doesn't take too long and there's more status information avilable now than there was when this stuff was written. Test Plan: - Ran `bin/phd debug pull`. - Saw sensible, increasing backoffs selected for repositories with errors. - Saw sensible backoffs selected for empty repositories. Reviewers: chad Maniphest Tasks: T11665 Differential Revision: https://secure.phabricator.com/D16575
This commit is contained in:
parent
e41a64607e
commit
d3280c406d
5 changed files with 73 additions and 37 deletions
2
resources/sql/autopatches/20160919.repo.messagecount.sql
Normal file
2
resources/sql/autopatches/20160919.repo.messagecount.sql
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
ALTER TABLE {$NAMESPACE}_repository.repository_statusmessage
|
||||||
|
ADD messageCount INT UNSIGNED NOT NULL;
|
|
@ -359,20 +359,13 @@ final class DiffusionRepositoryStatusManagementPanel
|
||||||
return $view;
|
return $view;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
case PhabricatorRepositoryStatusMessage::CODE_WORKING:
|
default:
|
||||||
$view->addItem(
|
$view->addItem(
|
||||||
id(new PHUIStatusItemView())
|
id(new PHUIStatusItemView())
|
||||||
->setIcon(PHUIStatusItemView::ICON_CLOCK, 'green')
|
->setIcon(PHUIStatusItemView::ICON_CLOCK, 'green')
|
||||||
->setTarget(pht('Initializing Working Copy'))
|
->setTarget(pht('Initializing Working Copy'))
|
||||||
->setNote(pht('Daemons are initializing the working copy.')));
|
->setNote(pht('Daemons are initializing the working copy.')));
|
||||||
return $view;
|
return $view;
|
||||||
default:
|
|
||||||
$view->addItem(
|
|
||||||
id(new PHUIStatusItemView())
|
|
||||||
->setIcon(PHUIStatusItemView::ICON_WARNING, 'red')
|
|
||||||
->setTarget(pht('Unknown Init Status'))
|
|
||||||
->setNote($message->getStatusCode()));
|
|
||||||
return $view;
|
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
$view->addItem(
|
$view->addItem(
|
||||||
|
|
|
@ -172,8 +172,6 @@ final class PhabricatorRepositoryPullEngine
|
||||||
}
|
}
|
||||||
|
|
||||||
private function logPull($message) {
|
private function logPull($message) {
|
||||||
$code_working = PhabricatorRepositoryStatusMessage::CODE_WORKING;
|
|
||||||
$this->updateRepositoryInitStatus($code_working, $message);
|
|
||||||
$this->log('%s', $message);
|
$this->log('%s', $message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -1649,21 +1649,33 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
|
||||||
$this->getID(),
|
$this->getID(),
|
||||||
$status_type);
|
$status_type);
|
||||||
} else {
|
} else {
|
||||||
|
// If the existing message has the same code (e.g., we just hit an
|
||||||
|
// error and also previously hit an error) we increment the message
|
||||||
|
// count by 1. This allows us to determine how many times in a row
|
||||||
|
// we've run into an error.
|
||||||
|
|
||||||
queryfx(
|
queryfx(
|
||||||
$conn_w,
|
$conn_w,
|
||||||
'INSERT INTO %T
|
'INSERT INTO %T
|
||||||
(repositoryID, statusType, statusCode, parameters, epoch)
|
(repositoryID, statusType, statusCode, parameters, epoch,
|
||||||
VALUES (%d, %s, %s, %s, %d)
|
messageCount)
|
||||||
|
VALUES (%d, %s, %s, %s, %d, %d)
|
||||||
ON DUPLICATE KEY UPDATE
|
ON DUPLICATE KEY UPDATE
|
||||||
statusCode = VALUES(statusCode),
|
statusCode = VALUES(statusCode),
|
||||||
parameters = VALUES(parameters),
|
parameters = VALUES(parameters),
|
||||||
epoch = VALUES(epoch)',
|
epoch = VALUES(epoch),
|
||||||
|
messageCount =
|
||||||
|
IF(
|
||||||
|
statusCode = VALUES(statusCode),
|
||||||
|
messageCount + 1,
|
||||||
|
VALUES(messageCount))',
|
||||||
$table_name,
|
$table_name,
|
||||||
$this->getID(),
|
$this->getID(),
|
||||||
$status_type,
|
$status_type,
|
||||||
$status_code,
|
$status_code,
|
||||||
json_encode($parameters),
|
json_encode($parameters),
|
||||||
time());
|
time(),
|
||||||
|
1);
|
||||||
}
|
}
|
||||||
|
|
||||||
return $this;
|
return $this;
|
||||||
|
@ -1738,6 +1750,33 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
|
||||||
* @return int Repository update interval, in seconds.
|
* @return int Repository update interval, in seconds.
|
||||||
*/
|
*/
|
||||||
public function loadUpdateInterval($minimum = 15) {
|
public function loadUpdateInterval($minimum = 15) {
|
||||||
|
// First, check if we've hit errors recently. If we have, wait one period
|
||||||
|
// for each consecutive error. Normally, this corresponds to a backoff of
|
||||||
|
// 15s, 30s, 45s, etc.
|
||||||
|
|
||||||
|
$message_table = new PhabricatorRepositoryStatusMessage();
|
||||||
|
$conn = $message_table->establishConnection('r');
|
||||||
|
$error_count = queryfx_one(
|
||||||
|
$conn,
|
||||||
|
'SELECT MAX(messageCount) error_count FROM %T
|
||||||
|
WHERE repositoryID = %d
|
||||||
|
AND statusType IN (%Ls)
|
||||||
|
AND statusCode IN (%Ls)',
|
||||||
|
$message_table->getTableName(),
|
||||||
|
$this->getID(),
|
||||||
|
array(
|
||||||
|
PhabricatorRepositoryStatusMessage::TYPE_INIT,
|
||||||
|
PhabricatorRepositoryStatusMessage::TYPE_FETCH,
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
PhabricatorRepositoryStatusMessage::CODE_ERROR,
|
||||||
|
));
|
||||||
|
|
||||||
|
$error_count = (int)$error_count['error_count'];
|
||||||
|
if ($error_count > 0) {
|
||||||
|
return (int)($minimum * $error_count);
|
||||||
|
}
|
||||||
|
|
||||||
// If a repository is still importing, always pull it as frequently as
|
// If a repository is still importing, always pull it as frequently as
|
||||||
// possible. This prevents us from hanging for a long time at 99.9% when
|
// possible. This prevents us from hanging for a long time at 99.9% when
|
||||||
// importing an inactive repository.
|
// importing an inactive repository.
|
||||||
|
@ -1758,6 +1797,13 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
|
||||||
$window_start);
|
$window_start);
|
||||||
if ($last_commit) {
|
if ($last_commit) {
|
||||||
$time_since_commit = ($window_start - $last_commit['epoch']);
|
$time_since_commit = ($window_start - $last_commit['epoch']);
|
||||||
|
} else {
|
||||||
|
// If the repository has no commits, treat the creation date as
|
||||||
|
// though it were the date of the last commit. This makes empty
|
||||||
|
// repositories update quickly at first but slow down over time
|
||||||
|
// if they don't see any activity.
|
||||||
|
$time_since_commit = ($window_start - $this->getDateCreated());
|
||||||
|
}
|
||||||
|
|
||||||
$last_few_days = phutil_units('3 days in seconds');
|
$last_few_days = phutil_units('3 days in seconds');
|
||||||
|
|
||||||
|
@ -1777,11 +1823,7 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
|
||||||
// We'll never wait more than 6 hours to pull a repository.
|
// We'll never wait more than 6 hours to pull a repository.
|
||||||
$longest_wait = phutil_units('6 hours in seconds');
|
$longest_wait = phutil_units('6 hours in seconds');
|
||||||
$smart_wait = min($smart_wait, $longest_wait);
|
$smart_wait = min($smart_wait, $longest_wait);
|
||||||
|
|
||||||
$smart_wait = max($minimum, $smart_wait);
|
$smart_wait = max($minimum, $smart_wait);
|
||||||
} else {
|
|
||||||
$smart_wait = $minimum;
|
|
||||||
}
|
|
||||||
|
|
||||||
return (int)$smart_wait;
|
return (int)$smart_wait;
|
||||||
}
|
}
|
||||||
|
|
|
@ -8,7 +8,6 @@ final class PhabricatorRepositoryStatusMessage
|
||||||
const TYPE_NEEDS_UPDATE = 'needs-update';
|
const TYPE_NEEDS_UPDATE = 'needs-update';
|
||||||
|
|
||||||
const CODE_ERROR = 'error';
|
const CODE_ERROR = 'error';
|
||||||
const CODE_WORKING = 'working';
|
|
||||||
const CODE_OKAY = 'okay';
|
const CODE_OKAY = 'okay';
|
||||||
|
|
||||||
protected $repositoryID;
|
protected $repositoryID;
|
||||||
|
@ -16,6 +15,7 @@ final class PhabricatorRepositoryStatusMessage
|
||||||
protected $statusCode;
|
protected $statusCode;
|
||||||
protected $parameters = array();
|
protected $parameters = array();
|
||||||
protected $epoch;
|
protected $epoch;
|
||||||
|
protected $messageCount;
|
||||||
|
|
||||||
protected function getConfiguration() {
|
protected function getConfiguration() {
|
||||||
return array(
|
return array(
|
||||||
|
@ -26,6 +26,7 @@ final class PhabricatorRepositoryStatusMessage
|
||||||
self::CONFIG_COLUMN_SCHEMA => array(
|
self::CONFIG_COLUMN_SCHEMA => array(
|
||||||
'statusType' => 'text32',
|
'statusType' => 'text32',
|
||||||
'statusCode' => 'text32',
|
'statusCode' => 'text32',
|
||||||
|
'messageCount' => 'uint32',
|
||||||
),
|
),
|
||||||
self::CONFIG_KEY_SCHEMA => array(
|
self::CONFIG_KEY_SCHEMA => array(
|
||||||
'repositoryID' => array(
|
'repositoryID' => array(
|
||||||
|
|
Loading…
Reference in a new issue