Page MenuHomePhabricator

Fix a possible deadlock in unit tests after an error
ClosedPublic

Authored by epriestley on Dec 24 2015, 2:43 PM.
Tags
None
Referenced Files
F13996391: D14875.id.diff
Wed, Oct 23, 8:49 PM
F13955704: D14875.id35952.diff
Mon, Oct 14, 2:37 AM
Unknown Object (File)
Oct 2 2024, 6:49 AM
Unknown Object (File)
Sep 20 2024, 10:15 PM
Unknown Object (File)
Sep 2 2024, 11:46 PM
Unknown Object (File)
Aug 29 2024, 2:04 AM
Unknown Object (File)
Aug 19 2024, 2:25 PM
Unknown Object (File)
Aug 18 2024, 10:57 AM
Subscribers
None

Details

Summary

After certain types of errors, we may deadlock when trying to destroy test databases.

Specifically, we still have connections open to, say, phabricator_unittest_abasonaknlbaklnasb_herald (or whatever) and MySQL sometimes (not sure exactly when?) waits for them before destorying the database.

Test Plan
  • Added $m = null; $m->method() to a fixture test to force a fatal.
  • Saw consistent deadlock, with storage destroy never exiting.
  • Added --trace to the storage destroy command and made it use phutil_passthru() so I could see what was happening.
  • Saw it hang on some arbitrary database.
  • Conneced to MySQL, used show full processlist; to see what was wrong.
  • Saw the DROP DATABASE ... command waiting for locks to release on the database, and other connections still open.
  • Applied patch.
  • Saw consistent success.
  • Used storage destroy --unittest-fixtures to clean up extra databases.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Fix a possible deadlock in unit tests after an error.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.

Here's a production example of exactly what this looks like:

mysql> show full processlist;
+-------+------+-----------+-------------------------------------------------------+---------+------+---------------------------------+---------------------------------------------------------------------------------+
| Id    | User | Host      | db                                                    | Command | Time | State                           | Info                                                                            |
+-------+------+-----------+-------------------------------------------------------+---------+------+---------------------------------+---------------------------------------------------------------------------------+
| 33384 | root | localhost | phabricator_unittest_zsjm7ota2gj4rxpmp3cg7cvo_metamta | Sleep   | 5303 |                                 | NULL                                                                            |
| 33385 | root | localhost | phabricator_unittest_zsjm7ota2gj4rxpmp3cg7cvo_user    | Sleep   | 5303 |                                 | NULL                                                                            |
| 33386 | root | localhost | phabricator_unittest_zsjm7ota2gj4rxpmp3cg7cvo_worker  | Sleep   | 5303 |                                 | NULL                                                                            |
| 33388 | root | localhost | NULL                                                  | Sleep   | 5303 |                                 | NULL                                                                            |
| 33389 | root | localhost | NULL                                                  | Query   | 5303 | Waiting for table metadata lock | DROP DATABASE IF EXISTS `phabricator_unittest_zsjm7ota2gj4rxpmp3cg7cvo_metamta` |
| 33527 | root | localhost | phabricator_unittest_7wzk2ezno57z5ufzibk5deqh_metamta | Sleep   | 3307 |                                 | NULL                                                                            |
| 33528 | root | localhost | phabricator_unittest_7wzk2ezno57z5ufzibk5deqh_user    | Sleep   | 3307 |                                 | NULL                                                                            |
| 33529 | root | localhost | phabricator_unittest_7wzk2ezno57z5ufzibk5deqh_worker  | Sleep   | 3307 |                                 | NULL                                                                            |
| 33531 | root | localhost | NULL                                                  | Sleep   | 3307 |                                 | NULL                                                                            |
| 33532 | root | localhost | NULL                                                  | Query   | 3307 | Waiting for table metadata lock | DROP DATABASE IF EXISTS `phabricator_unittest_7wzk2ezno57z5ufzibk5deqh_metamta` |
| 34221 | root | localhost | NULL                                                  | Query   |    0 | NULL                            | show full processlist                                                           |
+-------+------+-----------+-------------------------------------------------------+---------+------+---------------------------------+---------------------------------------------------------------------------------+
11 rows in set (0.00 sec)
chad edited edge metadata.

obviously.

This revision is now accepted and ready to land.Dec 24 2015, 4:38 PM
This revision was automatically updated to reflect the committed changes.