Re: Incorrect CONTEXT reported for errors from parallel apply worker in logical replication

On Mon, 26 Oct 2026 at 13:56, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Hi,
>
> One of my colleagues requested me to update the commit message. I did for 0002 and
> 0004. I did not do for 0001 and 0003 because it's not intended to be included.
>

Thanks for the updated patch.
Few minor suggestions:
1) apply_error_context_stack used to mean "the stack including
apply_error_callback", and with this patch it deliberately means the
opposite. The name now says the reverse of what the variable holds.
How about renaming it to apply_loop_outer_error_context_stack or a
better one if you have, and adding a short comment at the definition
saying that it is the stack before LogicalRepApplyLoop() pushed
apply_error_callback, and that parallel apply worker errors are
reported with it?

2) The patch reverses the choice made in 216a784829c, which states:
"Context beyond that should use the error context callbacks that were
in effect in LogicalRepApplyLoop()". The reasoning for this change is
sound. Could the commit message explicitly mention that it changes the
earlier choice and explain why?

How about something like(feel free to change it):
An error reported by a parallel apply worker is relayed by the leader
apply worker. Before rethrowing it, ProcessParallelApplyMessage()
replaces error_context_stack with a stack saved in
LogicalRepApplyLoop(), so that the context callbacks of whatever the
leader is doing at that moment (for example, a callback pushed while
applying another transaction) are not run for this error.

The saved stack was taken after apply_error_callback was pushed, so
that callback was still run. It formats the leader's own apply state
(the relation, the remote transaction and the message being applied),
which has nothing to do with the transaction that failed in the
worker. If the leader was applying another transaction at that
moment, the error got a misleading CONTEXT line naming the relation
the leader was working on, instead of just the worker's own context.

This changes what commit 216a784829c chose, namely to keep the
callbacks in effect in LogicalRepApplyLoop(), apply_error_callback
included. That is not useful here, because the worker's own context,
relayed in the error message, already says where the error happened.
Save the stack before pushing apply_error_callback.

Regards,
Vignesh