Re: patch for parallel pg_dump

On Tue, Feb 7, 2012 at 10:21 PM, Joachim Wieland <joe(at)mcknight(dot)de> wrote:
> On Tue, Feb 7, 2012 at 4:59 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> It turns out that (as you anticipated) there are some problems with my
>> previous proposal.
>
> I assume you're talking to Tom, as my powers of anticipation are
> actually quite limited... :-)

No, I was talking to you, actually. You earlier suggested that an
ArchiveHandle was only meant to contain a single PGconn, and it seems
you're right. We can refactor that assumption away, but not
trivially.

> In my patch I dealt with exactly the same problem for the error
> handler by creating a separate function that has a static variable (a
> pointer to the ParallelState). The value is set and retrieved through
> the same function, so yes, it's kinda global but then again it can
> only be accessed from this function, which is only called from the
> error handler.

How did you make this thread-safe?

>> I'm starting to think it might make sense to press on with this
>> refactoring just a bit further and eliminate the distinction between
>> Archive and ArchiveHandle.
>
> How about doing more refactoring after applying the patch, you'd then
> see what is really needed and then we'd also have an actual use case
> for more than one connection (You might have already guessed that this
> proposal is heavily influenced by my self-interest of avoiding too
> much work to make my patch match your refactoring)...

Well, I don't think your patch is going to be too heavily affected
either way, because most of your changes were not in pg_dump.c, and
the need for any changes at all in pg_dump.c should rapidly be going
away. At any rate, the problem with stopping here is that g_conn is
still floating around, and one way or the other we've got to get rid
of it if you want to have more than one ArchiveHandle floating around
at a time. So we at least need to press on far enough to get to that
point.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company