Out-of-date comments about RecentGlobalXmin?

Hi,

Regards,
Zhang Mingli
On Sep 6, 2022, 16:03 +0800, Japin Li <japinli@hotmail.com>, wrote:

In commit dc7420c2c9, the RecentGlobalXmin variable is removed, however,
there are some places that reference it.

$ grep 'RecentGlobalXmin' -rn src/
src/backend/replication/logical/launcher.c:101: * the secondary effect that it sets RecentGlobalXmin. (This is critical
src/backend/utils/init/postinit.c:790: * interested in the secondary effect that it sets RecentGlobalXmin. (This
src/backend/postmaster/autovacuum.c:1898: * the secondary effect that it sets RecentGlobalXmin. (This is critical

Yeah, RecentGlobalXmin is removed.

It's out-of-date, doesn't it? I'm not sure s/RecentGlobalXmin/RecentXmin/g
is right. Any thoughts?

I’m afraid not, RecentGlobalXmin is split to several GlobalVis* variables.
Need to check one by one.

On 6 Sep 2022, at 10:10, Zhang Mingli <zmlpostgres@gmail.com> wrote:
On Sep 6, 2022, 16:03 +0800, Japin Li <japinli@hotmail.com>, wrote:

It's out-of-date, doesn't it? I'm not sure s/RecentGlobalXmin/RecentXmin/g
is right. Any thoughts?
I’m afraid not, RecentGlobalXmin is split to several GlobalVis* variables.
Need to check one by one.

It's a set of functions actually and not variables IIRC.

It's worth looking at the entire comment and not just the grep output though,
as these three places share the exact same comment. Note the second paragraph:

/*
* Start a new transaction here before first access to db, and get a
* snapshot. We don't have a use for the snapshot itself, but we're
* interested in the secondary effect that it sets RecentGlobalXmin. (This
* is critical for anything that reads heap pages, because HOT may decide
* to prune them even if the process doesn't attempt to modify any
* tuples.)
*
* FIXME: This comment is inaccurate / the code buggy. A snapshot that is
* not pushed/active does not reliably prevent HOT pruning (->xmin could
* e.g. be cleared when cache invalidations are processed).
*/

This was added in dc7420c2c92 which removed RecentGlobalXmin, addressing that
FIXME would of course be very welcome.

--
Daniel Gustafsson https://vmware.com/

On Tue, 06 Sep 2022 at 16:17, Daniel Gustafsson <daniel@yesql.se> wrote:

On 6 Sep 2022, at 10:10, Zhang Mingli <zmlpostgres@gmail.com> wrote:
On Sep 6, 2022, 16:03 +0800, Japin Li <japinli@hotmail.com>, wrote:

It's out-of-date, doesn't it? I'm not sure s/RecentGlobalXmin/RecentXmin/g
is right. Any thoughts?
I’m afraid not, RecentGlobalXmin is split to several GlobalVis* variables.
Need to check one by one.

It's a set of functions actually and not variables IIRC.

It's worth looking at the entire comment and not just the grep output though,
as these three places share the exact same comment. Note the second paragraph:

/*
* Start a new transaction here before first access to db, and get a
* snapshot. We don't have a use for the snapshot itself, but we're
* interested in the secondary effect that it sets RecentGlobalXmin. (This
* is critical for anything that reads heap pages, because HOT may decide
* to prune them even if the process doesn't attempt to modify any
* tuples.)
*
* FIXME: This comment is inaccurate / the code buggy. A snapshot that is
* not pushed/active does not reliably prevent HOT pruning (->xmin could
* e.g. be cleared when cache invalidations are processed).
*/

This was added in dc7420c2c92 which removed RecentGlobalXmin, addressing that
FIXME would of course be very welcome.

My bad! Thanks for pointing out this.

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.