wpackagist
wpackagist copied to clipboard
outlandishideas
Parallel on-demand updates – and sometimes scheduled updates – occasionally hit non-unique PackageData crash
In the last couple of months, 1 task run and 2 web requests have hit pretty much this same error.
We should further refine its handling to reduce log noise if possible. In particular, sending 2 identical update requests in rapid succession, which e.g. a browser is pretty likely to do on a shaky connection, should not lead to any ERROR level logs or 500 responses to the browser. We should instead re-query the database to get the new data if it seems like an update was just persisted with the same data we were going to save.
A recent web example log and screenshot of log in context:
[2022-04-12T12:00:56.701240+00:00] app.CRITICAL: Doctrine\DBAL\Exception\UniqueConstraintViolationException – An exception occurred while executing 'INSERT INTO package_data (type, name, hash, value, is_latest) VALUES (?, ?, ?, ?, ?)' with params ["package", "wpackagist-plugin\/fallback", "bf17f4a85a718b9455ea07fa7449301a2f4dc9cf17528cc212b82d04a1f6f01a", "{\"packages\":{\"wpackagist-plugin\\\/fallback\":{\"1.0.0\":{\"name\":\"wpackagist-plugin\\\/fallback\",\"version\":\"1.0.0\",\"version_normalized\":\"1.0.0.0\",\"uid\":\"58c3b5d1a4210d9f59eb0490d71f70477ffcd7f3f79997fa7c8dbdb104dfb581\",\"dist\":{\"type\":\"zip\",\"url\":\"https:\\\/\\\/downloads.wordpress.org\\\/plugin\\\/fallback.1.0.0.zip\"},\"source\":{\"type\":\"svn\",\"url\":\"https:\\\/\\\/plugins.svn.wordpress.org\\\/fallback\\\/\",\"reference\":\"tags\\\/1.0.0\"},\"homepage\":\"https:\\\/\\\/wordpress.org\\\/plugins\\\/fallback\\\/\",\"require\":{\"composer\\\/installers\":\"^1.0 || ^2.0\"},\"type\":\"wordpress-plugin\"},\"1.0.1\":{\"name\":\"wpackagist-plugin\\\/fallback\",\"version\":\"1.0.1\",\"version_normalized\":\"1.0.1.0\",\"uid\":\"9304299f0196b4e3eebb48202a8aed1b470103f6abf4bb5b0df600a217432dd4\",\"dist\":{\"type\":\"zip\",\"url\":\"https:\\\/\\\/downloads.wordpress.org\\\/plugin\\\/fallback.1.0.1.zip\"},\"source\":{\"type\":\"svn\",\"url\":\"https:\\\/\\\/plugins.svn.wordpress.org\\\/fallback\\\/\",\"reference\":\"tags\\\/1.0.1\"},\"homepage\":\"https:\\\/\\\/wordpress.org\\\/plugins\\\/fallback\\\/\",\"require\":{\"composer\\\/installers\":\"^1.0 || ^2.0\"},\"type\":\"wordpress-plugin\"},\"1.0.2\":{\"name\":\"wpackagist-plugin\\\/fallback\",\"version\":\"1.0.2\",\"version_normalized\":\"1.0.2.0\",\"uid\":\"f64bd5a4bb013dcf658b5b265003c69d1dea49bea7089d6154c116de276fafb3\",\"dist\":{\"type\":\"zip\",\"url\":\"https:\\\/\\\/downloads.wordpress.org\\\/plugin\\\/fallback.1.0.2.zip\"},\"source\":{\"type\":\"svn\",\"url\":\"https:\\\/\\\/plugins.svn.wordpress.org\\\/fallback\\\/\",\"reference\":\"tags\\\/1.0.2\"},\"homepage\":\"https:\\\/\\\/wordpress.org\\\/plugins\\\/fallback\\\/\",\"require\":{\"composer\\\/installers\":\"^1.0 || ^2.0\"},\"type\":\"wordpress-plugin\"},\"dev-trunk\":{\"name\":\"wpackagist-plugin\\\/fallback\",\"version\":\"dev-trunk\",\"version_normalized\":\"9999999-dev\",\"uid\":\"d8aa5aacc9481c4ec531b9d76f68a84df2ad65fc17403c91017bf6a33c0ba16f\",\"time\":\"2021-01-10 15:14:10\",\"dist\":{\"type\":\"zip\",\"url\":\"https:\\\/\\\/downloads.wordpress.org\\\/plugin\\\/fallback.zip?timestamp=1610291650\"},\"source\":{\"type\":\"svn\",\"url\":\"https:\\\/\\\/plugins.svn.wordpress.org\\\/fallback\\\/\",\"reference\":\"trunk\"},\"homepage\":\"https:\\\/\\\/wordpress.org\\\/plugins\\\/fallback\\\/\",\"require\":{\"composer\\\/installers\":\"^1.0 || ^2.0\"},\"type\":\"wordpress-plugin\"}}}}", 1]: SQLSTATE[23505]: Unique violation: 7 ERROR: duplicate key value violates unique constraint "package_data_pkey" DETAIL: Key (type, name, hash)=(package, wpackagist-plugin/fallback, bf17f4a85a718b9455ea07fa7449301a2f4dc9cf17528cc212b82d04a1f6f01a) already exists. [] []
Reopening this because, while I think the previous retry mechanism is harmless and possibly useful, it works on Packages and not PackageData. We continue to see occasional instances of the original problem impacting both package and provider group persists to the database when a single package update web request overlaps with scheduled bulk updates.
I think an additional retry mechanism in Outlandish\Wpackagist\Storage::saveEntity() may allow us to sidestep the problem.
As the exception occurs only on flush of everything, not on a single entity persist, we need to allow retries further 'up' the call stack. For now I am proposing that on-demand web update calls are allowed 1 retry, in https://github.com/outlandishideas/wpackagist/pull/469
I think this was left open in error, but #469 seems not to fix the problem successfully as we have new, similarly rare errors now because EntityManagerClosed is occasionally thrown during an on-demand update, presumably during flush(). I'll propose a fix on a new PR shortly.