On Tue, May 26, 2026 at 4:39 PM Amit Kapila <[email protected]> wrote: > > On Tue, May 26, 2026 at 1:01 PM Xuneng Zhou <[email protected]> wrote: > > > > > >> On Mon, May 25, 2026 at 7:03 PM Amit Kapila <[email protected]> > >> wrote: > >> > > >> > Okay, then let's go with a per-retry memory context approach. > >> > > >> > @@ -579,6 +579,8 @@ drop_local_obsolete_slots(List *remote_slot_list) > >> > local_slot->data.database)); > >> > } > >> > } > >> > + > >> > + list_free(local_slots); > >> > } > >> > > >> > Why do we need this retail pfree if the caller is using memory context? > >> > > >> > >> I see that the latest patch in email [1] has already addressed this > >> point. So, I'll push the v2 version. > >> > >> [1] - > >> https://www.postgresql.org/message-id/CABPTF7WB4Z62sPoZkhSygOCAo3OiTDLpMELxZDuwCb3HYgM_pQ%40mail.gmail.com > > > > > > Thanks. My original reasoning for adding the pfree here was to act as a > > safety guard in case other future callers might not manage the memory > > properly. But Zhijie pointed out that this double-free pattern is not > > favored in previous community discussions. I'll post the worst-case test > > and its results later. > > > > Pushed.
This patch is already pushed by Amit but I wanted to add my review from a validation standpoint. I was able to use the measure_slotsync_memory.sh to verify the leak that Xuneng reported. pre-patch timestamp,backend_pid,total_bytes,delta_bytes 2026-05-27T16:52:20Z,78800,1339920, 2026-05-27T16:52:22Z,78800,1405456,65536 2026-05-27T16:52:24Z,78800,1405456,0 2026-05-27T16:52:26Z,78800,1405456,0 2026-05-27T16:52:28Z,78800,1536528,131072 2026-05-27T16:52:30Z,78800,1536528,0 2026-05-27T16:52:32Z,78800,1536528,0 patched timestamp,backend_pid,total_bytes,delta_bytes 2026-05-27T01:17:50Z,66917,1315344, 2026-05-27T01:17:52Z,66917,1315344,0 2026-05-27T01:17:54Z,66917,1315344,0 2026-05-27T01:17:56Z,66917,1315344,0 2026-05-27T01:17:58Z,66917,1315344,0 2026-05-27T01:18:00Z,66917,1315344,0 2026-05-27T01:18:02Z,66917,1315344,0 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.
