On Thu, Nov 5, 2026 at 2:38 PM vignesh C <[email protected]> wrote:
>
> On Wed, 4 Nov 2026 at 19:27, Hayato Kuroda (Fujitsu)
> <[email protected]> wrote:
> >
> > Hi Amit,
> >
> > Thanks for giving comment!
> >
> > > One thing we can try to see what
> > > if ProcessParallelApplyMessage() instead of throwing an explicit ERROR
> > > just rethrows what it gets from the parallel apply worker by using
> > > ThrowErrorData().
> >
> > To confirm, apply_loop_outer_error_context_stack is anyway needed even if
> > we use
> > ThrowErrorData(), right? Otherwise the output can be like [1], context from
> > leader
> > worker can exist without any marks.
> >
> > IIUC ThrowErrorData() has benefits that 1) SQLSTATE can be preserved and 2)
> > we
> > can make the shape of codes similar. Parallel workers and REPACK workers
> > has the same.
> > But I do not have preference here. 0002 implements this.
> >
> > > The other idea to make this information meaningful
> > > is to display it in something like:
> > > CONTEXT: PL/pgSQL function public.tab_a_boom_fn() line 3 at RAISE
> > > processing remote data ... relation "public.tab_a" ...
> > > logical replication parallel apply worker
> > > leader apply worker was processing:
> > > processing remote data ... relation "public.tab_b" ...
> > >
> > > This will make it clear what the leader worker was processing during
> > > the ERROR. I think for this we may need to register a leader specific
> > > error-callback.
> >
> > 0003 tried the idea. Per my understanding we can introduce an ephemeral
> > error
> > context to append the leader's work.
> >
> > How do you feel?
>
> Thanks for the patches.
>
> With 0002 and 0003, the leader's entry still contains the error of the
> parallel apply worker, or its context, followed by the context of the
> leader's own transaction. The error of a different process then sits
> next to the leader's context, and the context of the failed
> transaction is logged a second time. This can be misleading.
>
> Kuroda-san and me had an offline discussion and came up with the
> following approach: The parallel apply worker will reports its error
> with the context of the failed transaction, and the leader only says
> that the worker failed, with its own context like:
> logical replication parallel worker[17710] ERROR: tab_a trigger boom
> logical replication parallel worker[17710] CONTEXT: PL/pgSQL function
> public.tab_a_boom_fn() line 3 at RAISE
> processing remote data for replication origin "pg_16411"
> during message type "INSERT" for replication target relation
> "public.tab_a" in transaction 667
> logical replication apply worker[17698] ERROR: logical replication
> parallel apply worker exited due to error, exiting leader as well
> logical replication apply worker[17698] CONTEXT: processing remote
> data for replication origin "pg_16411" during message type "INSERT"
> for
> replication target relation "public.tab_b" in transaction 668,
> finished at 0/0177DBA0
> postmaster[17642] LOG: background worker "logical replication
> parallel worker" (PID 17710) exited with exit code 1
> postmaster[17642] LOG: background worker "logical replication apply
> worker" (PID 17698) exited with exit code 1
>
> That way the transaction is not logged a second time, and the error of
> another process is no longer attached next to the leader's context.
> Thanks Kuroda-san for the offline discussion.
>
> The attached v8 version patch has the changes to drop the copying of
> the worker's context, keeps the pinned context stack, and changes the
> message.
> Thoughts?
Some observation while started reading the patch, I haven't done the
complete review yet, will do in sometime.
1.
/*
* Context beyond that should use the error context callbacks
* that were in effect in LogicalRepApplyLoop().
@@ -1052,13 +1035,12 @@ ProcessParallelApplyMessage(StringInfo msg)
error_context_stack = apply_error_context_stack;
IMHO in this comment "Context beyond that" refers to "edata.context",
which was just deleted above this comment. So, it seems we need to
adjust this comment?
2.
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
- errmsg("logical replication parallel apply worker exited due to error"),
- errcontext("%s", edata.context)));
+ errmsg("logical replication parallel apply worker exited due to
error, exiting leader as well")));
Do we really need ", exiting leader as well"" part of the error
message? I mean if we just say "logical replication parallel apply
worker exited due to error" isn't it obvious that leader will be
exited as well with elevel ERROR. Like other places[1] we just say
"lost connection to the logical replication parallel apply worker" and
that means leader will also be exited, I do not like the pattern of
saying "exiting leader as well"
[1]
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("lost connection to the logical replication parallel apply worker")));
--
Regards,
Dilip Kumar
Google
