| From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
|---|---|
| To: | 'vignesh C' <vignesh21(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Takeshi Ideriha <iderihatakeshi(at)gmail(dot)com>, Zhijie Hou <houzhijie22(at)gmail(dot)com> |
| Subject: | RE: Incorrect CONTEXT reported for errors from parallel apply worker in logical replication |
| Date: | 2026-11-02 05:33:35 |
| Message-ID: | OS7PR01MB18317C73E32944F04A33E0290F56A2@OS7PR01MB18317.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Vignesh,
Thanks for the review!
> 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?
Indeed, replaced to apply_loop_outer_error_context_stack. Code comments were
added atop the declaration, so a comment in LogicalRepApplyLoop is adjusted (reduced).
> 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?
+1, but adjusted on my side. The commit id should be clarified at second paragraph,
and changes are clarified at the end.
I checked sentences via Grammarly.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Test-to-reproduce-parallel-apply-error-context-is.patch | application/octet-stream | 10.0 KB |
| v6-0002-Avoid-adding-leader-apply-context-to-parallel-wor.patch | application/octet-stream | 4.8 KB |
| v6-0003-Reproduce-the-mixture-of-the-error-context-by-REP.patch | application/octet-stream | 4.8 KB |
| v6-0004-Avoid-adding-backend-context-to-repack-worker-mes.patch | application/octet-stream | 2.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-11-02 05:34:47 | Re: Fix trigger memory leak |
| Previous Message | surya poondla | 2026-11-02 04:49:13 | Re: Wrong LSN in WAL decoding error messages |