Straighten out replication/cache behavior in "bin/storage dump"

Summary:
Fixes T13336.

  - Prevent `--no-indexes` from being combined with `--for-replica`, since combining these options can only lead to heartbreak.
  - In `--for-replica` mode, dump caches too. See discussion in T13336. It is probably "safe" to not dump these today, but fragile and not correct.
  - Mark the "MarkupCache" table as having "Cache" persistence, not "Data" persistence (no need to back it up, since it can be fully regenerated from other datasources).

Test Plan: Ran `bin/storage dump` with various combinations of flags.

Maniphest Tasks: T13336

Differential Revision: https://secure.phabricator.com/D20743

src/applications/cache/storage/PhabricatorMarkupCache.php

@@ -30,4 +30,8 @@ final class PhabricatorMarkupCache extends PhabricatorCacheDAO {
     ) + parent::getConfiguration();
   }
 
+  public function getSchemaPersistence() {
+    return PhabricatorConfigTableSchema::PERSISTENCE_CACHE;
+  }
+
 }

src/applications/config/schema/PhabricatorConfigSchemaSpec.php

@@ -48,11 +48,19 @@ abstract class PhabricatorConfigSchemaSpec extends Phobject {
   abstract public function buildSchemata();
 
   protected function buildLiskObjectSchema(PhabricatorLiskDAO $object) {
+    $index_options = array();
+
+    $persistence = $object->getSchemaPersistence();
+    if ($persistence !== null) {
+      $index_options['persistence'] = $persistence;
+    }
+
     $this->buildRawSchema(
       $object->getApplicationName(),
       $object->getTableName(),
       $object->getSchemaColumns(),
-      $object->getSchemaKeys());
+      $object->getSchemaKeys(),
+      $index_options);
   }
 
   protected function buildFerretIndexSchema(PhabricatorFerretEngine $engine) {

src/infrastructure/storage/lisk/LiskDAO.php

@@ -1881,6 +1881,10 @@ abstract class LiskDAO extends Phobject
       ->getMaximumByteLengthForDataType($data_type);
   }
 
+  public function getSchemaPersistence() {
+    return null;
+  }
+
 
 /* -(  AphrontDatabaseTableRefInterface  )----------------------------------- */
 

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

@@ -14,7 +14,8 @@ final class PhabricatorStorageManagementDumpWorkflow
             'name' => 'for-replica',
             'help' => pht(
               'Add __--master-data__ to the __mysqldump__ command, '.
-              'generating a CHANGE MASTER statement in the output.'),
+              'generating a CHANGE MASTER statement in the output. This '.
+              'option also dumps all data, including caches.'),
           ),
           array(
             'name' => 'output',
@@ -54,6 +55,8 @@ final class PhabricatorStorageManagementDumpWorkflow
     $output_file = $args->getArg('output');
     $is_compress = $args->getArg('compress');
     $is_overwrite = $args->getArg('overwrite');
+    $is_noindex = $args->getArg('no-indexes');
+    $is_replica = $args->getArg('for-replica');
 
     if ($is_compress) {
       if ($output_file === null) {
@@ -79,6 +82,14 @@ final class PhabricatorStorageManagementDumpWorkflow
       }
     }
 
+    if ($is_replica && $is_noindex) {
+      throw new PhutilArgumentUsageException(
+        pht(
+          'The "--for-replica" flag can not be used with the '.
+          '"--no-indexes" flag. Replication dumps must contain a complete '.
+          'representation of database state.'));
+    }
+
     if ($output_file !== null) {
       if (Filesystem::pathExists($output_file)) {
         if (!$is_overwrite) {
@@ -94,8 +105,6 @@ final class PhabricatorStorageManagementDumpWorkflow
     $api = $this->getSingleAPI();
     $patches = $this->getPatches();
 
-    $with_indexes = !$args->getArg('no-indexes');
-
     $applied = $api->getAppliedPatches();
     if ($applied === null) {
       throw new PhutilArgumentUsageException(
@@ -119,6 +128,9 @@ final class PhabricatorStorageManagementDumpWorkflow
     $schemata = $actual_map[$ref_key];
     $expect = $expect_map[$ref_key];
 
+    $with_caches = $is_replica;
+    $with_indexes = !$is_noindex;
+
     $targets = array();
     foreach ($schemata->getDatabases() as $database_name => $database) {
       $expect_database = $expect->getDatabase($database_name);
@@ -143,7 +155,7 @@ final class PhabricatorStorageManagementDumpWorkflow
             // When dumping tables, leave the data in cache tables in the
             // database. This will be automatically rebuild after the data
             // is restored and does not need to be persisted in backups.
-            $with_data = false;
+            $with_data = $with_caches;
             break;
           case PhabricatorConfigTableSchema::PERSISTENCE_INDEX:
             // When dumping tables, leave index data behind of the caller
@@ -183,7 +195,7 @@ final class PhabricatorStorageManagementDumpWorkflow
     $argv[] = '--default-character-set';
     $argv[] = $api->getClientCharset();
 
-    if ($args->getArg('for-replica')) {
+    if ($is_replica) {
       $argv[] = '--master-data';
     }
 
@@ -342,7 +354,6 @@ final class PhabricatorStorageManagementDumpWorkflow
     return 0;
   }
 
-
   private function writeData($data, $file, $is_compress, $output_file) {
     if (!strlen($data)) {
       return;