MIME-Version: 1.0
References: 
 <CABPTF7VVPxgfYyr8Kyi=+JACjckQ6NpniV9eRtHboj2hMn0REw@mail.gmail.com>
 <CAA4eK1+4KCedvh1AWuWf_A9AvS79Jdt8Mu-22ofKV143gEu8Ow@mail.gmail.com>
 <CABPTF7X5mn8qupFxTfDL0SSzEzepRM4ou3rm7Lw=Z=nX3PEDgg@mail.gmail.com>
 <CABPTF7VRRfkyTEBuzdvN=Tniki6ugXo6Wy3Ejx6xUAF1-F9ACA@mail.gmail.com>
 <CAA4eK1Kc_wF_sN=fjBAOPqbTonM71bd+aM0CfHg9D7oObogCEQ@mail.gmail.com>
 <CAA4eK1L=P+4hdnic8gQ1QFL9KZiM_mReCNTYWx99ct23mCoZ8g@mail.gmail.com>
 <CABPTF7XCcMq=AeCY-E67VRwo=ap53bfmKngr289PirAo4XuUXA@mail.gmail.com>
 <CAA4eK1+mGigBhtZG3ZNZim0EDDcrEjauCTNp7S1166xesSRnFQ@mail.gmail.com>
 <CAONt3B0UAQtmNFLyMWTrfbP1xUFtJsyCBn-7tNKAobSy4rDkYQ@mail.gmail.com>
In-Reply-To: 
 <CAONt3B0UAQtmNFLyMWTrfbP1xUFtJsyCBn-7tNKAobSy4rDkYQ@mail.gmail.com>
From: Xuneng Zhou <xunengzhou@gmail.com>
Date: Fri, 29 May 2026 10:30:57 +0800
Message-ID: 
 <CABPTF7WpEb8hBYyLU7wKjmxuMrtjgyKN-AMLsyTb+RhS9pFatw@mail.gmail.com>
Subject: Re: Bound memory usage during manual slot sync retries
To: Khoa Nguyen <kdnguyen9.oss@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>,
	pgsql-hackers <pgsql-hackers@lists.postgresql.org>,
	"itsajin@gmail.com" <itsajin@gmail.com>,
 "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Content-Type: multipart/mixed; boundary="0000000000004d289d0652eba372"
Archived-At: 
 <https://www.postgresql.org/message-id/CABPTF7WpEb8hBYyLU7wKjmxuMrtjgyKN-AMLsyTb%2BRhS9pFatw%40mail.gmail.com>
Precedence: bulk

--0000000000004d289d0652eba372
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Khoa,

On Fri, May 29, 2026 at 12:43=E2=80=AFAM Khoa Nguyen <kdnguyen9.oss@gmail.c=
om> wrote:
>
> On Tue, May 26, 2026 at 4:39=E2=80=AFPM Amit Kapila <amit.kapila16@gmail.=
com> wrote:
> >
> > On Tue, May 26, 2026 at 1:01=E2=80=AFPM Xuneng Zhou <xunengzhou@gmail.c=
om> wrote:
> > >
> > >
> > >> On Mon, May 25, 2026 at 7:03=E2=80=AFPM Amit Kapila <amit.kapila16@g=
mail.com> wrote:
> > >> >
> > >> > Okay, then let's go with a per-retry memory context approach.
> > >> >
> > >> > @@ -579,6 +579,8 @@ drop_local_obsolete_slots(List *remote_slot_li=
st)
> > >> >      local_slot->data.database));
> > >> >   }
> > >> >   }
> > >> > +
> > >> > + list_free(local_slots);
> > >> >  }
> > >> >
> > >> > Why do we need this retail pfree if the caller is using memory con=
text?
> > >> >
> > >>
> > >> 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/CABPTF7WB4Z62sPoZkhSygOC=
Ao3OiTDLpMELxZDuwCb3HYgM_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 pr=
operly. 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.

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.

--=20
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

--0000000000004d289d0652eba372
Content-Type: application/octet-stream;
	name="v1-0001-Use-outer-memory-context-across-slot-sync-retries.patch"
Content-Disposition: attachment;
	filename="v1-0001-Use-outer-memory-context-across-slot-sync-retries.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mpqb13q50>
X-Attachment-Id: f_mpqb13q50

RnJvbSBhYTk5ZDAxN2IwNGFmNTg2ZDc0ZmFmYzY3NjU2Mjg5Zjg1NjMzYTg3IE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBhbHRlcmVnbzY1NSA8ODI0NjYyNTI2QHFxLmNvbT4KRGF0ZTog
RnJpLCAyOSBNYXkgMjAyNiAxMDoyODo1NyArMDgwMApTdWJqZWN0OiBbUEFUQ0ggdjFdIFVzZSBv
dXRlciBtZW1vcnkgY29udGV4dCBhY3Jvc3Mgc2xvdCBzeW5jIHJldHJpZXMKCkNhcHR1cmUgdGhl
IG91dGVyIG1lbW9yeSBjb250ZXh0IG9uY2UgYmVmb3JlIHRoZSByZXRyeSBsb29wIGluClN5bmNS
ZXBsaWNhdGlvblNsb3RzKCkgdG8gYmV0dGVyIGJldHRlciBtYXRjaGVzIHRoZSBhY3R1YWwgbGlm
ZXRpbWUgbW9kZWw6CnN5bmNfcmV0cnlfY3R4IGlzIHRoZSBwZXItY3ljbGUgY2hpbGQgY29udGV4
dCwgd2hpbGUgc2xvdF9uYW1lcyBtdXN0IGxpdmUKaW4gdGhlIHBhcmVudC9vdXRlciBjb250ZXh0
IGFjcm9zcyByZXNldHMuIFJlbmFtZSB0aGUgb2xkY3R4IHRvIG91dGVyY3R4CnRvIGV4cHJlc3Mg
dGhlIHBhcmVudC9jaGlsZCByZWxhdGlvbnNoaXAgbW9yZSBjbGVhcmx5LgotLS0KIHNyYy9iYWNr
ZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvc2xvdHN5bmMuYyB8IDEwICsrKysrKy0tLS0KIDEgZmls
ZSBjaGFuZ2VkLCA2IGluc2VydGlvbnMoKyksIDQgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEv
c3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9zbG90c3luYy5jIGIvc3JjL2JhY2tlbmQv
cmVwbGljYXRpb24vbG9naWNhbC9zbG90c3luYy5jCmluZGV4IDkzZjQxYmUzMmFmLi5lMjIxNTVi
NzE5MiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcmVwbGljYXRpb24vbG9naWNhbC9zbG90c3lu
Yy5jCisrKyBiL3NyYy9iYWNrZW5kL3JlcGxpY2F0aW9uL2xvZ2ljYWwvc2xvdHN5bmMuYwpAQCAt
MjAxNCwxNyArMjAxNCwyMCBAQCBTeW5jUmVwbGljYXRpb25TbG90cyhXYWxSZWNlaXZlckNvbm4g
KndyY29ubikKIAkJTGlzdAkgICAqcmVtb3RlX3Nsb3RzID0gTklMOwogCQlMaXN0CSAgICpzbG90
X25hbWVzID0gTklMOwkvKiBMaXN0IG9mIHNsb3QgbmFtZXMgdG8gdHJhY2sgKi8KIAkJTWVtb3J5
Q29udGV4dCBzeW5jX3JldHJ5X2N0eDsKKwkJTWVtb3J5Q29udGV4dCBvdXRlcmN0eDsKIAogCQlj
aGVja19hbmRfc2V0X3N5bmNfaW5mbyhNeVByb2NQaWQpOwogCiAJCXZhbGlkYXRlX3JlbW90ZV9p
bmZvKHdyY29ubik7CiAKKwkJb3V0ZXJjdHggPSBDdXJyZW50TWVtb3J5Q29udGV4dDsKKwogCQkv
KgogCQkgKiBTZXR1cCBhbmQgdXNlIGEgcGVyLXN5bmMtY3ljbGUgbWVtb3J5IGNvbnRleHQsIHdo
aWNoIGlzIHJlc2V0IGV2ZXJ5CiAJCSAqIHRpbWUgd2UgbG9vcCBiZWxvdy4gVGhpcyBhdm9pZHMg
aGF2aW5nIHRvIHJldGFpbCBmcmVlaW5nIHRoZSBtZW1vcnkKIAkJICogdXNlZCBpbiBlYWNoIHN5
bmMgY3ljbGUuCiAJCSAqLwotCQlzeW5jX3JldHJ5X2N0eCA9IEFsbG9jU2V0Q29udGV4dENyZWF0
ZShDdXJyZW50TWVtb3J5Q29udGV4dCwKKwkJc3luY19yZXRyeV9jdHggPSBBbGxvY1NldENvbnRl
eHRDcmVhdGUob3V0ZXJjdHgsCiAJCQkJCQkJCQkJCSAgICJzbG90IHN5bmMgcmV0cnkgY29udGV4
dCIsCiAJCQkJCQkJCQkJCSAgIEFMTE9DU0VUX0RFRkFVTFRfU0laRVMpOwogCkBAIC0yMDMzLDcg
KzIwMzYsNiBAQCBTeW5jUmVwbGljYXRpb25TbG90cyhXYWxSZWNlaXZlckNvbm4gKndyY29ubikK
IAkJewogCQkJYm9vbAkJc2xvdF9wZXJzaXN0ZW5jZV9wZW5kaW5nID0gZmFsc2U7CiAJCQlib29s
CQlzb21lX3Nsb3RfdXBkYXRlZCA9IGZhbHNlOwotCQkJTWVtb3J5Q29udGV4dCBvbGRjdHg7CiAK
IAkJCS8qIENoZWNrIGZvciBpbnRlcnJ1cHRzIGFuZCBjb25maWcgY2hhbmdlcyAqLwogCQkJQ0hF
Q0tfRk9SX0lOVEVSUlVQVFMoKTsKQEAgLTIwNDUsNyArMjA0Nyw3IEBAIFN5bmNSZXBsaWNhdGlv
blNsb3RzKFdhbFJlY2VpdmVyQ29ubiAqd3Jjb25uKQogCQkJQXNzZXJ0KElzVHJhbnNhY3Rpb25T
dGF0ZSgpKTsKIAogCQkJTWVtb3J5Q29udGV4dFJlc2V0KHN5bmNfcmV0cnlfY3R4KTsKLQkJCW9s
ZGN0eCA9IE1lbW9yeUNvbnRleHRTd2l0Y2hUbyhzeW5jX3JldHJ5X2N0eCk7CisJCQlNZW1vcnlD
b250ZXh0U3dpdGNoVG8oc3luY19yZXRyeV9jdHgpOwogCiAJCQkvKgogCQkJICogRmV0Y2ggcmVt
b3RlIHNsb3QgaW5mbyBmb3IgdGhlIGdpdmVuIHNsb3RfbmFtZXMuIElmIHNsb3RfbmFtZXMKQEAg
LTIwNjcsNyArMjA2OSw3IEBAIFN5bmNSZXBsaWNhdGlvblNsb3RzKFdhbFJlY2VpdmVyQ29ubiAq
d3Jjb25uKQogCQkJICogc2xvdF9uYW1lcyBtdXN0IHN1cnZpdmUgbGF0ZXIgc3luY19yZXRyeV9j
dHggcmVzZXRzLCBzbyBjb3B5IGl0CiAJCQkgKiBpbiB0aGUgb3V0ZXIgY29udGV4dC4KIAkJCSAq
LwotCQkJTWVtb3J5Q29udGV4dFN3aXRjaFRvKG9sZGN0eCk7CisJCQlNZW1vcnlDb250ZXh0U3dp
dGNoVG8ob3V0ZXJjdHgpOwogCiAJCQkvKgogCQkJICogSWYgc2xvdF9wZXJzaXN0ZW5jZV9wZW5k
aW5nIGlzIHRydWUsIGV4dHJhY3Qgc2xvdCBuYW1lcyBmb3IKLS0gCjIuNTEuMAoK
--0000000000004d289d0652eba372--