Re: Parallel pg_dump's error reporting doesn't work worth squat

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: tgl(at)sss(dot)pgh(dot)pa(dot)us
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Parallel pg_dump's error reporting doesn't work worth squat
Date: 2016-06-02 08:49:41
Message-ID: 20160602.174941.256342236.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Apart from the invalid snapshot problem, I looked the patch
previously mentioned mainly for Windows.

At Tue, 31 May 2016 12:29:50 -0400, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote in <7445(dot)1464712190(at)sss(dot)pgh(dot)pa(dot)us>
> In the patch I posted yesterday, I reversed the order of those two
> steps, which should fix this problem in most scenarios:
> https://www.postgresql.org/message-id/7005.1464657274@sss.pgh.pa.us

Entering ctrl-C while parallel pg_dump is running, on my console
I saw a repetition of the following error message, which is not
seen on Linux thanks to forcible termination of worker processes
in sigTermHandler().

> pg_dump: Dumping the contents of table "t" failed: PQgetResult() failed.
> pg_dump: Error message from server: ERROR: canceling statement due to user request
> pg_dump: The command was: COPY public.t (a) TO stdout;

We could provide a global flag (wantAbort?) that inform the
canceling of queries but it needs adding checks for it
everywhere.

Even though the threads started by beginthread cannot be
terminated cleanly from outside, but the whole process will soon
terminate anyway, so we could use TreminateThread. This seems
working. (Attached patch) We might be allowed to do exit() in
colsoleHandler().

Other questions follow.

Is there any reason for the name "ourAH" not to be "myAH"?

setup_cancel_handler looks somewhat bizarre. It eventually works
only for the main process/thread and does nothing for workers. It
is enough to be run once before forking in ParalleBackupStart and
that makes handler_set unnecessary.

In EndDBCopyMode, the result of PQgetResult is abandoned. This
can leak memory and such usage is not seen elsewhere in the
source tree. Shouldn't we hold the result and PQclear it? (Mainly
as a convention, not for benefit.)

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
dump-restore-sigint-1-silence-master-win32.patch text/x-patch 865 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2016-06-02 08:51:59 An extra error for client disconnection on Windows
Previous Message Michael Paquier 2016-06-02 07:00:33 Re: COMMENT ON, psql and access methods