MIME-Version: 1.0
References: <utrlxij43fbguzw4kldte2spc4btoldizutcqyrfakqnbrp3ir@ph3sphpj4asz>
 <CAPpHfdt5BHkpvFnwg_RiMrCdD8W5FhOoahqY2Td-y3kb45UpZw@mail.gmail.com>
 <CAPpHfduJQPTrXpJr=-mHtsJH8+1v_X389K6trajPzhnA37C9nA@mail.gmail.com>
 <CALT9ZEGioDdvkx1rmjVzOW5nyxsxjX16FVifYY4b7MVFuc_80g@mail.gmail.com>
 <CABPTF7X2HXXiUk1+Vd_pzPhiYOHw0y=OnDXW87G7hfPL0577hw@mail.gmail.com>
 <CALT9ZEGsc3oSqtaHtevz5=SZaooW6hPP8ZwNEZCSEw2kePaHvQ@mail.gmail.com>
 <CAPpHfdvwdoMNZh-pgg9NiYT_rexcsze5OfyB8cZHS6XFSNMsnw@mail.gmail.com>
 <CALT9ZEFnE_aJ=nr5nMSxLYsNCfnKVOPFbiMZo5T_VoMb-sjO6Q@mail.gmail.com>
 <CAPpHfdtfJ66rfZXO__gyDCyKBbusqXA_JtebXQNZuE_O-smRVw@mail.gmail.com>
In-Reply-To: 
 <CAPpHfdtfJ66rfZXO__gyDCyKBbusqXA_JtebXQNZuE_O-smRVw@mail.gmail.com>
From: Xuneng Zhou <xunengzhou@gmail.com>
Date: Mon, 16 Mar 2026 16:31:45 +0800
Message-ID: 
 <CABPTF7WtCQT=gFuRNRVFMXyJB-U1MBAmFyie+pyQUSZ+FzwWig@mail.gmail.com>
Subject: Re: Odd code around ginScanToDelete
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>,
 Andres Freund <andres@anarazel.de>,
	pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CABPTF7WtCQT%3DgFuRNRVFMXyJB-U1MBAmFyie%2BpyQUSZ%2BFzwWig%40mail.gmail.com>
Precedence: bulk

On Thu, Mar 12, 2026 at 6:37=E2=80=AFAM Alexander Korotkov <aekorotkov@gmai=
l.com> wrote:
>
> On Thu, Mar 12, 2026 at 12:35=E2=80=AFAM Pavel Borisov <pashkin.elfe@gmai=
l.com> wrote:
> >
> > On Thu, 12 Mar 2026 at 02:22, Alexander Korotkov <aekorotkov@gmail.com>=
 wrote:
> > >
> > > On Tue, Mar 10, 2026 at 11:29=E2=80=AFAM Pavel Borisov <pashkin.elfe@=
gmail.com> wrote:
> > > > Hi, Xuneng
> > > >
> > > > > > Is it worth/possible in recursive calls of ginScanToDelete() to=
 free
> > > > > > allocated myStackItem->child after processing all children of t=
he
> > > > > > current level, when they are not needed anymore?
> > > > > > Previously to this patch, palloc-ed "me" variable also was't fr=
eed at
> > > > > > recursion levels.
> > > > >
> > > > > Freeing/reallocating it per subtree would add churn and make the
> > > > > lifetime rules harder to reason about without meaningful memory
> > > > > savings (the number of nodes is bounded by tree depth, not number=
 of
> > > > > pages). We currently free the chain once after ginScanToDelete()
> > > > > returns in ginVacuumPostingTree(), which matches the natural life=
time
> > > > > boundary
> > > > I proposed not freeing child when child iteration is complete. They
> > > > indeed can be reused. I proposed cleaning children when "my" iterat=
ion
> > > > is complete. At that time all the children iterations are completed
> > > > and not needed when we return level up.
> > > This is not clear for me.  We need stack items to keep track of left
> > > pages until we scan the whole posting tree.  After scanning the whole
> > > posting tree we can free stack items as we do now.
> >
> > You are right, that we can free all posting tree stack items after the
> > whole tree, as we do now. But I think we can also do it earlier. It
> > looks like all "children" items are needed and could be reused only
> > until iteration on "my" level ends. When function returns up the
> > recursion "my" level becomes "child" for a caller, and previous
> > "child" is not used anymore.
>
> No matter how many levels we can go up, we can still descend and need
> the leftBuffer stored at any stack level.
>

Yeah, the important point is that a stack item here represents
per-depth scan state, not just one recursive invocation. Returning
from one subtree does not necessarily mean that depth is finished
globally: the caller may move to a sibling and descend again, and that
later descent can still need the child level's saved leftBuffer from
the subtree we just finished. The stronger condition is that no more
pages remain to be scanned to the right at that depth; the code
already uses GinPageRightMost(page) for that when releasing the child
level's leftBuffer.

--=20
Best,
Xuneng