On 2020-Jan-29, Peter Eisentraut wrote:
> On 2020-01-21 10:36, Luis Carril wrote:
> > > Yes we can support --include-foreign-data without parallel option and
> > > later add support for parallel option as a different patch.
> >
> > I've attached a new version of the patch in which an error is
> > emitted if the parallel backup is used with the --include-foreign-data
> > option.
>
> This seems like an overreaction. The whole point of lockTableForWorker() is
> to avoid deadlocks, but foreign tables don't have locks, so it's not a
> problem. I think you can just skip foreign tables in lockTableForWorker()
> using the same logic that getTables() uses.
>
> I think parallel data dump would be an especially interesting option when
> using foreign tables, so it's worth figuring this out.
I agree it would be nice to implement this, so I tried to implement it.
I found it's not currently workable, because parallel.c only has a tocEntry
to work with, not a DumpableObject, so it doesn't know that the table is
foreign; to find that out, parallel.c could use findObjectByDumpId, but
parallel.c is used by both pg_dump and pg_restore, and findObjectByDumpId is
in common.c which cannot be linked in pg_restore because of numerous
incompatibilities.
One way to make this work would be to put lockTableForWorker somewhere other
than parallel.c. Foe example maybe have CreateArchive() set up a new "lock
table" ArchiveHandle function ptr that parallel.c can call;
lockTableForWorker() becomes the pg_dump implementation of that, while
pg_restore uses NULL.
Anyway, I think Luis has it right that this should not be a blocker for
this feature.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services