aboutsummaryrefslogtreecommitdiff
path: root/src/backend/access/transam/parallel.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2024-11-08 13:42:01 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2024-11-08 13:42:10 -0500
commitb8df690492568d7852b799b4eff3274fbbd91e78 (patch)
treec97cc6e4cc3b67da1bd4e45e9aeb1086e80d643f /src/backend/access/transam/parallel.c
parentb5ee4e52026b03b19ba47cde27bed9cf740576cc (diff)
downloadpostgresql-b8df690492568d7852b799b4eff3274fbbd91e78.tar.gz
postgresql-b8df690492568d7852b799b4eff3274fbbd91e78.zip
Improve fix for not entering parallel mode when holding interrupts.
Commit ac04aa84a put the shutoff for this into the planner, which is not ideal because it doesn't prevent us from re-using a previously made parallel plan. Revert the planner change and instead put the shutoff into InitializeParallelDSM, modeling it on the existing code there for recovering from failure to allocate a DSM segment. However, that code path is mostly untested, and testing a bit harder showed there's at least one bug: ExecHashJoinReInitializeDSM is not prepared for us to have skipped doing parallel DSM setup. I also thought the Assert in ReinitializeParallelWorkers is pretty ill-advised, and replaced it with a silent Min() operation. The existing test case added by ac04aa84a serves fine to test this version of the fix, so no change needed there. Patch by me, but thanks to Noah Misch for the core idea that we could shut off worker creation when !INTERRUPTS_CAN_BE_PROCESSED. Back-patch to v12, as ac04aa84a was. Discussion: https://postgr.es/m/CAC-SaSzHUKT=vZJ8MPxYdC_URPfax+yoA1hKTcF4ROz_Q6z0_Q@mail.gmail.com
Diffstat (limited to 'src/backend/access/transam/parallel.c')
-rw-r--r--src/backend/access/transam/parallel.c19
1 files changed, 16 insertions, 3 deletions
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 4a2e352d579..a10bf02ccff 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -231,6 +231,15 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_estimate_keys(&pcxt->estimator, 1);
/*
+ * If we manage to reach here while non-interruptible, it's unsafe to
+ * launch any workers: we would fail to process interrupts sent by them.
+ * We can deal with that edge case by pretending no workers were
+ * requested.
+ */
+ if (!INTERRUPTS_CAN_BE_PROCESSED())
+ pcxt->nworkers = 0;
+
+ /*
* Normally, the user will have requested at least one worker process, but
* if by chance they have not, we can skip a bunch of things here.
*/
@@ -476,6 +485,9 @@ InitializeParallelDSM(ParallelContext *pcxt)
shm_toc_insert(pcxt->toc, PARALLEL_KEY_ENTRYPOINT, entrypointstate);
}
+ /* Update nworkers_to_launch, in case we changed nworkers above. */
+ pcxt->nworkers_to_launch = pcxt->nworkers;
+
/* Restore previous memory context. */
MemoryContextSwitchTo(oldcontext);
}
@@ -539,10 +551,11 @@ ReinitializeParallelWorkers(ParallelContext *pcxt, int nworkers_to_launch)
{
/*
* The number of workers that need to be launched must be less than the
- * number of workers with which the parallel context is initialized.
+ * number of workers with which the parallel context is initialized. But
+ * the caller might not know that InitializeParallelDSM reduced nworkers,
+ * so just silently trim the request.
*/
- Assert(pcxt->nworkers >= nworkers_to_launch);
- pcxt->nworkers_to_launch = nworkers_to_launch;
+ pcxt->nworkers_to_launch = Min(pcxt->nworkers, nworkers_to_launch);
}
/*