Re: Parallel Apply

Hi,

On Thu, Oct 29, 2026 at 2:22 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Tue, 6 Oct 2026 at 12:07, Hayato Kuroda (Fujitsu)
> <kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
> >
> > Thanks for the contact. Here is an updated version.
>
> Few comments on 0002, 0003 and 0004 patch:

Thanks for the comments.

>
> 3. In pa_setup_dsm (the leader path), the handles are declared on the
> stack but not initialized:
...
>
> If pa_attach_parallelized_txn_hash were to return without assigning
> these (which it does in the worker path, see below), the leader would
> be checking
> uninitialized memory against DSA_HANDLE_INVALID. While the leader
> currently always hits a path that assigns them, initializing these to
> INVALID at the
> call site is safer.
>
> 4. We could set pa_dsa_handle and pa_dshash_handle outparams in
> pa_attach_parallelized_txn_hash of the am_parallel_apply_worker()
> branch to prevent check with uninitialized value in the caller:

Thinking a bit more about this, I think it would be cleaner to remove the
parameters entirely, since we rely on the global one anyway, the caller can
then refer to it directly instead of handling the input/output parameters. I've
changed it accordingly.

>
> 5. Busy Waiting in pa_wait_for_depended_transaction, the dependency
> wait loop uses a busy-wait strategy with a 1-millisecond sleep:
>
> + * The above race window is small and infrequent, so
> we avoid WaitLatch
> + * and just sleep briefly to prevent busy waiting.
> + *
> + * XXX: might be a performance bottleneck due to busy waiting.
> + */
> + pg_usleep(1000L);
>
> Have we tried to use a condition variable where the worker wakes up
> any waiters instead of hard coding with 1 ms sleep.

Since this wait is to avoid a busy loop in rare cases as mentioned in the
comments, we should avoid heavier waits like a conditional wait, which would
require each worker to signal on applying every commit. A simple sleep is
preferable, similar to the existing one in XactLockTableWait().

~

Here is the v28 patch set, which addresses all the comments so far, including
the above and:

* Manu's comments, including one performance bottleneck due to pg_sleep() [1]
* Vignesh's comments [2][3]
* Shlok's comments [4]

Note that, as mentioned before, we're currently working on another project
while also handling PG 19 open items and newly reported bugs in the stable
branches, so we're not updating the patch at full speed.
We'll switch back to this later once the other work is done.

Also, just to mention: the plan is to get the first 7 patches ready and then
work on the later additional-functionality patches.

[1] https://www.postgresql.org/message-id/179248187532.3609897.5928325649068374506%40gmail.com
[2] https://www.postgresql.org/message-id/CALDaNm1EM5Xasch8YkEau7OnZ8z71i6ABsKXnKaLk416N%2BZBAw%40mail.gmail.com
[3] https://www.postgresql.org/message-id/CALDaNm11d%3DkVUbjg8Et9zPYN88EFZi27awb77ki22i54K3n0-Q%40mail.gmail.com
[4] https://www.postgresql.org/message-id/CANhcyEXuDB3SSsQGADwCfLxn8J%3DD_Gf7T2wfgX4vBa-%3Dde5F8A%40mail.gmail.com

--
Best Regards,
Zhijie Hou