Race conditions during parallel worker (unclean) exit - Mailing list pgsql-hackers

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