Page MenuHomePhabricator

Assign PHIDs to all diffs
ClosedPublic

Authored by epriestley on Nov 6 2013, 2:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Apr 19, 3:27 AM
Unknown Object (File)
Fri, Apr 19, 3:27 AM
Unknown Object (File)
Fri, Apr 19, 3:27 AM
Unknown Object (File)
Fri, Apr 19, 3:27 AM
Unknown Object (File)
Mon, Apr 15, 3:21 AM
Unknown Object (File)
Fri, Apr 12, 11:46 PM
Unknown Object (File)
Thu, Apr 11, 4:38 AM
Unknown Object (File)
Sun, Mar 31, 9:14 AM
Subscribers

Details

Summary

Ref T1049. Ref T2222. DifferentialDiff does not currently have a PHID, but we need it for Harbormaster and ApplicationTransactions. See some discussion in D7501.

(I split the SQL into two sections so we can't fail in the middle. At some point, I'd like to do a pass on the migration stuff and get this happening automatically, and also simplify the PatchList.)

Test Plan
  • Ran bin/storage upgrade.
  • Checked for valid PHIDs in the database.
  • Used phid.query to look up a diff by PHID.
  • Created a new diff and verified it got a PHID.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

Exciting time

resources/sql/patches/20131106.diffphid.3.mig.php
6โ€“22 โ†—(On Diff #16939)

can you do more of a chunking strategy like in D6789? this is going to be sloooooow for most as I assume this is a pretty big table

Haha -- I was just about to write PhutilChunkedIterator to try to make the chunking a bit easier, but @vrana already did it. I'll switch to using that. It probably would have made D6789 easier too.

epriestley updated this revision to Unknown Object (????).Nov 6 2013, 7:22 PM
  • Use PhutilChunkedIterator to chunk the rows.
  • Use chunkSQL to chunk the SQL.
  • Use INSERT IGNORE ... ON DUPLICATE KEY UPDATE to avoid specifying all the columns with no defaults. This appears to work correctly.
  • Sequence these as: add column, migrate, add key, so the key can be unique.
  • Make the key unique.
  • Ran ./bin/storage upgrade --force --apply phabricator:20131106.diffphid.2.mig.php --trace to verify this still works. Saw a giant batched chunk query.

An abstraction which might make this easier is some kind of AutochunkSQLQuery, which might look like this:

$auto_sql = new AutochunkSQLQuery(
  $conn_w,
  'SELECT .. %T .. %Q ... %s ...',
  $table_name,
  AutochunkSQLQuery::BIND,
  $whatever);

Then you just:

$auto_sql->addItem('(%d, %s)', $id, $phid);

When the internal buffer gets big enough, it runs the query. At the end, you:

$auto_sql->flush();

...and it runs a final query with whatever's in its buffer.

Maybe the next time we have one of these that'll be worth writing.