archive/phabricator
1
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Recommend dumping database backups with "--compress --output X" instead of "| gzip > X"

Browse Source 
Summary:
Fixes T13304. Shell pipes and redirects do not have robust behavior when errors occur. We provide "--compress" and "--output" flags as robust alternatives, but do not currently recommend their use.

  - Recommend their use, since their error handling behavior is more robust in the face of issues like full disks.
  - If "--compress" is provided but won't work because the "zlib" extension is missing, raise an explicit error. I believe this extension is very common and this error should be rare. If that turns out to be untrue, we could take another look at this.
  - Also, verify some flag usage sooner so we can exit with an error faster if you mistype a "bin/storage dump" command.

Test Plan: Read documentation, hit affected error cases, did a dump and spot-checked that it came out sane looking.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13304

Differential Revision: https://secure.phabricator.com/D20572
This commit is contained in:
epriestley
2019-06-05 14:56:10 -07:00
parent 1dd62f79ce
commit 6219d30f6b
2 changed files with 46 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/docs/user/configuration/configuring_backups.diviner
View File

@@ -42,7 +42,7 @@ but will only dump databases Phabricator owns.
Since most of this data is compressible, it may be helpful to run it through Since most of this data is compressible, it may be helpful to run it through
gzip prior to storage. For example: gzip prior to storage. For example:
phabricator/ $ ./bin/storage dump | gzip > backup.sql.gz phabricator/ $ ./bin/storage dump --compress --output backup.sql.gz
Then store the backup somewhere safe, like in a box buried under an old tree Then store the backup somewhere safe, like in a box buried under an old tree
stump. No one will ever think to look for it there. stump. No one will ever think to look for it there.

87
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
View File

@@ -51,24 +51,59 @@ final class PhabricatorStorageManagementDumpWorkflow
} }
public function didExecute(PhutilArgumentParser $args) { public function didExecute(PhutilArgumentParser $args) {
$output_file = $args->getArg('output');
$is_compress = $args->getArg('compress');
$is_overwrite = $args->getArg('overwrite');
if ($is_compress) {
if ($output_file === null) {
throw new PhutilArgumentUsageException(
pht(
'The "--compress" flag can only be used alongside "--output".'));
}
if (!function_exists('gzopen')) {
throw new PhutilArgumentUsageException(
pht(
'The "--compress" flag requires the PHP "zlib" extension, but '.
'that extension is not available. Install the extension or '.
'omit the "--compress" option.'));
}
}
if ($is_overwrite) {
if ($output_file === null) {
throw new PhutilArgumentUsageException(
pht(
'The "--overwrite" flag can only be used alongside "--output".'));
}
}
if ($output_file !== null) {
if (Filesystem::pathExists($output_file)) {
if (!$is_overwrite) {
throw new PhutilArgumentUsageException(
pht(
'Output file "%s" already exists. Use "--overwrite" '.
'to overwrite.',
$output_file));
}
}
}
$api = $this->getSingleAPI(); $api = $this->getSingleAPI();
$patches = $this->getPatches(); $patches = $this->getPatches();
$console = PhutilConsole::getConsole();
$with_indexes = !$args->getArg('no-indexes'); $with_indexes = !$args->getArg('no-indexes');
$applied = $api->getAppliedPatches(); $applied = $api->getAppliedPatches();
if ($applied === null) { if ($applied === null) {
$namespace = $api->getNamespace(); throw new PhutilArgumentUsageException(
$console->writeErr(
pht( pht(
'**Storage Not Initialized**: There is no database storage '. 'There is no database storage initialized in the current storage '.
'initialized in this storage namespace ("%s"). Use '. 'namespace ("%s"). Use "bin/storage upgrade" to initialize '.
'**%s** to initialize storage.', 'storage or use "--namespace" to choose a different namespace.',
$namespace, $api->getNamespace()));
'./bin/storage upgrade'));
return 1;
} }
$ref = $api->getRef(); $ref = $api->getRef();
@@ -141,38 +176,6 @@ final class PhabricatorStorageManagementDumpWorkflow
} }
} }
$output_file = $args->getArg('output');
$is_compress = $args->getArg('compress');
$is_overwrite = $args->getArg('overwrite');
if ($is_compress) {
if ($output_file === null) {
throw new PhutilArgumentUsageException(
pht(
'The "--compress" flag can only be used alongside "--output".'));
}
}
if ($is_overwrite) {
if ($output_file === null) {
throw new PhutilArgumentUsageException(
pht(
'The "--overwrite" flag can only be used alongside "--output".'));
}
}
if ($output_file !== null) {
if (Filesystem::pathExists($output_file)) {
if (!$is_overwrite) {
throw new PhutilArgumentUsageException(
pht(
'Output file "%s" already exists. Use "--overwrite" '.
'to overwrite.',
$output_file));
}
}
}
$argv = array(); $argv = array();
$argv[] = '--hex-blob'; $argv[] = '--hex-blob';
$argv[] = '--single-transaction'; $argv[] = '--single-transaction';