| From: | Xuneng Zhou <xunengzhou(at)gmail(dot)com> |
|---|---|
| To: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
| Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "itsajin(at)gmail(dot)com" <itsajin(at)gmail(dot)com>, Khoa Nguyen <kdnguyen9(dot)oss(at)gmail(dot)com> |
| Subject: | Re: Bound memory usage during manual slot sync retries |
| Date: | 2026-05-29 07:29:07 |
| Message-ID: | CABPTF7VSg2UK1K1b3UMV6TNNWeZmHRdriF5KqpBScoOz4LNy=Q@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, May 29, 2026 at 10:59 AM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Friday, May 29, 2026 10:31 AM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
> >
> > On Fri, May 29, 2026 at 12:43 AM Khoa Nguyen <kdnguyen9(dot)oss(at)gmail(dot)com>
> > wrote:
> > >
> > > I also reviewed the script and it is well done. The script setup and
> > > capture data points showing memory leak within the session over time.
> > > The patch looks fine though I think that oldctx should be captured
> > > once outside the retry loop since the current logic is more about
> > > parent and child memory context rather than previous and current
> > > context. Nevertheless, the current code works as well.
> >
> > Thanks for your review and verification. I agree with your suggestion
> > that capturing the outer memory context once outside the retry loop
> > looks cleaner and reflect the lifetime model better. I am also
> > wondering whether renaming the oldctx to outerctx could express the
> > parent/child relationship more clearly. That said, I am ok with the
> > status quo, if Amit thinks no further modification is needed.
>
> I am personally -1 on this change. The patch makes the logic somewhat harder for
> me to follow and understand. IIUC, the patch assumes that all code executed
> after creating sync_retry_ctx should fall under the 'outerctx'. This assumption
> seems fragile as it could easily be broken if someone creates another temp
> context and places the entire loop logic inside that context.
Both patterns exist in the tree. I've studied the use of them a bit.
My current takeaway is that the fixed-context is preferred when the
saved context has a named semantic role like result, caller context
and the code switches to it only for allocations that must live there.
The slotsyc case does not meet these conditions. Its semantic rule is
that slot_names must survive sysnc_retry_ctx resets. It does not
specifically need to live in "the context active before sync_retry_ctx
was created." So I am also inclined to keep the status quo.
--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Japin Li | 2026-05-29 07:38:32 | Re: [PATCH] Add pg_get_policy_ddl() function to reconstruct CREATE POLICY statement |
| Previous Message | Jakub Wartak | 2026-05-29 07:25:58 | Re: PostgreSQL 19 Beta 1 release announcement draft |