Re: Parallel Apply

On Tue, 6 Oct 2026 at 12:07, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Hi Vignesh,
>
> > I was planning to review this patch set and noticed the patch does not
> > apply anymore, could you post a rebased version for reviewing?
>
> Thanks for the contact. Here is an updated version.

Few comments for 0001 independently:
1) There is a minor typo regarding the message byte alias in the
comment:

+ /*
+ * Ensure LOGICAL_REP_MSG_INTERNAL_MESSAGE does not conflict with
+ * PqReplMsg_WALData ('d'), as parallel apply workers may
receive both types
+ * of messages.
+ */
+ StaticAssertDecl(LOGICAL_REP_MSG_INTERNAL_MESSAGE != PqReplMsg_WALData,
+
"LOGICAL_REP_MSG_INTERNAL_MESSAGE conflicts with PqReplMsg_WALData");
+

PqReplMsg_WALData is defined as 'w', not 'd'. The byte 'd' identifies
CopyData (PqMsg_CopyData). The comment should say ('w') to prevent
confusion for future developers tracking protocol bytes.

2) The ??? in error message is not required as we have invalid type
included in the error message:
ereport(ERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("invalid worker internal message type
\"??? (%d)\"", action)));

3) NULL Pointer Dereference on Worker Launch Failure:
In pa_launch_parallel_worker(), if logicalrep_worker_launch() fails,
winfo is freed and set to NULL. However, the code later calls
pa_send_data(winfo, ...) without checking it. This can cause a
segmentation fault when pa_send_data() accesses the NULL winfo.
@@ -460,6 +462,26 @@ pa_launch_parallel_worker(void)

MemoryContextSwitchTo(oldcontext);

+ initStringInfo(&out);
+
+ /*
+ * Send all existing remote relation information to the parallel apply
+ * worker. This allows the parallel worker to initialize the
+ * LogicalRepRelMapEntry locally before applying remote changes. This is
+ * needed since the walsender does not send remote relation information
+ * with every transaction.
+ */
+ logicalrep_write_all_internal_rels(&out, &num_rels);
+
+ /*
+ * Timeout is unlikely here because the worker doesn't hold
any locks while
+ * processing relation information, so it's safe from deadlocks.
+ */
+ if (num_rels && !pa_send_data(winfo, out.len, out.data))
+ ereport(ERROR,
+
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("could not send remote
relation information to the logical replication parallel apply
worker")));

I noticed that this is handled later in the 0006 patch. I'm not sure
about the intended commit order, but if possible, could you keep this
fix in 0001? It would make the patch easier to review.

Regards,
Vignesh