Skip to content

Commit 9c798e5

Browse files
author
epriestley
committed
Provide bin/garbage for interacting with garbage collection
Summary: Fixes T9494. This: - Removes all the random GC.x.y.z config. - Puts it all in one place that's locked and which you use `bin/garbage set-policy ...` to adjust. - Makes every TTL-based GC configurable. - Simplifies the code in the actual GCs. Test Plan: - Ran `bin/garbage collect` to collect some garbage, until it stopped collecting. - Ran `bin/garbage set-policy ...` to shorten policy. Saw change in web UI. Ran `bin/garbage collect` again and saw it collect more garbage. - Set policy to indefinite and saw it not collect garabge. - Set policy to default and saw it reflected in web UI / `collect`. - Ran `bin/phd debug trigger` and saw all GCs fire with reasonable looking queries. - Read new docs. {F857928} Reviewers: chad Reviewed By: chad Maniphest Tasks: T9494 Differential Revision: https://secure.phabricator.com/D14219
1 parent bb4667c commit 9c798e5

File tree

36 files changed

+486
-188
lines changed

36 files changed

+486
-188
lines changed

bin/garbage

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
../scripts/setup/manage_garbage.php

scripts/setup/manage_garbage.php

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#!/usr/bin/env php
2+
<?php
3+
4+
$root = dirname(dirname(dirname(__FILE__)));
5+
require_once $root.'/scripts/__init_script__.php';
6+
7+
$args = new PhutilArgumentParser($argv);
8+
$args->setTagline(pht('manage garbage colletors'));
9+
$args->setSynopsis(<<<EOSYNOPSIS
10+
**garbage** __command__ [__options__]
11+
Manage garbage collectors.
12+
13+
EOSYNOPSIS
14+
);
15+
$args->parseStandardArguments();
16+
17+
$workflows = id(new PhutilClassMapQuery())
18+
->setAncestorClass('PhabricatorGarbageCollectorManagementWorkflow')
19+
->execute();
20+
$workflows[] = new PhutilHelpArgumentWorkflow();
21+
$args->parseWorkflows($workflows);

src/__phutil_library_map__.php

+6-2
Original file line numberDiff line numberDiff line change
@@ -2197,7 +2197,9 @@
21972197
'PhabricatorFundApplication' => 'applications/fund/application/PhabricatorFundApplication.php',
21982198
'PhabricatorGDSetupCheck' => 'applications/config/check/PhabricatorGDSetupCheck.php',
21992199
'PhabricatorGarbageCollector' => 'infrastructure/daemon/garbagecollector/PhabricatorGarbageCollector.php',
2200-
'PhabricatorGarbageCollectorConfigOptions' => 'applications/config/option/PhabricatorGarbageCollectorConfigOptions.php',
2200+
'PhabricatorGarbageCollectorManagementCollectWorkflow' => 'infrastructure/daemon/garbagecollector/management/PhabricatorGarbageCollectorManagementCollectWorkflow.php',
2201+
'PhabricatorGarbageCollectorManagementSetPolicyWorkflow' => 'infrastructure/daemon/garbagecollector/management/PhabricatorGarbageCollectorManagementSetPolicyWorkflow.php',
2202+
'PhabricatorGarbageCollectorManagementWorkflow' => 'infrastructure/daemon/garbagecollector/management/PhabricatorGarbageCollectorManagementWorkflow.php',
22012203
'PhabricatorGestureUIExample' => 'applications/uiexample/examples/PhabricatorGestureUIExample.php',
22022204
'PhabricatorGitGraphStream' => 'applications/repository/daemon/PhabricatorGitGraphStream.php',
22032205
'PhabricatorGitHubAuthProvider' => 'applications/auth/provider/PhabricatorGitHubAuthProvider.php',
@@ -6197,7 +6199,9 @@
61976199
'PhabricatorFundApplication' => 'PhabricatorApplication',
61986200
'PhabricatorGDSetupCheck' => 'PhabricatorSetupCheck',
61996201
'PhabricatorGarbageCollector' => 'Phobject',
6200-
'PhabricatorGarbageCollectorConfigOptions' => 'PhabricatorApplicationConfigOptions',
6202+
'PhabricatorGarbageCollectorManagementCollectWorkflow' => 'PhabricatorGarbageCollectorManagementWorkflow',
6203+
'PhabricatorGarbageCollectorManagementSetPolicyWorkflow' => 'PhabricatorGarbageCollectorManagementWorkflow',
6204+
'PhabricatorGarbageCollectorManagementWorkflow' => 'PhabricatorManagementWorkflow',
62016205
'PhabricatorGestureUIExample' => 'PhabricatorUIExample',
62026206
'PhabricatorGitGraphStream' => 'PhabricatorRepositoryGraphStream',
62036207
'PhabricatorGitHubAuthProvider' => 'PhabricatorOAuth2AuthProvider',

src/applications/auth/garbagecollector/PhabricatorAuthSessionGarbageCollector.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ final class PhabricatorAuthSessionGarbageCollector
66
const COLLECTORCONST = 'auth.sessions';
77

88
public function getCollectorName() {
9-
return pht('Auth Sessions');
9+
return pht('Authentication Sessions');
1010
}
1111

1212
public function hasAutomaticPolicy() {
1313
return true;
1414
}
1515

16-
public function collectGarbage() {
16+
protected function collectGarbage() {
1717
$session_table = new PhabricatorAuthSession();
1818
$conn_w = $session_table->establishConnection('w');
1919

src/applications/auth/garbagecollector/PhabricatorAuthTemporaryTokenGarbageCollector.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ final class PhabricatorAuthTemporaryTokenGarbageCollector
66
const COLLECTORCONST = 'auth.tokens';
77

88
public function getCollectorName() {
9-
return pht('Auth Tokens');
9+
return pht('Authentication Tokens');
1010
}
1111

1212
public function hasAutomaticPolicy() {
1313
return true;
1414
}
1515

16-
public function collectGarbage() {
16+
protected function collectGarbage() {
1717
$session_table = new PhabricatorAuthTemporaryToken();
1818
$conn_w = $session_table->establishConnection('w');
1919

src/applications/cache/garbagecollector/PhabricatorCacheGeneralGarbageCollector.php

+2-8
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,7 @@ public function getDefaultRetentionPolicy() {
1313
return phutil_units('30 days in seconds');
1414
}
1515

16-
public function collectGarbage() {
17-
$key = 'gcdaemon.ttl.general-cache';
18-
$ttl = PhabricatorEnv::getEnvConfig($key);
19-
if ($ttl <= 0) {
20-
return false;
21-
}
22-
16+
protected function collectGarbage() {
2317
$cache = new PhabricatorKeyValueDatabaseCache();
2418
$conn_w = $cache->establishConnection('w');
2519

@@ -28,7 +22,7 @@ public function collectGarbage() {
2822
'DELETE FROM %T WHERE cacheCreated < %d
2923
ORDER BY cacheCreated ASC LIMIT 100',
3024
$cache->getTableName(),
31-
time() - $ttl);
25+
$this->getGarbageEpoch());
3226

3327
return ($conn_w->getAffectedRows() == 100);
3428
}

src/applications/cache/garbagecollector/PhabricatorCacheMarkupGarbageCollector.php

+2-8
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,15 @@ public function getDefaultRetentionPolicy() {
1313
return phutil_units('30 days in seconds');
1414
}
1515

16-
public function collectGarbage() {
17-
$key = 'gcdaemon.ttl.markup-cache';
18-
$ttl = PhabricatorEnv::getEnvConfig($key);
19-
if ($ttl <= 0) {
20-
return false;
21-
}
22-
16+
protected function collectGarbage() {
2317
$table = new PhabricatorMarkupCache();
2418
$conn_w = $table->establishConnection('w');
2519

2620
queryfx(
2721
$conn_w,
2822
'DELETE FROM %T WHERE dateCreated < %d LIMIT 100',
2923
$table->getTableName(),
30-
time() - $ttl);
24+
$this->getGarbageEpoch());
3125

3226
return ($conn_w->getAffectedRows() == 100);
3327
}

src/applications/cache/garbagecollector/PhabricatorCacheTTLGarbageCollector.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ public function hasAutomaticPolicy() {
1313
return true;
1414
}
1515

16-
public function collectGarbage() {
16+
protected function collectGarbage() {
1717
$cache = new PhabricatorKeyValueDatabaseCache();
1818
$conn_w = $cache->establishConnection('w');
1919

@@ -22,7 +22,7 @@ public function collectGarbage() {
2222
'DELETE FROM %T WHERE cacheExpires < %d
2323
ORDER BY cacheExpires ASC LIMIT 100',
2424
$cache->getTableName(),
25-
time());
25+
PhabricatorTime::getNow());
2626

2727
return ($conn_w->getAffectedRows() == 100);
2828
}

src/applications/conduit/garbagecollector/ConduitConnectionGarbageCollector.php

+3-8
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,16 @@ public function getDefaultRetentionPolicy() {
1313
return phutil_units('180 days in seconds');
1414
}
1515

16-
public function collectGarbage() {
17-
$key = 'gcdaemon.ttl.conduit-logs';
18-
$ttl = PhabricatorEnv::getEnvConfig($key);
19-
if ($ttl <= 0) {
20-
return false;
21-
}
22-
16+
protected function collectGarbage() {
2317
$table = new PhabricatorConduitConnectionLog();
2418
$conn_w = $table->establishConnection('w');
19+
2520
queryfx(
2621
$conn_w,
2722
'DELETE FROM %T WHERE dateCreated < %d
2823
ORDER BY dateCreated ASC LIMIT 100',
2924
$table->getTableName(),
30-
time() - $ttl);
25+
$this->getGarbageEpoch());
3126

3227
return ($conn_w->getAffectedRows() == 100);
3328
}

src/applications/conduit/garbagecollector/ConduitLogGarbageCollector.php

+3-8
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,16 @@ public function getDefaultRetentionPolicy() {
1313
return phutil_units('180 days in seconds');
1414
}
1515

16-
public function collectGarbage() {
17-
$key = 'gcdaemon.ttl.conduit-logs';
18-
$ttl = PhabricatorEnv::getEnvConfig($key);
19-
if ($ttl <= 0) {
20-
return false;
21-
}
22-
16+
protected function collectGarbage() {
2317
$table = new PhabricatorConduitMethodCallLog();
2418
$conn_w = $table->establishConnection('w');
19+
2520
queryfx(
2621
$conn_w,
2722
'DELETE FROM %T WHERE dateCreated < %d
2823
ORDER BY dateCreated ASC LIMIT 100',
2924
$table->getTableName(),
30-
time() - $ttl);
25+
$this->getGarbageEpoch());
3126

3227
return ($conn_w->getAffectedRows() == 100);
3328
}

src/applications/conduit/garbagecollector/ConduitTokenGarbageCollector.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ public function hasAutomaticPolicy() {
1313
return true;
1414
}
1515

16-
public function collectGarbage() {
16+
protected function collectGarbage() {
1717
$table = new PhabricatorConduitToken();
1818
$conn_w = $table->establishConnection('w');
19+
1920
queryfx(
2021
$conn_w,
2122
'DELETE FROM %T WHERE expires <= %d

src/applications/config/check/PhabricatorExtraConfigSetupCheck.php

+12
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ public static function getAncientConfig() {
176176
'Inbound mail addresses are now configured for each application '.
177177
'in the Applications tool.');
178178

179+
$gc_reason = pht(
180+
'Garbage collectors are now configured with "%s".',
181+
'bin/garbage set-policy');
182+
179183
$ancient_config += array(
180184
'phid.external-loaders' =>
181185
pht(
@@ -280,6 +284,14 @@ public static function getAncientConfig() {
280284
'auth.login-message' => pht(
281285
'This configuration option has been replaced with a modular '.
282286
'handler. See T9346.'),
287+
288+
'gcdaemon.ttl.herald-transcripts' => $gc_reason,
289+
'gcdaemon.ttl.daemon-logs' => $gc_reason,
290+
'gcdaemon.ttl.differential-parse-cache' => $gc_reason,
291+
'gcdaemon.ttl.markup-cache' => $gc_reason,
292+
'gcdaemon.ttl.task-archive' => $gc_reason,
293+
'gcdaemon.ttl.general-cache' => $gc_reason,
294+
'gcdaemon.ttl.conduit-logs' => $gc_reason,
283295
);
284296

285297
return $ancient_config;

src/applications/config/module/PhabricatorConfigCollectorsModule.php

+20-2
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ public function renderModuleStatus(AphrontRequest $request) {
1717
$collectors = msort($collectors, 'getCollectorConstant');
1818

1919
$rows = array();
20+
$rowc = array();
2021
foreach ($collectors as $key => $collector) {
22+
$class = null;
2123
if ($collector->hasAutomaticPolicy()) {
2224
$policy_view = phutil_tag('em', array(), pht('Automatic'));
2325
} else {
24-
$policy = $collector->getDefaultRetentionPolicy();
26+
$policy = $collector->getRetentionPolicy();
2527
if ($policy === null) {
2628
$policy_view = pht('Indefinite');
2729
} else {
@@ -30,8 +32,15 @@ public function renderModuleStatus(AphrontRequest $request) {
3032
'%s Day(s)',
3133
new PhutilNumber($days));
3234
}
35+
36+
$default = $collector->getDefaultRetentionPolicy();
37+
if ($policy !== $default) {
38+
$class = 'highlighted';
39+
$policy_view = phutil_tag('strong', array(), $policy_view);
40+
}
3341
}
3442

43+
$rowc[] = $class;
3544
$rows[] = array(
3645
$collector->getCollectorConstant(),
3746
$collector->getCollectorName(),
@@ -40,6 +49,7 @@ public function renderModuleStatus(AphrontRequest $request) {
4049
}
4150

4251
$table = id(new AphrontTableView($rows))
52+
->setRowClasses($rowc)
4353
->setHeaders(
4454
array(
4555
pht('Constant'),
@@ -53,8 +63,16 @@ public function renderModuleStatus(AphrontRequest $request) {
5363
null,
5464
));
5565

66+
$header = id(new PHUIHeaderView())
67+
->setHeader(pht('Garbage Collectors'))
68+
->setSubheader(
69+
pht(
70+
'Collectors with custom policies are highlighted. Use '.
71+
'%s to change retention policies.',
72+
phutil_tag('tt', array(), 'bin/garbage set-policy')));
73+
5674
return id(new PHUIObjectBoxView())
57-
->setHeaderText(pht('Garbage Collectors'))
75+
->setHeader($header)
5876
->setTable($table);
5977
}
6078

src/applications/config/option/PhabricatorGarbageCollectorConfigOptions.php

-70
This file was deleted.

src/applications/config/option/PhabricatorPHDConfigOptions.php

+11
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,17 @@ public function getOptions() {
8080
'and the daemons. Primarily, this is a way to suppress the '.
8181
'"Daemons and Web Have Different Config" setup issue on a per '.
8282
'config key basis.')),
83+
$this->newOption('phd.garbage-collection', 'wild', array())
84+
->setLocked(true)
85+
->setLockedMessage(
86+
pht(
87+
'This option can not be edited from the web UI. Use %s to adjust '.
88+
'garbage collector policies.',
89+
phutil_tag('tt', array(), 'bin/garbage set-policy')))
90+
->setSummary(pht('Retention policies for garbage collection.'))
91+
->setDescription(
92+
pht(
93+
'Customizes retention policies for garbage collectors.')),
8394
);
8495
}
8596

0 commit comments

Comments
 (0)