From 46b50a5995cbdac49434aa2ced2b33155e748a3f Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 26 Apr 2017 11:48:44 -0700 Subject: [PATCH] (stable) Reduce the impact of `bin/storage dump` Summary: Ref T12646. - Use "wb1" instead of "wb" to use level 1 gzip compression (faster, less compressy). Locally, this went about 2x faster and the output only grew 4% larger. - LinesOfALargeExecFuture does a lot of unnecessary string operations, and can boil down to a busy wait. The process is pretty saturated by I/O so this isn't the end of the world, but just use raw ExecFuture with FutureIterator so that we wait in `select()`. - Also, nice the process to +19 so we try to give other things CPU. Test Plan: - Ran `bin/storage dump --compress --output ...`. - Saw CPU time for my local database drop from ~240s to ~90s, with a 4% larger output. Most of this was adding the `1`, but the ExecFuture thing helped a little, too. - I'm not sure what a great way to test `nice` in a local environment is and it's system dependent anyway, but nothing got worse / blew up. - Used `gzcat | head` and `gzcat | tail` on the result to sanity-check that everything was preserved. Reviewers: chad, amckinley Reviewed By: chad Maniphest Tasks: T12646 Differential Revision: https://secure.phabricator.com/D17795 --- ...abricatorStorageManagementDumpWorkflow.php | 49 +++++++++++++------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php index 7195e735c6..4dc5c64042 100644 --- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php +++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php @@ -138,6 +138,13 @@ final class PhabricatorStorageManagementDumpWorkflow $command = csprintf('mysqldump %Ls', $argv); } + // Decrease the CPU priority of this process so it doesn't contend with + // other more important things. + if (function_exists('proc_nice')) { + proc_nice(19); + } + + // If we aren't writing to a file, just passthru the command. if ($output_file === null) { return phutil_passthru('%C', $command); @@ -148,7 +155,7 @@ final class PhabricatorStorageManagementDumpWorkflow // a full disk). See T6996 for discussion. if ($is_compress) { - $file = gzopen($output_file, 'wb'); + $file = gzopen($output_file, 'wb1'); } else { $file = fopen($output_file, 'wb'); } @@ -162,23 +169,35 @@ final class PhabricatorStorageManagementDumpWorkflow $future = new ExecFuture('%C', $command); - $lines = new LinesOfALargeExecFuture($future); - try { - foreach ($lines as $line) { - $line = $line."\n"; - if ($is_compress) { - $ok = gzwrite($file, $line); - } else { - $ok = fwrite($file, $line); + $iterator = id(new FutureIterator(array($future))) + ->setUpdateInterval(0.100); + foreach ($iterator as $ready) { + list($stdout, $stderr) = $future->read(); + $future->discardBuffers(); + + if (strlen($stderr)) { + fwrite(STDERR, $stderr); } - if ($ok !== strlen($line)) { - throw new Exception( - pht( - 'Failed to write %d byte(s) to file "%s".', - new PhutilNumber(strlen($line)), - $output_file)); + if (strlen($stdout)) { + if ($is_compress) { + $ok = gzwrite($file, $stdout); + } else { + $ok = fwrite($file, $stdout); + } + + if ($ok !== strlen($stdout)) { + throw new Exception( + pht( + 'Failed to write %d byte(s) to file "%s".', + new PhutilNumber(strlen($stdout)), + $output_file)); + } + } + + if ($ready !== null) { + $ready->resolvex(); } }