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

Dear Vignesh,

While reviewing another thread at [1], I found an issue where an error
relayed from a parallel apply worker can get an unrelated CONTEXT line
from the leader process.
ProcessParallelApplyMessage() in applyparallelworker.c sets
error_context_stack to the leader's apply_error_context_stack before
calling ereport(ERROR). As a result, when errfinish() processes the
error, the leader's error context callback runs again and adds the
leader's current replication context. This can be incorrect if the
leader is processing a different transaction when it receives the
error from the parallel worker.

Good catch and agreed your analysis.

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.
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.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

On Thu, 8 Oct 2026 at 13:23, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.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

Dear Vignesh,

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:

Added. Also, I updated the comment in ProcessParallelApplyMessage().

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.

OK, then let me attach and describe once. 0003 is the reproducer, and 0004 is the fix
patch.

001_repack_error_context reproduced the issue. A functional index is defined with
the injection point. While the backend waits at the point the repack worker does
the error out (also by the injection point). In this case the backend re-throws
the propagated from the repack worker, so the unrelated context can be printed.
Below is the obtained result, lines after the "SQL statement..." is not propagated
from the repack worker.

```
ERROR: error triggered for injection point repack-worker-error-context
CONTEXT: REPACK decoding worker
SQL statement "SELECT public.injection_points_run('repack-leader-error-context')"
PL/pgSQL function public.repack_error_context(integer) line 3 at PERFORM
```

This means the misleading lines in CONTEXT can be reported if the backend
installed the transient callbacks while processing tuples.

The fix idea is to preserve the context like parallel query. I added an attribute
in the DecodingWorker which controls the repack worker. It's preserved when the
worker is started, and it's restored when the backend reports the log.
It's mostly same as ProcessParallelMessage().

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi,

On Thu, Oct 8, 2026 at 3:54 PM Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:

Dear Vignesh,

While reviewing another thread at [1], I found an issue where an error
relayed from a parallel apply worker can get an unrelated CONTEXT line
from the leader process.
ProcessParallelApplyMessage() in applyparallelworker.c sets
error_context_stack to the leader's apply_error_context_stack before
calling ereport(ERROR). As a result, when errfinish() processes the
error, the leader's error context callback runs again and adds the
leader's current replication context. This can be incorrect if the
leader is processing a different transaction when it receives the
error from the parallel worker.

Good catch and agreed your analysis.

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.
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.

I think it makes sense to preserve the error context before the loop, to be
consistent with what parallel query does.

But I think we'd better simply save the error context to
apply_error_context_stack before adding the error callback as that
looks better than
indirectly getting the previous one after adding a new one, and it can save
readers a few cycles. E.g.,

+apply_error_context_stack = error_context_stack;
errcallback.callback = apply_error_callback;
...

Best Regards,
Zhijie Hou

Dear Hou,

But I think we'd better simply save the error context to
apply_error_context_stack before adding the error callback as that
looks better than
indirectly getting the previous one after adding a new one, and it can save
readers a few cycles. E.g.,

Indeed, PSA the new version.
Also, I adjusted the order of the initialization of DecodingWorker to align with
the location of attributes.

Best regards,
Hayato Kuroda
FUJITSU LIMITED

Hi all

On 2026/10/09 11:34, Hayato Kuroda (Fujitsu) wrote:

Indeed, PSA the new version.
Also, I adjusted the order of the initialization of DecodingWorker to align with
the location of attributes.

I don't have any objections to the fixes and am learning from the fixes.

But let me add two minor comments.
Do we plan to commit the test codes as well?
While I was running test codes, I noticed
src/test/subscription/meson.build lacks
't/039_streaming_parallel_apply_error_context.pl line. Without this
line, meson test don't execute this test.

+ 'enable_injection_points': 'yes',

The other minor thing is, I believe we can remove this line from
injection_points/meson.build since
the repack test code does not check this environment value and without
this I was able to execute this test. Moreover, the tests under
injection_points directory just finish if injection_points is not
configured according to meson.build.

Regards,
Takeshi Ideriha

Hi Ideriha-san,

Thanks for the review. PSA v4 patch set.

I don't have any objections to the fixes and am learning from the fixes.

But let me add two minor comments.
Do we plan to commit the test codes as well?
While I was running test codes, I noticed
src/test/subscription/meson.build lacks
't/039_streaming_parallel_apply_error_context.pl line. Without this
line, meson test don't execute this test.

I think the current code won't be included to core, thus we do not take care
the shape of meson.build (I have tested with fixing it locally).
But it might be better to add for the cfbot. Done.
Also, I found the test sometimes could not launch the parallel apply worker
because of the race condition with the table synchronization. I set to
copy_data = false instead.

+ 'enable_injection_points': 'yes',

The other minor thing is, I believe we can remove this line from
injection_points/meson.build since
the repack test code does not check this environment value and without
this I was able to execute this test. Moreover, the tests under
injection_points directory just finish if injection_points is not
configured according to meson.build.

Indeed, removed. I have not checked well due to the above reason.

Best regards,
Hayato Kuroda
FUJITSU LIMITED