LiskMigrationIterator improves the peak memory usage but won't let us do the needCommitData() part cheaply, so I suspect the migration would become dramatically more expensive if we only did that swap. In theory, we could write some kind of QueryIterator which takes a CursorPagedPolicyAwareQuery and pages through it. This seems like a generally reasonable sort of thing to write, but I don't think we have one right now.

(I'll go through this in more detail, but I don't think we're aiming for this release on it no matter what, so you've got a minimum of a week to hyperventilate about deploying this. 🐱)

so you've got a minimum of a week to hyperventilate about deploying this.

thank god :D

I think this is in pretty good shape, but we should probably do something like this:

• put it in bin/repository rebuild-identities <repository | commit | --all> or similar;
• ship that for at least a week with changelog guidance so installs can do an online rebuild at their leisure if they want;
• when we do the migration, basically just have it run bin/repository rebuild-identities --all.

This will also put us in a better position to change how DiffusionResolveUserQuery works later (e.g., drop the "Real Name" rule) and let installs rebuild the guesses easily.

I'll see what I can do about providing a QueryIterator, I think that's probably the best way forward for dealing with the commit data + iterator memory usage issue and that it shouldn't be very involved. Putting this in a repeatable bin/repository workflow will also let us debug/test that more flexibly.

I think D19450 will give us an Iterator which can take a Query and iterate over all results with constant memory usage.

when we do the migration, basically just have it run bin/repository rebuild-identities --all

I rewrote the migration as PhabricatorRepositoryManagementRebuildIdentitiesWorkflow. What's the most straightforward way to invoke that via a migration? Something like this:

<?php
execx('bin/repository rebuild-identities');

Or this:

<?php
$workflow = new PhabricatorRepositoryManagementRebuildIdentitiesWorkflow();
$args = getFakedOutArgs();
$workflow->execute($args);

Or put the code in a class that doesn't extend Workflow and have both the migration and the workflow instantiate it?

I think we can't just execx() / phutil_passthru() because it won't inherit some of the bin/storage state -- flags like bin/storage --namespace xyz, particularly.

So I'd try the PhabricatorRepositoryManagementRebuildIdentitiesWorkflow approach, I think.