pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedparallel restore sometimes fails for FKs to partitioned tables
2+ messages / 1 participants
[nested] [flat]
Commits from this thread:
2019-10-17 1752e35163 b304b2b65f Fix parallel restore of FKs to partitioned tables
* parallel restore sometimes fails for FKs to partitioned tables
@ 2019-10-05 22:43 Alvaro Herrera <alvherre@2ndquadrant.com>
2019-10-17 08:14 ` Re: parallel restore sometimes fails for FKs to partitioned tables Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 1 reply; 2+ messages in thread
From: Alvaro Herrera @ 2019-10-05 22:43 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Hello
While playing around I noticed that depending on the number of parallel
workers in pg_restore compared to the number of partitions a table has,
restoring an FK fails because the FK itself is restored before the index
partitions have completed restoring. The exact conditions to cause the
failure seem to vary depending on whether the dump is schema-only or not.
This can seemingly be fixed by having pg_dump make the constraint depend
on the attach of each partition, as in the attached patch. With this
patch I no longer see failures.
This patch is a bit weird because I added a new "simple list" type, to
store pointers. One alternative would be to store the dumpId values for
the partitions instead, but we don't have a dumpId-typed simple list
either. We could solve that by casting the dumpId to Oid, but that
seems almost as strange as the current proposal.
The other thing that makes this patch a little weird is that we have to
scan the list of indexes in the referenced partitioned table in order to
find the correct one. This should be okay, as the number of indexes in
any one table is not expected to grow very large. This isn't easy to
fix because we don't have a bsearchable array of indexes like we do of
other object types, and this already requires some contortions nearby.
Still, I'm not sure that this absolutely needs fixing now.
--
Álvaro Herrera Developer, https://www.PostgreSQL.org/
Attachments:
dump-fks-parallel.patch (text/x-diff, 6.0K) download
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 02a865f456..3549f7bc08 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -412,6 +412,9 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
addObjectDependency(&attachinfo[k].dobj,
parentidx->indextable->dobj.dumpId);
+ /* keep track of the list of partitions in the parent index */
+ simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj);
+
k++;
}
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a9c868b9af..06b07a5c96 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7109,6 +7109,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
indxinfo[j].indisclustered = (PQgetvalue(res, j, i_indisclustered)[0] == 't');
indxinfo[j].indisreplident = (PQgetvalue(res, j, i_indisreplident)[0] == 't');
indxinfo[j].parentidx = atooid(PQgetvalue(res, j, i_parentidx));
+ indxinfo[j].partattaches = (SimplePtrList) { NULL, NULL };
contype = *(PQgetvalue(res, j, i_contype));
if (contype == 'p' || contype == 'u' || contype == 'x')
@@ -7241,6 +7242,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
i_conoid,
i_conname,
i_confrelid,
+ i_conindid,
i_condef;
int ntups;
@@ -7266,7 +7268,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
resetPQExpBuffer(query);
if (fout->remoteVersion >= 110000)
appendPQExpBuffer(query,
- "SELECT tableoid, oid, conname, confrelid, "
+ "SELECT tableoid, oid, conname, confrelid, conindid, "
"pg_catalog.pg_get_constraintdef(oid) AS condef "
"FROM pg_catalog.pg_constraint "
"WHERE conrelid = '%u'::pg_catalog.oid "
@@ -7275,7 +7277,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
tbinfo->dobj.catId.oid);
else
appendPQExpBuffer(query,
- "SELECT tableoid, oid, conname, confrelid, "
+ "SELECT tableoid, oid, conname, confrelid, 0 as conindid, "
"pg_catalog.pg_get_constraintdef(oid) AS condef "
"FROM pg_catalog.pg_constraint "
"WHERE conrelid = '%u'::pg_catalog.oid "
@@ -7289,12 +7291,15 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
i_conoid = PQfnumber(res, "oid");
i_conname = PQfnumber(res, "conname");
i_confrelid = PQfnumber(res, "confrelid");
+ i_conindid = PQfnumber(res, "conindid");
i_condef = PQfnumber(res, "condef");
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
for (j = 0; j < ntups; j++)
{
+ TableInfo *reftable;
+
constrinfo[j].dobj.objType = DO_FK_CONSTRAINT;
constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid));
constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid));
@@ -7311,6 +7316,34 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
constrinfo[j].condeferred = false;
constrinfo[j].conislocal = true;
constrinfo[j].separate = true;
+
+ /*
+ * Restoring an FK that points to a partitioned table requires
+ * that all partition indexes have been attached beforehand.
+ * Ensure that happens by making the constraint depend on each
+ * index partition attach object.
+ */
+ reftable = findTableByOid(constrinfo[j].confrelid);
+
+ if (reftable->relkind == RELKIND_PARTITIONED_TABLE)
+ {
+ IndxInfo *refidx;
+ OId indexOid = atooid(PQgetvalue(res, j, i_conindid));
+
+ for (int k = 0; k < reftable->numIndexes; k++)
+ {
+ if (reftable->indexes[k].dobj.catId.oid == indexOid)
+ {
+ SimplePtrListCell *cell;
+
+ refidx = &reftable->indexes[k];
+ for (cell = refidx->partattaches.head; cell; cell = cell->next)
+ addObjectDependency(&constrinfo[j].dobj,
+ ((DumpableObject *) cell->ptr)->dumpId);
+ break;
+ }
+ }
+ }
}
PQclear(res);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index c3c2ea1473..ce04f914e1 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -371,6 +371,8 @@ typedef struct _indxInfo
bool indisclustered;
bool indisreplident;
Oid parentidx; /* if partitioned, parent index OID */
+ SimplePtrList partattaches; /* if partitioned, partition attach objects */
+
/* if there is an associated constraint object, its dumpId: */
DumpId indexconstraint;
} IndxInfo;
diff --git a/src/fe_utils/simple_list.c b/src/fe_utils/simple_list.c
index 8d605140a7..2232e8db73 100644
--- a/src/fe_utils/simple_list.c
+++ b/src/fe_utils/simple_list.c
@@ -114,3 +114,24 @@ simple_string_list_not_touched(SimpleStringList *list)
}
return NULL;
}
+
+/*
+ * Append a pointer to the list.
+ *
+ * Caller must ensure that the pointer remains valid.
+ */
+void
+simple_ptr_list_append(SimplePtrList *list, void *ptr)
+{
+ SimplePtrListCell *cell;
+
+ cell = (SimplePtrListCell *) pg_malloc(sizeof(SimplePtrListCell));
+ cell->next = NULL;
+ cell->ptr = ptr;
+
+ if (list->tail)
+ list->tail->next = cell;
+ else
+ list->head = cell;
+ list->tail = cell;
+}
diff --git a/src/include/fe_utils/simple_list.h b/src/include/fe_utils/simple_list.h
index 8a95cbb3a8..1443b68888 100644
--- a/src/include/fe_utils/simple_list.h
+++ b/src/include/fe_utils/simple_list.h
@@ -43,6 +43,17 @@ typedef struct SimpleStringList
SimpleStringListCell *tail;
} SimpleStringList;
+typedef struct SimplePtrListCell
+{
+ struct SimplePtrListCell *next;
+ void *ptr;
+} SimplePtrListCell;
+
+typedef struct SimplePtrList
+{
+ SimplePtrListCell *head;
+ SimplePtrListCell *tail;
+} SimplePtrList;
extern void simple_oid_list_append(SimpleOidList *list, Oid val);
extern bool simple_oid_list_member(SimpleOidList *list, Oid val);
@@ -52,4 +63,6 @@ extern bool simple_string_list_member(SimpleStringList *list, const char *val);
extern const char *simple_string_list_not_touched(SimpleStringList *list);
+extern void simple_ptr_list_append(SimplePtrList *list, void *val);
+
#endif /* SIMPLE_LIST_H */
* Re: parallel restore sometimes fails for FKs to partitioned tables
2019-10-05 22:43 parallel restore sometimes fails for FKs to partitioned tables Alvaro Herrera <alvherre@2ndquadrant.com>
@ 2019-10-17 08:14 ` Alvaro Herrera <alvherre@2ndquadrant.com>
0 siblings, 0 replies; 2+ messages in thread
From: Alvaro Herrera @ 2019-10-17 08:14 UTC (permalink / raw)
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
On 2019-Oct-05, Alvaro Herrera wrote:
> While playing around I noticed that depending on the number of parallel
> workers in pg_restore compared to the number of partitions a table has,
> restoring an FK fails because the FK itself is restored before the index
> partitions have completed restoring. The exact conditions to cause the
> failure seem to vary depending on whether the dump is schema-only or not.
>
> This can seemingly be fixed by having pg_dump make the constraint depend
> on the attach of each partition, as in the attached patch. With this
> patch I no longer see failures.
Pushed with some additional tweaks.
Parallel restore of partitioned tables containing data and unique keys
still fails with some deadlock errors, though :-(
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2019-10-17 08:14 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 22:43 parallel restore sometimes fails for FKs to partitioned tables Alvaro Herrera <alvherre@2ndquadrant.com>
2019-10-17 08:14 ` Alvaro Herrera <alvherre@2ndquadrant.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox