In [1] I noted that ERROR like "lost connection to ... worker" (currently
issued in ProcessParallelMessages()) might also be appropriate for REPACK
(CONCURRENTLY). While I'm still not sure about that, I suspect that something
is wrong about handling unclean exits in parallel.c.
According to the header comment, ParallelWorkerShutdown() should handle cases
like direct call of proc_exit(). By adding one like this
diff --git a/src/backend/access/transam/parallel.c
b/src/backend/access/transam/parallel.c
index 17fcd246b0c..e8700ce8594 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1385,6 +1385,7 @@ ParallelWorkerMain(Datum main_arg)
* Hooray! Primary initialization is complete. Now, we need to set up
our
* backend-local state to match the original backend.
*/
+ proc_exit(1);
/*
* Join locking group. We must do this before anything that could try
to
and by running the following
begin;
create table a(i int);
set parallel_setup_cost to 0;
set parallel_tuple_cost to 0;
set min_parallel_table_scan_size to 0;
set min_parallel_index_scan_size to 0;
set max_parallel_workers_per_gather to 1;
set parallel_leader_participation to off;
set debug_parallel_query to 'regress';
table a;
rollback;
I can reproduce the expected behavior:
ERROR: lost connection to parallel worker
However, an additional sleep() call in ParallelWorkerShutdown()
diff --git a/src/backend/access/transam/parallel.c
b/src/backend/access/transam/parallel.c
index 17fcd246b0c..1a05f16e0bc 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1618,7 +1618,7 @@ ParallelWorkerShutdown(int code, Datum arg)
SendProcSignal(ParallelLeaderPid,
PROCSIG_PARALLEL_MESSAGE,
ParallelLeaderProcNumber);
-
+ sleep(1);
dsm_detach((dsm_segment *) DatumGetPointer(arg));
}
appears to cause race conditions. What I see is that the leader process runs
ProcessParallelMessages() while the worker is still attached to the error
queue, so the leader gets SHM_MQ_WOULD_BLOCK when trying to read from the
queue. Thus the ERROR is not raised.
Moreover, the leader, after having seen the worker detached from the tuple
queue (TupleQueueReaderNext) a bit later, starts executor cleanup and, as the
worker does not send signals anymore, it gets stuck in
WaitForParallelWorkersToExit().
Attached is what might be a fix, but I'm not sure if the Terminate message is
appropriate even if the worker in fact didn't finish with success.
[1] https://www.postgresql.org/message-id/32123.1788806780%40localhost
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 17fcd246b0c..b4802623b80 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -1597,10 +1598,9 @@ ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end)
}
/*
- * Make sure the leader tries to read from our error queue one more time.
- * This guards against the case where we exit uncleanly without sending an
- * ErrorResponse to the leader, for example because some code calls proc_exit
- * directly.
+ * Make sure the leader knows we're done. This guards against the case where
+ * we exit uncleanly without sending an ErrorResponse to the leader, for
+ * example because some code calls proc_exit directly.
*
* Also explicitly detach from dsm segment so that subsystems using
* on_dsm_detach() have a chance to send stats before the stats subsystem is
@@ -1615,6 +1615,8 @@ ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end)
static void
ParallelWorkerShutdown(int code, Datum arg)
{
+ pq_putmessage(PqMsg_Terminate, NULL, 0);
+
SendProcSignal(ParallelLeaderPid,
PROCSIG_PARALLEL_MESSAGE,
ParallelLeaderProcNumber);
