| From: | vignesh C <vignesh21(at)gmail(dot)com> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Incorrect CONTEXT reported for errors from parallel apply worker in logical replication |
| Date: | 2026-10-08 09:33:26 |
| Message-ID: | CALDaNm3BLmfboFtVeejPs1GRQ8V_O1HGEaNWpU=tLp07uDap0A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Thu, 8 Oct 2026 at 13:23, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> I checked other examples in core, and parallel query seems to handle correctly.
> It has an attribute ParallelContext::error_context_stack which preserves an error
> context at that time, see CreateParallelContext(). When the parallel worker raises
> an ERROR, the leader backend can consume its message and raise it again, and at
> that time the error context is restored from the ParallelContext.
>
> I think the easiest fix is to use apply_error_context_stack for preserving the
> error context before entering the loop, attached patch implements the idea.
Thanks for the patch, the issue is resolved with your patch. One suggestion:
Can we add a comment saying the saved stack deliberately excludes
apply_error_callback. That would stop someone from "fixing" it back
later:
diff --git a/src/backend/replication/logical/worker.c
b/src/backend/replication/logical/worker.c
index 37b37500213..3d331c356a6 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4088,7 +4088,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
errcallback.callback = apply_error_callback;
errcallback.previous = error_context_stack;
error_context_stack = &errcallback;
- apply_error_context_stack = error_context_stack;
+ apply_error_context_stack = errcallback.previous;
> IIUC no contexts can be stacked at the begining of the worker, i.e.,
> apply_error_context_stack can be NULL in any case. So the global variable can be
> removed if we clean up more aggressively. I also felt the name can be changed,
> but it's retained for now because the variable has been exposed.
>
> BTW, I also checked the REPACK CONCURRENTLY, but it seemed to have the same issue
> as the parallel apply. I locally found a reproducer and wrote a fix patch,
> I can share here or another place.
I felt we can discuss here itself as the fix should be similar for
both the cases and reviewing also would be easier.
Regards,
Vignesh
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Manu | 2026-10-08 09:34:50 | Re: Add a hint to the "WAL summaries are required" errors |
| Previous Message | Ashutosh Bapat | 2026-10-08 09:26:24 | Re: Make memory checking / sanitizing infrastructure better |