Fixes T8475. Fixes T8623. A long time ago (in D9290), a new hunk storage was added. From this point, new Differential diffs were written to the "modern" storage, whilst existing diffs were kept in the "legacy" storage. A command line script (./bin/hunks migrate) was provided to migrate data from legacy storage to modern storage, but this script only migrates one-hunk-at-a-time. Given that this code has proven to be stable (all of the hunks on our install were migrated over 6 months ago, and @epriestley mentioned that https://secure.phabricator.com did the same), we should force all installs to migrate hunks to modern storage.
Details
Details
- Reviewers
joshuaspence - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T8623: Upgrading: Differential Hunk Migration
T8475: Clean up remnants of old Differential hunk storage
Checked out the following versions of the code base:
| Repository | Commit |
|---|---|
| rARC Arcanist | rARC50caec620a8ed45c54323cb71fee72fd0d935115 |
| rPHU libphutil | rPHU1e025a87a4cbd987ecabf9924d20412e24e6cf87 |
| rP Phabricator | rP4b9765b89672d5d592580d6001e6763db5ee0358 |
Created a few diffs and then upgraded to HEAD. Applied migrations successfully and then browsed around migrated diffs in the UI.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 6777 Build 19474: Run Core Tests Build 6799: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
This seems generally reasonable, but I want to give installs a little more warning:
- I'll issue guidance in the next couple changelogs to do this yourself (it can be done while Phabricator is running, cancelled/resumed safely, etc), so it's better for large installs if they do this prior to a normal upgrade.
- After the final warning, we can land this and force everyone else through it.
| src/applications/differential/query/DifferentialHunkQuery.php | ||
|---|---|---|
| 33–34 | $all_results and comment both obsolete now? | |
Comment Actions
@joshuaspence I'm curious about the status of this. It doesn't cleanly apply to HEAD but still seems like something that is rather useful to get done for both performance and code cleanlyness.