Skip to content

Commit 7164606

Browse files
author
Joshua Spence
committedDec 1, 2015
Add a lock to storage upgrade and adjustment
Summary: Fixes T9715. Adds a MySQL-based lock to ensure that schema migrations are not applied on multiple hosts simultaneously. Test Plan: Ran `./bin/storage upgrade` concurrently. One invocation was successful whilst the other hit a `PhutilLockException`. Reviewers: #blessed_reviewers, epriestley Subscribers: Korvin Maniphest Tasks: T9715 Differential Revision: https://secure.phabricator.com/D14463
1 parent bbd1da4 commit 7164606

13 files changed

+354
-251
lines changed
 

‎scripts/sql/manage_storage.php

+21-25
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,17 @@
6363
$default_namespace),
6464
),
6565
array(
66-
'name' => 'dryrun',
67-
'help' => pht(
66+
'name' => 'dryrun',
67+
'help' => pht(
6868
'Do not actually change anything, just show what would be changed.'),
6969
),
7070
array(
71-
'name' => 'disable-utf8mb4',
72-
'help' => pht(
73-
'Disable utf8mb4, even if the database supports it. This is an '.
71+
'name' => 'disable-utf8mb4',
72+
'help' => pht(
73+
'Disable %s, even if the database supports it. This is an '.
7474
'advanced feature used for testing changes to Phabricator; you '.
75-
'should not normally use this flag.'),
75+
'should not normally use this flag.',
76+
'utf8mb4'),
7677
),
7778
));
7879
} catch (PhutilArgumentUsageException $ex) {
@@ -83,12 +84,12 @@
8384
// First, test that the Phabricator configuration is set up correctly. After
8485
// we know this works we'll test any administrative credentials specifically.
8586

86-
$test_api = new PhabricatorStorageManagementAPI();
87-
$test_api->setUser($default_user);
88-
$test_api->setHost($default_host);
89-
$test_api->setPort($default_port);
90-
$test_api->setPassword($conf->getPassword());
91-
$test_api->setNamespace($args->getArg('namespace'));
87+
$test_api = id(new PhabricatorStorageManagementAPI())
88+
->setUser($default_user)
89+
->setHost($default_host)
90+
->setPort($default_port)
91+
->setPassword($conf->getPassword())
92+
->setNamespace($args->getArg('namespace'));
9293

9394
try {
9495
queryfx(
@@ -113,13 +114,10 @@
113114
'--password'),
114115
pht('Raw MySQL Error'),
115116
$ex->getMessage());
116-
117117
echo phutil_console_wrap($message);
118-
119118
exit(1);
120119
}
121120

122-
123121
if ($args->getArg('password') === null) {
124122
// This is already a PhutilOpaqueEnvelope.
125123
$password = $conf->getPassword();
@@ -129,14 +127,14 @@
129127
PhabricatorEnv::overrideConfig('mysql.pass', $args->getArg('password'));
130128
}
131129

132-
$api = new PhabricatorStorageManagementAPI();
133-
$api->setUser($args->getArg('user'));
134-
PhabricatorEnv::overrideConfig('mysql.user', $args->getArg('user'));
135-
$api->setHost($default_host);
136-
$api->setPort($default_port);
137-
$api->setPassword($password);
138-
$api->setNamespace($args->getArg('namespace'));
139-
$api->setDisableUTF8MB4($args->getArg('disable-utf8mb4'));
130+
$api = id(new PhabricatorStorageManagementAPI())
131+
->setUser($args->getArg('user'))
132+
->setHost($default_host)
133+
->setPort($default_port)
134+
->setPassword($password)
135+
->setNamespace($args->getArg('namespace'))
136+
->setDisableUTF8MB4($args->getArg('disable-utf8mb4'));
137+
PhabricatorEnv::overrideConfig('mysql.user', $api->getUser());
140138

141139
try {
142140
queryfx(
@@ -154,9 +152,7 @@
154152
'--password'),
155153
pht('Raw MySQL Error'),
156154
$ex->getMessage());
157-
158155
echo phutil_console_wrap($message);
159-
160156
exit(1);
161157
}
162158

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementAdjustWorkflow.php

+2-4
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,11 @@ protected function didConstruct() {
2424
));
2525
}
2626

27-
public function execute(PhutilArgumentParser $args) {
28-
$force = $args->getArg('force');
27+
public function didExecute(PhutilArgumentParser $args) {
2928
$unsafe = $args->getArg('unsafe');
30-
$dry_run = $args->getArg('dryrun');
3129

3230
$this->requireAllPatchesApplied();
33-
return $this->adjustSchemata($force, $unsafe, $dry_run);
31+
return $this->adjustSchemata($unsafe);
3432
}
3533

3634
private function requireAllPatchesApplied() {

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDatabasesWorkflow.php

+3-4
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,12 @@ protected function didConstruct() {
1010
->setSynopsis(pht('List Phabricator databases.'));
1111
}
1212

13-
public function execute(PhutilArgumentParser $args) {
14-
$api = $this->getAPI();
13+
public function didExecute(PhutilArgumentParser $args) {
14+
$api = $this->getAPI();
1515
$patches = $this->getPatches();
1616

17-
$databases = $api->getDatabaseList($patches, $only_living = true);
17+
$databases = $api->getDatabaseList($patches, true);
1818
echo implode("\n", $databases)."\n";
19-
2019
return 0;
2120
}
2221

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDestroyWorkflow.php

+23-18
Original file line numberDiff line numberDiff line change
@@ -20,29 +20,29 @@ protected function didConstruct() {
2020
));
2121
}
2222

23-
public function execute(PhutilArgumentParser $args) {
24-
$is_dry = $args->getArg('dryrun');
25-
$is_force = $args->getArg('force');
23+
public function didExecute(PhutilArgumentParser $args) {
24+
$console = PhutilConsole::getConsole();
2625

27-
if (!$is_dry && !$is_force) {
28-
echo phutil_console_wrap(
29-
pht(
30-
'Are you completely sure you really want to permanently destroy all '.
31-
'storage for Phabricator data? This operation can not be undone and '.
32-
'your data will not be recoverable if you proceed.'));
26+
if (!$this->isDryRun() && !$this->isForce()) {
27+
$console->writeOut(
28+
phutil_console_wrap(
29+
pht(
30+
'Are you completely sure you really want to permanently destroy '.
31+
'all storage for Phabricator data? This operation can not be '.
32+
'undone and your data will not be recoverable if you proceed.')));
3333

3434
if (!phutil_console_confirm(pht('Permanently destroy all data?'))) {
35-
echo pht('Cancelled.')."\n";
35+
$console->writeOut("%s\n", pht('Cancelled.'));
3636
exit(1);
3737
}
3838

3939
if (!phutil_console_confirm(pht('Really destroy all data forever?'))) {
40-
echo pht('Cancelled.')."\n";
40+
$console->writeOut("%s\n", pht('Cancelled.'));
4141
exit(1);
4242
}
4343
}
4444

45-
$api = $this->getAPI();
45+
$api = $this->getAPI();
4646
$patches = $this->getPatches();
4747

4848
if ($args->getArg('unittest-fixtures')) {
@@ -55,27 +55,32 @@ public function execute(PhutilArgumentParser $args) {
5555
PhabricatorTestCase::NAMESPACE_PREFIX);
5656
$databases = ipull($databases, 'db');
5757
} else {
58-
$databases = $api->getDatabaseList($patches);
58+
$databases = $api->getDatabaseList($patches);
5959
$databases[] = $api->getDatabaseName('meta_data');
60+
6061
// These are legacy databases that were dropped long ago. See T2237.
6162
$databases[] = $api->getDatabaseName('phid');
6263
$databases[] = $api->getDatabaseName('directory');
6364
}
6465

6566
foreach ($databases as $database) {
66-
if ($is_dry) {
67-
echo pht("DRYRUN: Would drop database '%s'.", $database)."\n";
67+
if ($this->isDryRun()) {
68+
$console->writeOut(
69+
"%s\n",
70+
pht("DRYRUN: Would drop database '%s'.", $database));
6871
} else {
69-
echo pht("Dropping database '%s'...", $database)."\n";
72+
$console->writeOut(
73+
"%s\n",
74+
pht("Dropping database '%s'...", $database));
7075
queryfx(
7176
$api->getConn(null),
7277
'DROP DATABASE IF EXISTS %T',
7378
$database);
7479
}
7580
}
7681

77-
if (!$is_dry) {
78-
echo pht('Storage was destroyed.')."\n";
82+
if (!$this->isDryRun()) {
83+
$console->writeOut("%s\n", pht('Storage was destroyed.'));
7984
}
8085

8186
return 0;

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php

+6-5
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ protected function didConstruct() {
1010
->setSynopsis(pht('Dump all data in storage to stdout.'));
1111
}
1212

13-
public function execute(PhutilArgumentParser $args) {
14-
$console = PhutilConsole::getConsole();
15-
$api = $this->getAPI();
13+
public function didExecute(PhutilArgumentParser $args) {
14+
$api = $this->getAPI();
1615
$patches = $this->getPatches();
1716

17+
$console = PhutilConsole::getConsole();
18+
1819
$applied = $api->getAppliedPatches();
1920
if ($applied === null) {
2021
$namespace = $api->getNamespace();
@@ -24,11 +25,11 @@ public function execute(PhutilArgumentParser $args) {
2425
'initialized in this storage namespace ("%s"). Use '.
2526
'**%s** to initialize storage.',
2627
$namespace,
27-
'storage upgrade'));
28+
'./bin/storage upgrade'));
2829
return 1;
2930
}
3031

31-
$databases = $api->getDatabaseList($patches, $only_living = true);
32+
$databases = $api->getDatabaseList($patches, true);
3233

3334
list($host, $port) = $this->getBareHostAndPort($api->getHost());
3435

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementProbeWorkflow.php

+4-4
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,15 @@ protected function didConstruct() {
1010
->setSynopsis(pht('Show approximate table sizes.'));
1111
}
1212

13-
public function execute(PhutilArgumentParser $args) {
13+
public function didExecute(PhutilArgumentParser $args) {
1414
$console = PhutilConsole::getConsole();
1515
$console->writeErr(
1616
"%s\n",
1717
pht('Analyzing table sizes (this may take a moment)...'));
1818

19-
$api = $this->getAPI();
20-
$patches = $this->getPatches();
21-
$databases = $api->getDatabaseList($patches, $only_living = true);
19+
$api = $this->getAPI();
20+
$patches = $this->getPatches();
21+
$databases = $api->getDatabaseList($patches, true);
2222

2323
$conn_r = $api->getConn(null);
2424

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementQuickstartWorkflow.php

+7-3
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ protected function didConstruct() {
2222
}
2323

2424
public function execute(PhutilArgumentParser $args) {
25+
parent::execute($args);
26+
2527
$output = $args->getArg('output');
2628
if (!$output) {
2729
throw new PhutilArgumentUsageException(
@@ -38,8 +40,10 @@ public function execute(PhutilArgumentParser $args) {
3840
throw new PhutilArgumentUsageException(
3941
pht(
4042
'You can only generate a new quickstart file if MySQL supports '.
41-
'the utf8mb4 character set (available in MySQL 5.5 and newer). The '.
42-
'configured server does not support utf8mb4.'));
43+
'the %s character set (available in MySQL 5.5 and newer). The '.
44+
'configured server does not support %s.',
45+
'utf8mb4',
46+
'utf8mb4'));
4347
}
4448

4549
$err = phutil_passthru(
@@ -139,7 +143,7 @@ public function execute(PhutilArgumentParser $args) {
139143
$dump = preg_replace('/^--.*$/m', '', $dump);
140144

141145
// Remove table drops, locks, and unlocks. These are never relevant when
142-
// performing q quickstart.
146+
// performing a quickstart.
143147
$dump = preg_replace(
144148
'/^(DROP TABLE|LOCK TABLES|UNLOCK TABLES).*$/m',
145149
'',

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementRenamespaceWorkflow.php

+10-4
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,31 @@ protected function didConstruct() {
3030
));
3131
}
3232

33-
public function execute(PhutilArgumentParser $args) {
33+
public function didExecute(PhutilArgumentParser $args) {
3434
$console = PhutilConsole::getConsole();
3535

3636
$in = $args->getArg('in');
3737
if (!strlen($in)) {
3838
throw new PhutilArgumentUsageException(
39-
pht('Specify the dumpfile to read with --in.'));
39+
pht(
40+
'Specify the dumpfile to read with %s.',
41+
'--in'));
4042
}
4143

4244
$from = $args->getArg('from');
4345
if (!strlen($from)) {
4446
throw new PhutilArgumentUsageException(
45-
pht('Specify namespace to rename from with --from.'));
47+
pht(
48+
'Specify namespace to rename from with %s.',
49+
'--from'));
4650
}
4751

4852
$to = $args->getArg('to');
4953
if (!strlen($to)) {
5054
throw new PhutilArgumentUsageException(
51-
pht('Specify namespace to rename to with --to.'));
55+
pht(
56+
'Specify namespace to rename to with %s.',
57+
'--to'));
5258
}
5359

5460
$patterns = array(

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementShellWorkflow.php

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ protected function didConstruct() {
1111
}
1212

1313
public function execute(PhutilArgumentParser $args) {
14+
15+
1416
$api = $this->getAPI();
1517
list($host, $port) = $this->getBareHostAndPort($api->getHost());
1618

‎src/infrastructure/storage/management/workflow/PhabricatorStorageManagementStatusWorkflow.php

+7-7
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ protected function didConstruct() {
1010
->setSynopsis(pht('Show patch application status.'));
1111
}
1212

13-
public function execute(PhutilArgumentParser $args) {
14-
$api = $this->getAPI();
13+
public function didExecute(PhutilArgumentParser $args) {
14+
$api = $this->getAPI();
1515
$patches = $this->getPatches();
1616

1717
$applied = $api->getAppliedPatches();
@@ -20,18 +20,18 @@ public function execute(PhutilArgumentParser $args) {
2020
echo phutil_console_format(
2121
"**%s**: %s\n",
2222
pht('Database Not Initialized'),
23-
pht('Run **%s** to initialize.', 'storage upgrade'));
23+
pht('Run **%s** to initialize.', './bin/storage upgrade'));
2424

2525
return 1;
2626
}
2727

2828
$table = id(new PhutilConsoleTable())
2929
->setShowHeader(false)
30-
->addColumn('id', array('title' => pht('ID')))
31-
->addColumn('status', array('title' => pht('Status')))
30+
->addColumn('id', array('title' => pht('ID')))
31+
->addColumn('status', array('title' => pht('Status')))
3232
->addColumn('duration', array('title' => pht('Duration')))
33-
->addColumn('type', array('title' => pht('Type')))
34-
->addColumn('name', array('title' => pht('Name')));
33+
->addColumn('type', array('title' => pht('Type')))
34+
->addColumn('name', array('title' => pht('Name')));
3535

3636
$durations = $api->getPatchDurations();
3737

0 commit comments

Comments
 (0)
Failed to load comments.