From ccf207f2092889dc13dadfb8e4843015141cc494 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 25 Jun 2019 05:18:31 -0700
Subject: [PATCH] (stable) Limit the read buffer size in `bin/storage dump`

Summary:
Ref T13328. Currently, we read from `mysqldump` something like this:

```
until (done) {
  for (100 ms) {
    mysqldump > in-memory-buffer;
  }

  in-memory-buffer > disk;
}
```

This general structure isn't great. In this use case, where we're streaming a large amount of data from a source to a sink, we'd prefer to have a "select()"-like way to interact with futures, so our code is called after every read (or maybe once some small buffer fills up, if we want to do the writes in larger chunks).

We don't currently have this (`FutureIterator` can wake up every X milliseconds, or on future exit, but, today, can not wake for readable futures), so we may buffer an arbitrary amount of data into memory (however much data `mysqldump` can write in 100ms).

Reduce the update frequency from 100ms to 10ms, and limit the buffer size to 32MB. This effectively imposes an artificial 3,200MB/sec limit on throughput, but hopefully that's fast enough that we'll have a "wake on readable" mechanism by the time it's a problem.

Test Plan:
  - Replaced `mysqldump` with `cat /dev/zero` as the source command, to get fast input.
  - Ran `bin/storage dump` with `var_dump()` on the buffer size.
  - Before change: saw arbitrarily large buffers (300MB+).
  - After change: saw consistent maximum buffer size of 32MB.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13328

Differential Revision: https://secure.phabricator.com/D20617
---
 .../workflow/PhabricatorStorageManagementDumpWorkflow.php  | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
index c3b9a32327..28b188a873 100644
--- a/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
+++ b/src/infrastructure/storage/management/workflow/PhabricatorStorageManagementDumpWorkflow.php
@@ -285,10 +285,13 @@ final class PhabricatorStorageManagementDumpWorkflow
         $preamble = implode('', $preamble);
         $this->writeData($preamble, $file, $is_compress, $output_file);
 
-        $future = new ExecFuture('%C', $spec['command']);
+        // See T13328. The "mysql" command may produce output very quickly.
+        // Don't buffer more than a fixed amount.
+        $future = id(new ExecFuture('%C', $spec['command']))
+          ->setReadBufferSize(32 * 1024 * 1024);
 
         $iterator = id(new FutureIterator(array($future)))
-          ->setUpdateInterval(0.100);
+          ->setUpdateInterval(0.010);
         foreach ($iterator as $ready) {
           list($stdout, $stderr) = $future->read();
           $future->discardBuffers();