| From: | David Geier <geidav(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Tomas Vondra <tomas(at)vondra(dot)me>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, dilipbalaut(at)gmail(dot)com |
| Subject: | Re: Performance issues with parallelism and LIMIT |
| Date: | 2025-11-14 18:20:12 |
| Message-ID: | 1c1ea519-5a8a-41af-8bd2-20ec95a6a953@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Tomas!
On 13.11.2025 23:36, Tomas Vondra wrote:
> Sorry for not responding to this thread earlier.
No worries. Thanks for looking at it!
>>>> IMHO if this requires adding another macro to a bunch of ad hoc places
>>>> is rather inconvenient. It'd be much better to fix this in a localized
>>>> manner (especially as it seems related to a fairly specific place).
>>
>> I've written up a draft patch that instructs workers to stop, once the
>> leader has gotten enough rows according to the LIMIT clause. I'm using
>> SendProcSignal() to inform the workers to take action and stop executing
>> ExecutePlan(). I've implemented the stopping via sigsetjmp. I cannot see
>> a good way of doing this differently which is not much more intrusive.
>> The patch is incomplete (comments, support for Gather Merge, more
>> testing, etc.) but I'm mostly interested at this point if the overall
>> approach is deemed fine.
>>
>> I first tried to use TerminateBackgroundWorker() but postmaster.c then
>> logs the worker termination and also some of the cleanup code needed for
>> proper instrumentation doesn't run any longer in the parallel workers.
>>
>> With the patch applied, the query from the first mail of this thread
>> runs in a few milliseconds. That it still takes that long is because
>> forking, plan (de-)serialization and remaining initialization are fairly
>> heavy weight. With threads, the "fork" time would already much lower and
>> no (de-)serialization would be necessary. In the process-based
>> architecture it would be interesting to think about adding a parallel
>> worker pool.
>>
>
> Unfortunately, I don't think this patch is the way to go. When I apply
> it, I get:
>
> ERROR: InstrEndLoop called on running node
> CONTEXT: parallel worker
>
Ooops. That can likely be fixed.
> And I very much doubt inventing a new ad hoc way to signal workers is
> the right solution (even if there wasn't the InstrEndLoop issue).
>
> What I think we should do is much simpler - make the threshold in shm_mq
> dynamic, start with a very low value and gradually ramp up (up to 1/4).
> So we'd have
>
> if (mqh->mqh_consume_pending > threshold)
>
> We might start with
>
> threshold = (mq->mq_ring_size / 1024)
>
> or maybe some fixed value, list
>
> thredhold = 128
>
> And on every signal we'd double it, capping it to 1/4 of mq_ring_size.
>
> threshold = Min(threshold * 2, mq->mq_ring_size / 1024);
>
> This is very similar to other places doing this gradual ramp up, like in
> the prefetching / read_stream, etc. It allows fast termination for low
> LIMIT values, but quickly amortizes the cost for high LIMIT values.
That's a different problem though, isn't it? The original thread
contained two problems: (1) signaling the queue to late and (2) workers
stopping to late in the presence of LIMIT if they're not finding any
rows in their portion of the data.
Changing the queue thresholds is a solution for (1) but not for (2). For
(2) we need a mechanism to instruct the parallel workers to stop when we
find that other parallel workers have already produced enough rows to
answer the query.
An alternative mechanism that might work is using some stop_worker
boolean in shared memory that we check in CHECK_FOR_INTERRUPTS().
stop_worker is set to true by the leader as soon as it has collected
enough tuples. But then CHECK_FOR_INTERRUPTS() would have to have access
to the parallel context, which might also be a bit ugly.
--
David Geier
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2025-11-14 19:01:08 | Re: Uncommented GUC in postgresql.conf.sample |
| Previous Message | Peter Geoghegan | 2025-11-14 18:19:08 | Re: index prefetching |