[Fixed!] 2021 Week 8 - storage upgrade fails with access denied

Bug
#1

Reproduction Instructions
Complete steps which allow someone else who does not have access to your environment to reproduce the bug.

Applying the database patch from ⚙ D21570 Improve performance of "phabricator:20210215.changeset.02.phid-populate.php" fails with an access denied exception somehow when running bin/storage upgrade:

Applying patch "phabricator:20210215.changeset.02.phid-populate.php" to host "our_db_server"...
[2021-02-26 14:46:49] EXCEPTION: (AphrontAccessDeniedQueryException) #1044: Access denied for user 'phabricator'@'our_phabricator_servver' to database 'phabricator_differential' at [<phabricator>/src/infrastructure/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:350]

Loading Phabricator displays this message:

Run the storage upgrade script to upgrade databases (host “our_database_server” is out of date). Missing patches: phabricator:20210215.changeset.02.phid-populate.php, phabricator:20210216.index.01.version.sql, phabricator:20210216.index.02.epoch.sql.

and suggests running bin/storage upgrade.

Phabricator/Arcanist Version
Output from Config > Version Information or arc version.

Latest stable version 05745dfd024b1d5293cc1c894c26259f19975e80 “(stable) Promote 2021 Week 8”.

We’ve been running on the same database, applying updates and upgrading storage as we go, for several years now. MySQL 8.0.23 on Ubuntu.

#2

Does the mysql user has grants CREATE TEMPORARY TABLES and DROP?

(As a rule, I think we require GRANT ALL for running storage upgrades.)

#3

Pretty sure they were given GRANT ALL when I created them many moons ago. Also, pretty sure this would’ve caused a problem before now.

SHOW GRANTS for the phabricator user lists:

GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, REFERENCES, INDEX, ALTER, LOCK TABLES, EXECUTE, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, TRIGGER ON . TO phabricator@localhost

#4

Try adding CREATE TEMPORARY TABLES - this is the only instance I can see in the code of temp tables.

#5

Granting CREATE TEMPORARY TABLES to phabricator on *.* did the trick, the upgrade completed successfully!

Thanks @avivey!

#6

Yeah, this is the first time migrations have required temporary tables. For context, see https://secure.phabricator.com/T13613.

The performance improvement of using a temporary table over individual UPDATE statements is substantial (>3X faster) and it’s much simpler than INSERT ... ON DUPLICATE KEY UPDATE ... (you don’t need to provide default values for all columns with no default value), so I’d like to be able to continue using temporary tables in storage migrations.

It hasn’t been a terribly long time, but I haven’t seen other reports of this so far, which gives me some confidence that this permission is likely fairly common. It’s not clear to me why the user here would have all those other permissions but not TEMPORARY TABLES, but if the root cause was something like “in many versions of MySQL, GRANT ALL does not grant TEMPORARY TABLES”, I’d expect to see more reports of this.

I could add a setup warning, but setup warnings are probably too late to be helpful in cases like this. In particular, if I added a setup warning about TEMPORARY TABLES now, all existing installs wouldn’t be able to see it until resolving this.

That said, bin/storage upgrade could do a self-test for required permissions before running migrations, and raise a more helpful error. The 1044 error is also opaque and could be contextualized. I filed these improvements upstream in https://secure.phabricator.com/T13622.

#7

I improved error behavior a bit in https://secure.phabricator.com/D21608. Actually parsing SHOW GRANTS seems like more trouble than it’s worth, though.

#8