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
