| From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Andrei Lepikhov <lepihov(at)gmail(dot)com>, wenhui qiu <qiuwenhuifx(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | RE: Parallel Apply |
| Date: | 2026-04-24 11:26:05 |
| Message-ID: | TYRPR01MB14195EA120C2799CCC973835D942B2@TYRPR01MB14195.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thursday, April 23, 2026 5:25 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> I have started review the design and patches, couple of questions/suggestion
Thanks for the comments.
>
> 0001:
> 1. Looking at the commit message and patch, the motivation for
> WORKER_INTERNAL_MSG_RELATION isn't very clear to me. It's clear what it
> does, but the motivation isn't very clear to me.
The relation synchronization is necessary for parallel apply workers to map local
replication target relations to their remote counterparts during change
application. Since the walsender does not send remote relation information with
every transaction, the parallel apply worker may not have up-to-date relation
info unless synchronized by the leader.
I will add this to commit message and comments in next version.
>
> 2. +/*
> + * Wait for the given transaction to finish.
> + */
> +void
> +pa_wait_for_depended_transaction(TransactionId xid) { elog(DEBUG1,
> +"wait for depended xid %u", xid);
> +
> + for (;;)
> + {
> + /* XXX wait until given transaction is finished */ }
> +
> + elog(DEBUG1, "finish waiting for depended xid %u", xid); }
>
> Does that mean the waiting logic isn't implemented yet?
Yes, it's done in 0002.
>
> 3.
> + if (c == PqReplMsg_WALData)
> + {
> + /*
> + * Ignore statistics fields that have been updated by the
> + * leader apply worker.
> + *
> + * XXX We can avoid sending the statistics fields from the
> + * leader apply worker but for that, it needs to rebuild the
> + * entire message by removing these fields which could be more
> + * work than simply ignoring these fields in the parallel apply
> + * worker.
> + */
> + s.cursor += SIZE_STATS_MESSAGE;
>
> - apply_dispatch(&s);
> + apply_dispatch(&s);
> + }
>
> I could not understand how this change is relevant to patch 0001. This patch
> implements two internal messages; why ignoring statistics fields for non
> internal messages is relevant here?
I think the codes you referred are existing ones, the 0001 only add one if else
branch to handle the new internal message introduced in the patch.
Best Regards,
Hou zj
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-04-24 11:42:28 | Wrong results from inner-unique joins caused by collation mismatch |
| Previous Message | Fujii Masao | 2026-04-24 11:23:50 | Re: pgbench: make verbose error messages thread-safe |