@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Apply storage patches patch-by-patch, not database-by-database

Summary:
Ref T11044. Sometimes we have a sequence of patches like this:

- `01.newtable.sql`: Adds a new table to Files.
- `02.movedata.php`: Moves some data that used to live in Tokens to the new table.

This is fairly rare, but not unheard of. More commonly, we can have this sequence:

- `03.newtable.sql`: Add a new column to Phame.
- `04.setvalue.php`: Copy data into that new column.

In both cases, when applying database-by-database, we can get into trouble.

- In the first case, if Files is on a different master, we'll try to move data into a new table before creating the table.
- In the second case, if Phame is on a different master, the PHP patch will connect to it before we add the new column.

In either case, we try to interact with tables or columns which don't exist yet.

Instead, apply each patch in order, to all databases which need it. So we'll apply `01.newtable.sql` EVERYWHERE first, then move on.

In the case of PHP patches, we also now only apply them once, since they never make schema changes. It should normally be OK to apply them more than once safely, but this is a little faster and a little safer if we ever make a mistake.

Test Plan:
- Ran `bin/storage upgrade` on single-host and clustered setups.
- Initialized new storage on single-host and clustered setups.
- Upgraded again after initialization.
- Ran with `--apply`.
- Ran with `--dry-run`.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11044

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

+200 -80
+4
src/infrastructure/storage/management/PhabricatorStoragePatch.php
··· 48 48 return $this->dead; 49 49 } 50 50 51 + public function getIsGlobalPatch() { 52 + return ($this->getType() == 'php'); 53 + } 54 + 51 55 }
+7 -7
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementUpgradeWorkflow.php
··· 75 75 76 76 $apis = $this->getMasterAPIs(); 77 77 78 - foreach ($apis as $api) { 79 - $this->upgradeSchemata($api, $apply_only, $no_quickstart, $init_only); 78 + $this->upgradeSchemata($apis, $apply_only, $no_quickstart, $init_only); 80 79 81 - if ($no_adjust || $init_only || $apply_only) { 82 - $console->writeOut( 83 - "%s\n", 84 - pht('Declining to apply storage adjustments.')); 85 - } else { 80 + if ($no_adjust || $init_only || $apply_only) { 81 + $console->writeOut( 82 + "%s\n", 83 + pht('Declining to apply storage adjustments.')); 84 + } else { 85 + foreach ($apis as $api) { 86 86 $err = $this->adjustSchemata($api, false); 87 87 if ($err) { 88 88 return $err;
+189 -73
src/infrastructure/storage/management/workflow/PhabricatorStorageManagementWorkflow.php
··· 819 819 } 820 820 821 821 final protected function upgradeSchemata( 822 - PhabricatorStorageManagementAPI $api, 822 + array $apis, 823 823 $apply_only = null, 824 824 $no_quickstart = false, 825 825 $init_only = false) { 826 826 827 - $lock = $this->lock($api); 827 + $locks = array(); 828 + foreach ($apis as $api) { 829 + $ref_key = $api->getRef()->getRefKey(); 830 + $locks[] = $this->lock($api); 831 + } 828 832 829 833 try { 830 - $this->doUpgradeSchemata($api, $apply_only, $no_quickstart, $init_only); 834 + $this->doUpgradeSchemata($apis, $apply_only, $no_quickstart, $init_only); 831 835 } catch (Exception $ex) { 832 - $lock->unlock(); 836 + foreach ($locks as $lock) { 837 + $lock->unlock(); 838 + } 833 839 throw $ex; 834 840 } 835 841 836 - $lock->unlock(); 842 + foreach ($locks as $lock) { 843 + $lock->unlock(); 844 + } 837 845 } 838 846 839 847 final private function doUpgradeSchemata( 840 - PhabricatorStorageManagementAPI $api, 848 + array $apis, 841 849 $apply_only, 842 850 $no_quickstart, 843 851 $init_only) { 844 852 845 853 $patches = $this->patches; 854 + $is_dryrun = $this->dryRun; 846 855 847 - $applied = $api->getAppliedPatches(); 848 - if ($applied === null) { 849 - if ($this->dryRun) { 850 - echo pht( 851 - "DRYRUN: Patch metadata storage doesn't exist yet, ". 852 - "it would be created.\n"); 853 - return 0; 856 + $api_map = array(); 857 + foreach ($apis as $api) { 858 + $api_map[$api->getRef()->getRefKey()] = $api; 859 + } 860 + 861 + foreach ($api_map as $ref_key => $api) { 862 + $applied = $api->getAppliedPatches(); 863 + 864 + $needs_init = ($applied === null); 865 + if (!$needs_init) { 866 + continue; 867 + } 868 + 869 + if ($is_dryrun) { 870 + echo tsprintf( 871 + "%s\n", 872 + pht( 873 + 'DRYRUN: Storage on host "%s" does not exist yet, so it '. 874 + 'would be created.', 875 + $ref_key)); 876 + continue; 854 877 } 855 878 856 879 if ($apply_only) { 857 880 throw new PhutilArgumentUsageException( 858 881 pht( 859 - 'Storage has not been initialized yet, you must initialize '. 860 - 'storage before selectively applying patches.')); 861 - return 1; 882 + 'Storage on host "%s" has not been initialized yet. You must '. 883 + 'initialize storage before selectively applying patches.', 884 + $ref_key)); 862 885 } 863 886 864 - // If we're initializing storage for the first time, track it so that 865 - // we can give the user a nicer experience during the subsequent 866 - // adjustment phase. 887 + // If we're initializing storage for the first time on any host, track 888 + // it so that we can give the user a nicer experience during the 889 + // subsequent adjustment phase. 867 890 $this->didInitialize = true; 868 891 869 892 $legacy = $api->getLegacyPatches($patches); 870 893 if ($legacy || $no_quickstart || $init_only) { 871 - 872 894 // If we have legacy patches, we can't quickstart. 873 - 874 895 $api->createDatabase('meta_data'); 875 896 $api->createTable( 876 897 'meta_data', ··· 884 905 $api->markPatchApplied($patch); 885 906 } 886 907 } else { 887 - echo pht('Loading quickstart template...')."\n"; 908 + echo tsprintf( 909 + "%s\n", 910 + pht( 911 + 'Loading quickstart template onto "%s"...', 912 + $ref_key)); 913 + 888 914 $root = dirname(phutil_get_library_root('phabricator')); 889 915 $sql = $root.'/resources/sql/quickstart.sql'; 890 916 $api->applyPatchSQL($sql); ··· 896 922 return 0; 897 923 } 898 924 899 - $applied = $api->getAppliedPatches(); 900 - $applied = array_fuse($applied); 925 + $applied_map = array(); 926 + foreach ($api_map as $ref_key => $api) { 927 + $applied = $api->getAppliedPatches(); 901 928 902 - $skip_mark = false; 903 - if ($apply_only) { 904 - if (isset($applied[$apply_only])) { 929 + // If we still have nothing applied, this is a dry run and we didn't 930 + // actually initialize storage. Here, just do nothing. 931 + if ($applied === null) { 932 + if ($is_dryrun) { 933 + continue; 934 + } else { 935 + throw new Exception( 936 + pht( 937 + 'Database initialization on host "%s" applied no patches!', 938 + $ref_key)); 939 + } 940 + } 905 941 906 - unset($applied[$apply_only]); 907 - $skip_mark = true; 942 + $applied = array_fuse($applied); 908 943 909 - if (!$this->force && !$this->dryRun) { 910 - echo phutil_console_wrap( 911 - pht( 912 - "Patch '%s' has already been applied. Are you sure you want ". 913 - "to apply it again? This may put your storage in a state ". 914 - "that the upgrade scripts can not automatically manage.", 915 - $apply_only)); 916 - if (!phutil_console_confirm(pht('Apply patch again?'))) { 917 - echo pht('Cancelled.')."\n"; 918 - return 1; 944 + if ($apply_only) { 945 + if (isset($applied[$apply_only])) { 946 + if (!$this->force && !$is_dryrun) { 947 + echo phutil_console_wrap( 948 + pht( 949 + 'Patch "%s" has already been applied on host "%s". Are you '. 950 + 'sure you want to apply it again? This may put your storage '. 951 + 'in a state that the upgrade scripts can not automatically '. 952 + 'manage.', 953 + $apply_only, 954 + $ref_key)); 955 + if (!phutil_console_confirm(pht('Apply patch again?'))) { 956 + echo pht('Cancelled.')."\n"; 957 + return 1; 958 + } 919 959 } 960 + 961 + // Mark this patch as not yet applied on this host. 962 + unset($applied[$apply_only]); 920 963 } 921 964 } 965 + 966 + $applied_map[$ref_key] = $applied; 922 967 } 923 968 969 + // If we're applying only a specific patch, select just that patch. 970 + if ($apply_only) { 971 + $patches = array_select_keys($patches, array($apply_only)); 972 + } 973 + 974 + // Apply each patch to each database. We apply patches patch-by-patch, 975 + // not database-by-database: for each patch we apply it to every database, 976 + // then move to the next patch. 977 + 978 + // We must do this because ".php" patches may depend on ".sql" patches 979 + // being up to date on all masters, and that will work fine if we put each 980 + // patch on every host before moving on. If we try to bring database hosts 981 + // up to date one at a time we can end up in a big mess. 982 + 983 + $duration_map = array(); 924 984 while (true) { 925 985 $applied_something = false; 926 986 foreach ($patches as $key => $patch) { 927 - if (isset($applied[$key])) { 928 - unset($patches[$key]); 929 - continue; 987 + // First, check if any databases need this patch. We can just skip it 988 + // if it has already been applied everywhere. 989 + $need_patch = array(); 990 + foreach ($applied_map as $ref_key => $applied) { 991 + if (isset($applied[$key])) { 992 + continue; 993 + } 994 + $need_patch[] = $ref_key; 930 995 } 931 996 932 - if ($apply_only && $apply_only != $key) { 997 + if (!$need_patch) { 933 998 unset($patches[$key]); 934 999 continue; 935 1000 } 936 1001 937 - $can_apply = true; 1002 + // Check if we can apply this patch yet. Before we can apply a patch, 1003 + // all of the dependencies for the patch must have been applied on all 1004 + // databases. Requiring that all databases stay in sync prevents one 1005 + // database from racing ahead if it happens to get a patch that nothing 1006 + // else has yet. 1007 + $missing_patch = null; 938 1008 foreach ($patch->getAfter() as $after) { 939 - if (empty($applied[$after])) { 940 - if ($apply_only) { 941 - echo pht( 942 - "Unable to apply patch '%s' because it depends ". 943 - "on patch '%s', which has not been applied.\n", 944 - $apply_only, 945 - $after); 946 - return 1; 1009 + foreach ($applied_map as $ref_key => $applied) { 1010 + if (isset($applied[$after])) { 1011 + // This database already has the patch. We can apply it to 1012 + // other databases but don't need to apply it here. 1013 + continue; 947 1014 } 948 - $can_apply = false; 949 - break; 1015 + 1016 + $missing_patch = $after; 1017 + break 2; 950 1018 } 951 1019 } 952 1020 953 - if (!$can_apply) { 954 - continue; 1021 + if ($missing_patch) { 1022 + if ($apply_only) { 1023 + echo tsprintf( 1024 + "%s\n", 1025 + pht( 1026 + 'Unable to apply patch "%s" because it depends on patch '. 1027 + '"%s", which has not been applied on some hosts: %s.', 1028 + $apply_only, 1029 + $missing_patch, 1030 + implode(', ', $need_patch))); 1031 + return 1; 1032 + } else { 1033 + // Some databases are missing the dependencies, so keep trying 1034 + // other patches instead. If everything goes right, we'll apply the 1035 + // dependencies and then come back and apply this patch later. 1036 + continue; 1037 + } 955 1038 } 956 1039 957 - $applied_something = true; 1040 + $is_global = $patch->getIsGlobalPatch(); 1041 + $patch_apis = array_select_keys($api_map, $need_patch); 1042 + foreach ($patch_apis as $ref_key => $api) { 1043 + if ($is_global) { 1044 + // If this is a global patch which we previously applied, just 1045 + // read the duration from the map without actually applying 1046 + // the patch. 1047 + $duration = idx($duration_map, $key); 1048 + } else { 1049 + $duration = null; 1050 + } 1051 + 1052 + if ($duration === null) { 1053 + if ($is_dryrun) { 1054 + echo tsprintf( 1055 + "%s\n", 1056 + pht( 1057 + 'DRYRUN: Would apply patch "%s" to host "%s".', 1058 + $key, 1059 + $ref_key)); 1060 + } else { 1061 + echo tsprintf( 1062 + "%s\n", 1063 + pht( 1064 + 'Applying patch "%s" to host "%s"...', 1065 + $key, 1066 + $ref_key)); 1067 + } 958 1068 959 - if ($this->dryRun) { 960 - echo pht("DRYRUN: Would apply patch '%s'.", $key)."\n"; 961 - } else { 962 - echo pht("Applying patch '%s'...", $key)."\n"; 1069 + $t_begin = microtime(true); 1070 + $api->applyPatch($patch); 1071 + $t_end = microtime(true); 963 1072 964 - $t_begin = microtime(true); 965 - $api->applyPatch($patch); 966 - $t_end = microtime(true); 1073 + $duration = ($t_end - $t_begin); 1074 + $duration_map[$key] = $duration; 1075 + } 967 1076 968 - if (!$skip_mark) { 1077 + // If we're explicitly reapplying this patch, we don't need to 1078 + // mark it as applied. 1079 + if (!isset($applied_map[$ref_key][$key])) { 969 1080 $api->markPatchApplied($key, ($t_end - $t_begin)); 1081 + $applied_map[$ref_key][$key] = true; 970 1082 } 971 1083 } 972 1084 1085 + // We applied this everywhere, so we're done with the patch. 973 1086 unset($patches[$key]); 974 - $applied[$key] = true; 1087 + $applied_something = true; 975 1088 } 976 1089 977 1090 if (!$applied_something) { 978 - if (count($patches)) { 1091 + if ($patches) { 979 1092 throw new Exception( 980 1093 pht( 981 - 'Some patches could not be applied to "%s": %s', 982 - $api->getRef()->getRefKey(), 1094 + 'Some patches could not be applied: %s', 983 1095 implode(', ', array_keys($patches)))); 984 - } else if (!$this->dryRun && !$apply_only) { 1096 + } else if (!$is_dryrun && !$apply_only) { 985 1097 echo pht( 986 - 'Storage is up to date on "%s". Use "%s" for details.', 987 - $api->getRef()->getRefKey(), 1098 + 'Storage is up to date. Use "%s" for details.', 988 1099 'storage status')."\n"; 989 1100 } 990 1101 break; ··· 1013 1124 * @return PhabricatorGlobalLock 1014 1125 */ 1015 1126 final protected function lock(PhabricatorStorageManagementAPI $api) { 1016 - return PhabricatorGlobalLock::newLock(__CLASS__) 1127 + // Although we're holding this lock on different databases so it could 1128 + // have the same name on each as far as the database is concerned, the 1129 + // locks would be the same within this process. 1130 + $lock_name = 'adjust/'.$api->getRef()->getRefKey(); 1131 + 1132 + return PhabricatorGlobalLock::newLock($lock_name) 1017 1133 ->useSpecificConnection($api->getConn(null)) 1018 1134 ->lock(); 1019 1135 }