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

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@sss.pgh.pa.us> wrote in <7445.1464712190@sss.pgh.pa.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
diff --git a/src/bin/pg_dump/parallel.c b/src/bin/pg_dump/parallel.c
index cf97d9c..fb48a02 100644
--- a/src/bin/pg_dump/parallel.c
+++ b/src/bin/pg_dump/parallel.c
@@ -618,6 +618,12 @@ consoleHandler(DWORD dwCtrlType)            {                ArchiveHandle *AH =
signal_info.pstate->parallelSlot[i].args->AH;
+                /*
+                 * Using TerminateThread here may leave some resources leaked
+                 * but whole this process will soon die.
+                 */
+                TerminateThread(signal_info.pstate->parallelSlot[i].hThread, 0);
+                if (AH != NULL && AH->connCancel != NULL)                    (void) PQcancel(AH->connCancel, errbuf,
sizeof(errbuf));           }
 
@@ -632,6 +638,8 @@ consoleHandler(DWORD dwCtrlType)                            errbuf, sizeof(errbuf));
LeaveCriticalSection(&signal_info_lock);
+
+        write_msg(NULL, "terminated by user\n");    }    /* Always return FALSE to allow signal handling to continue
*/