Re: patch for parallel pg_dump - Mailing list pgsql-hackers

On Tue, Apr 3, 2012 at 12:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Apr 3, 2012 at 12:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> No, the reason for write_stderr() is that fprintf(stderr) is unreliable
>>> on Windows.  If memory serves, it can actually crash in some situations.
>
>> Dude, we're already doing fprintf(stderr) all over pg_dump.  If it's
>> unreliable even in front-end code, we're screwed anyway.  That is a
>> non-objection.
>
> No, it isn't.  The fact that it works in pg_dump doesn't extrapolate
> to other places.  (In particular, it will absolutely not work in libpq,
> at least not in all the environments where libpq is supposed to work.)

Well, I didn't propose putting the assert machinery into libpq, but
FWIW fprintf(stderr, ...) it's already used there - see
defaultNoticeProcessor() for example.  It's also used in libpgport,
which is where we're proposing to put this code (see pgsymlink, for
example).  Furthermore, the code would only run in the event that
assertions are enabled (which only developers normally do) and it
would only break if run as a service (which would be unusual when
debugging) and only in the case where the process was in the middle of
crashing anyway (in which case the worst case is that it crashes just
before printing the error message rather than just after).

However, if despite all that you're still worried about it, it appears
special handling is only needed to cover the case where code might be
running as a service, and apparently none of our utilities worry about
that right now except for pg_ctl, which handles it by defining a
frontend-safe version of write_stderr().  So perhaps we ought to merge
the frontend version in pg_ctl with the backend version and put the
result in libpgport (possibly renamed) and then ExceptionalCondition()
can go into pgport and simply call write_stderr() and it will get the
appropriate version depending on whether it's running in frontend or
backend code.  That's a tad more work than I was expecting to do here,
but I'm still not convinced that it's more than can be done in an
afternoon.

> I think what we've got at the moment is something that's adequate for
> pg_dump, and that's all that it is.  Concluding that it can be used in
> all frontend code is way premature, and therefore I'm -1 on the idea
> of exposing it in non-pg_dump header files.

It seems desirable to share as much of the implementation with the
backend as possible, and moving the macros from postgres.h into their
own header file (which postgres.h would include) would allow that,
leaving it as the application's problem to provide
ExceptionalCondition(), which pg_dump can surely do.  If we don't do
at least that much, then we're going to end up either (1) removing the
assertions, which doesn't seem prudent, or (2) doing a cut-and-paste
from postgres.h into a pg_dump header file, which seems to have no
virtues whatsoever, or (3) having an implementation in pg_dump that is
completely separate from and different from what we have in the
backend, which seems even worse than the preceding option.  The
minimum we need here to avoid creating a mess here is no more than
moving 25 lines of code from postgres.h to a separate header file that
pg_dump can include.  I don't see why that's a big deal.  I also don't
see why we can't fix the problem more completely, perhaps along the
lines suggested above.

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