Closed (fixed)
Project:
Drupal core
Version:
8.2.x-dev
Component:
postgresql db driver
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
18 Jun 2016 at 19:06 UTC
Updated:
4 Jul 2016 at 08:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
chx commentedComment #3
chx commentedComment #4
chx commentedComment #5
alexpottSo the fail are consistent with a patch for https://www.drupal.org/project/dynamic_entity_reference? Is there a way to add a test to core for this?
Comment #6
chx commentedNope. This is a race condition. It's literally impossible to test this. You'd need to fork and bomb PostgreSQL with CREATE FUNCTION calls and somehow ensure they fail without lock. My working theory is that DER testing is broken on PostgreSQL ATM because it yesterday it converted all of its WebTestBase to BrowserTestBase and apparently running two BrowserTestBase in parallel triggers this pretty reliably unlike WebTestBase did. That, actually, is the testing you are asking for. Core does not yet fail because there are so few BrowserTestBase classes. If my theory is correct then eventually it will begin to fail too.
Comment #7
alexpottI'm happy to try this in 8.2.x just as long as we add a code comment - ideally will a link to some postgres docs on the known race condition.
I can accept that the effort to try to reproduce a race condition is not worth it. Are there any performance considerations we should be making with this change?
Comment #8
chx commented> Are there any performance considerations we should be making with this change?
This is install time. Normal users don't install in parallel and PosgreSQL advisory locks were designed for speed. The only thing we can possibly hurt performance wise is the testbot and it's kicking a dead horse already: PostgreSQL testing is slow. Currently we are guaranteed to not hit this because if we were then it would die! In the future (if my theory is correct) we will hit this when BrowserTestBase is plenty but then we are talking of waiting for the grand total of four queries. AFAIK the race condition is not documented. Added a comment.
Comment #9
alexpottThis makes perfect sense to me - we're creating functions in the global database namespace and we're doing this is parallel. Given that this occurs in the installer there is not much to worry about here.
Comment #10
chx commentedFollowup at #2751581: PostgreSQL should use the simpletest prefix as schema but that's a bigger task.
Comment #11
alexpottComment #12
chx commentedComment #13
alexpottCommitted 3130842 and pushed to 8.1.x and 8.2.x. Thanks!
Comment #16
jibranThank you @chx. DER is green on on d.o ci as well. https://www.drupal.org/node/2250343/qa