We should explicitly warn users when storage patches have not been applied. In 90% of cases they hit the "missing column, go run storage upgrade" exception, but for some migrations they may not. We should just have an explicit check for this.
Description
Description
Event Timeline
Comment Actions
We do already have such a check, and it seems to be working properly.
Original report is from Twitter, I'll follow up there and see if I can get some more details, but I'm guessing this was just a case of git pull with no other upgrade steps, which is hard to detect without starting to sacrifice performance.
Comment Actions
Assuming I'm right about the root cause, I think we can do this relatively cheaply:
- During setup, read .git/refs/heads/master if it exists.
- If it doesn't exist, assume the install has everything under control (nonstandard deployment, etc) and bail.
- Check for some "current version" key in APC or the equivalent cache.
- If the key does not exist, write the ref we just read and bail. This should happen on the first load after restart.
- If the key does exist, compare it to the ref we just read.
- If the key differs, raise an error: "looks like you updated Phabricator but haven't restarted your webserver. You need to restart apache/nginx/php-fpm for changes to take effect".
This costs us a disk read per request, which is not ideal, but it should be coming out of disk cache basically all the time, and this error is relatively easy to make and comes up somewhat often. If this ever shows up in a profile, we could add a tuning flag for it.
Comment Actions
This hasn't come up too often recently, and I haven't developed a better approach for it than the one outlined above, which is quite complicated. I don't think this is currently coming up often enough to justify the complexity of trying to add the check for it.