Re: [HACKERS] identify_locking_dependencies is broken for schema-only dumps
On Fri, Sep 19, 2014 at 11:23 AM, Robert Haas robertmh...@gmail.com wrote: This can lead to deadlocks during parallel restore. Test case: create table bar (a int primary key, b int); create table baz (z int, a int references bar); create view foo as select a, b, sum(1) from bar group by a union all select z, a, 0 from baz; I dumped this with: pg_dump -Fc -s -f test.dmp Then: while (dropdb rhaas; createdb; pg_restore -O -d rhaas -j3 test.dmp); do true; done This quickly fails for me with: pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 2155; 2606 47822 FK CONSTRAINT baz_a_fkey rhaas pg_restore: [archiver (db)] could not execute query: ERROR: deadlock detected DETAIL: Process 81791 waits for AccessExclusiveLock on relation 47862 of database 47861; blocked by process 81789. Process 81789 waits for AccessShareLock on relation 47865 of database 47861; blocked by process 81791. HINT: See server log for query details. Command was: ALTER TABLE ONLY baz ADD CONSTRAINT baz_a_fkey FOREIGN KEY (a) REFERENCES bar(a); WARNING: errors ignored on restore: 2 The attached patch seems to fix it for me. Comments? If there are no comments on this soon-ish, I'm going to push and back-patched the patch I attached. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] identify_locking_dependencies is broken for schema-only dumps
Robert Haas robertmh...@gmail.com writes: If there are no comments on this soon-ish, I'm going to push and back-patched the patch I attached. Sorry for not paying attention sooner. After studying it for awhile, I think the change is probably all right but your proposed comment is entirely inadequate. There are extremely specific reasons why this works, and you removed an existing comment about that and replaced it with nothing but a wishy-washy maybe. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] identify_locking_dependencies is broken for schema-only dumps
On Wed, Sep 24, 2014 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: If there are no comments on this soon-ish, I'm going to push and back-patched the patch I attached. Sorry for not paying attention sooner. After studying it for awhile, I think the change is probably all right but your proposed comment is entirely inadequate. There are extremely specific reasons why this works, and you removed an existing comment about that and replaced it with nothing but a wishy-washy maybe. Well, I could write something like this: * We assume the item requires exclusive lock on each TABLE or TABLE DATA * item listed among its dependencies. (This was originally a dependency on * the TABLE, but fix_dependencies may have repointed it to the data item. * In a schema-only dump, however, this will not have been done.) If you don't like that version, can you suggest something you would like better? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] identify_locking_dependencies is broken for schema-only dumps
Robert Haas robertmh...@gmail.com writes: On Wed, Sep 24, 2014 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Sorry for not paying attention sooner. After studying it for awhile, I think the change is probably all right but your proposed comment is entirely inadequate. If you don't like that version, can you suggest something you would like better? Perhaps like this: * We assume the entry requires exclusive lock on each TABLE or TABLE DATA * item listed among its dependencies. Originally all of these would have * been TABLE items, but repoint_table_dependencies would have repointed * them to the TABLE DATA items if those are present (which they might not * be, eg in a schema-only dump). Note that all of the entries we are * processing here are POST_DATA; otherwise there might be a significant * difference between a dependency on a table and a dependency on its * data, so that closer analysis would be needed here. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] identify_locking_dependencies is broken for schema-only dumps
On Wed, Sep 24, 2014 at 4:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Wed, Sep 24, 2014 at 12:18 PM, Tom Lane t...@sss.pgh.pa.us wrote: Sorry for not paying attention sooner. After studying it for awhile, I think the change is probably all right but your proposed comment is entirely inadequate. If you don't like that version, can you suggest something you would like better? Perhaps like this: * We assume the entry requires exclusive lock on each TABLE or TABLE DATA * item listed among its dependencies. Originally all of these would have * been TABLE items, but repoint_table_dependencies would have repointed * them to the TABLE DATA items if those are present (which they might not * be, eg in a schema-only dump). Note that all of the entries we are * processing here are POST_DATA; otherwise there might be a significant * difference between a dependency on a table and a dependency on its * data, so that closer analysis would be needed here. Works for me. I'll push with that text unless you'd like to take care of it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] identify_locking_dependencies is broken for schema-only dumps
This can lead to deadlocks during parallel restore. Test case: create table bar (a int primary key, b int); create table baz (z int, a int references bar); create view foo as select a, b, sum(1) from bar group by a union all select z, a, 0 from baz; I dumped this with: pg_dump -Fc -s -f test.dmp Then: while (dropdb rhaas; createdb; pg_restore -O -d rhaas -j3 test.dmp); do true; done This quickly fails for me with: pg_restore: [archiver (db)] Error while PROCESSING TOC: pg_restore: [archiver (db)] Error from TOC entry 2155; 2606 47822 FK CONSTRAINT baz_a_fkey rhaas pg_restore: [archiver (db)] could not execute query: ERROR: deadlock detected DETAIL: Process 81791 waits for AccessExclusiveLock on relation 47862 of database 47861; blocked by process 81789. Process 81789 waits for AccessShareLock on relation 47865 of database 47861; blocked by process 81791. HINT: See server log for query details. Command was: ALTER TABLE ONLY baz ADD CONSTRAINT baz_a_fkey FOREIGN KEY (a) REFERENCES bar(a); WARNING: errors ignored on restore: 2 The attached patch seems to fix it for me. Comments? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index ded9135..0393153 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -4140,11 +4140,10 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te) return; /* - * We assume the item requires exclusive lock on each TABLE DATA item - * listed among its dependencies. (This was originally a dependency on - * the TABLE, but fix_dependencies repointed it to the data item. Note - * that all the entry types we are interested in here are POST_DATA, so - * they will all have been changed this way.) + * We assume the item requires exclusive lock on each TABLE or TABLE DATA + * item listed among its dependencies. (fix_dependencies may have + * repointed TABLE dependencies at TABLE DATA items, but not in a + * schema-only dump.) */ lockids = (DumpId *) pg_malloc(te-nDeps * sizeof(DumpId)); nlockids = 0; @@ -4153,7 +4152,8 @@ identify_locking_dependencies(ArchiveHandle *AH, TocEntry *te) DumpId depid = te-dependencies[i]; if (depid = AH-maxDumpId AH-tocsByDumpId[depid] != NULL - strcmp(AH-tocsByDumpId[depid]-desc, TABLE DATA) == 0) + ((strcmp(AH-tocsByDumpId[depid]-desc, TABLE DATA) == 0) || + strcmp(AH-tocsByDumpId[depid]-desc, TABLE) == 0)) lockids[nlockids++] = depid; } -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
