Execute project membership materialization as "SELECT" + "INSERT", not "INSERT ... SELECT"

Summary:
Ref T13596. See that task for discussion. Executing "INSERT ... SELECT" at default isolation levels requires more locking than executing "SELECT" + "INSERT" separately.

Decompose this "INSERT ... SELECT" into "SELECT + INSERT", and reformat it to execute a minimal set of changes instead of wiping everything out and then writing all of it back. In most cases, this means we write 1 row instead of `O(number of project members)` rows.

Test Plan:
- Created a project. Added and removed members, looked at database and saw a consistent membership/materialization list.
- Created a subproject. Added and removed members, looked at database and saw a consistent membership/materialization list.

I wasn't successful in reproducing the LOCK WAIT issue locally by trying various concurrent SELECT / INSERT / INSERT ... SELECT strategies. It may depend on the "DELETE + INSERT ... SELECT" structure used here, or versions/config/etc, so we'll have to see how that fares in production.

Maniphest Tasks: T13596

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

epriestley 5 years ago 9cbbbe2a d6fd3657

+70 -10

1 changed file

expand all

src

applications

project

engineextension

PhabricatorProjectsMembershipIndexEngineExtension.php

+70 -10

src/applications/project/engineextension/PhabricatorProjectsMembershipIndexEngineExtension.php

··· 73 73 74 74 $project->openTransaction(); 75 75 76 - // Delete any existing materialized member edges. 77 - queryfx( 76 + // Copy current member edges to create new materialized edges. 77 + 78 + // See T13596. Avoid executing this as an "INSERT ... SELECT" to reduce 79 + // the required level of table locking. Since we're decomposing it into 80 + // "SELECT" + "INSERT" anyway, we can also compute exactly which rows 81 + // need to be modified. 82 + 83 + $have_rows = queryfx_all( 78 84 $conn_w, 79 - 'DELETE FROM %T WHERE src = %s AND type = %s', 85 + 'SELECT dst FROM %T 86 + WHERE src = %s AND type = %d', 80 87 PhabricatorEdgeConfig::TABLE_NAME_EDGE, 81 88 $project_phid, 82 89 $material_type); 83 90 84 - // Copy current member edges to create new materialized edges. 85 - queryfx( 91 + $want_rows = queryfx_all( 86 92 $conn_w, 87 - 'INSERT IGNORE INTO %T (src, type, dst, dateCreated, seq) 88 - SELECT %s, %d, dst, dateCreated, seq FROM %T 93 + 'SELECT dst, dateCreated, seq FROM %T 89 94 WHERE src IN (%Ls) AND type = %d', 90 95 PhabricatorEdgeConfig::TABLE_NAME_EDGE, 91 - $project_phid, 92 - $material_type, 93 - PhabricatorEdgeConfig::TABLE_NAME_EDGE, 94 96 $source_phids, 95 97 $member_type); 98 + 99 + $have_phids = ipull($have_rows, 'dst', 'dst'); 100 + $want_phids = ipull($want_rows, null, 'dst'); 101 + 102 + $rem_phids = array_diff_key($have_phids, $want_phids); 103 + $rem_phids = array_keys($rem_phids); 104 + 105 + $add_phids = array_diff_key($want_phids, $have_phids); 106 + $add_phids = array_keys($add_phids); 107 + 108 + $rem_sql = array(); 109 + foreach ($rem_phids as $rem_phid) { 110 + $rem_sql[] = qsprintf( 111 + $conn_w, 112 + '%s', 113 + $rem_phid); 114 + } 115 + 116 + $add_sql = array(); 117 + foreach ($add_phids as $add_phid) { 118 + $add_row = $want_phids[$add_phid]; 119 + $add_sql[] = qsprintf( 120 + $conn_w, 121 + '(%s, %d, %s, %d, %d)', 122 + $project_phid, 123 + $material_type, 124 + $add_row['dst'], 125 + $add_row['dateCreated'], 126 + $add_row['seq']); 127 + } 128 + 129 + // Remove materialized members who are no longer project members. 130 + 131 + if ($rem_sql) { 132 + foreach (PhabricatorLiskDAO::chunkSQL($rem_sql) as $sql_chunk) { 133 + queryfx( 134 + $conn_w, 135 + 'DELETE FROM %T 136 + WHERE src = %s AND type = %s AND dst IN (%LQ)', 137 + PhabricatorEdgeConfig::TABLE_NAME_EDGE, 138 + $project_phid, 139 + $material_type, 140 + $sql_chunk); 141 + } 142 + } 143 + 144 + // Add project members who are not yet materialized members. 145 + 146 + if ($add_sql) { 147 + foreach (PhabricatorLiskDAO::chunkSQL($add_sql) as $sql_chunk) { 148 + queryfx( 149 + $conn_w, 150 + 'INSERT IGNORE INTO %T (src, type, dst, dateCreated, seq) 151 + VALUES %LQ', 152 + PhabricatorEdgeConfig::TABLE_NAME_EDGE, 153 + $sql_chunk); 154 + } 155 + } 96 156 97 157 // Update the hasSubprojects flag. 98 158 queryfx(

Configure Feed

Configure Feed