MIME-Version: 1.0
References: <ac_vLKVgs4RYK1P-@nathan>
 <CAA5RZ0sCRjH3xkHFdSXnKysdMZXFyaS_094+K-O_rr4Fkmwc=Q@mail.gmail.com>
 <adB6FQeoUst6gHA0@nathan>
 <CAA5RZ0t6bCR6Pasd_eWqXvyUmrNDWmHUy1yN_Po-c4+b9BtBqQ@mail.gmail.com>
 <adEhRzu4YM7ztLXr@nathan>
 <CAA5RZ0vxby2osMMaCuZ=680tmt583cF9n4rOzTGdsiS-1PJknA@mail.gmail.com>
 <adE9RbcA7lhH_w2d@nathan>
 <CAA5RZ0tYzPDeXbqz9uKacRV2xjD055qrg=s3w2PEQPK8EruuSg@mail.gmail.com>
 <adFiCgN22xJ7Z-oR@nathan>
 <CAA5RZ0vRP-W2wJD2OxEb-=VGj2sp5pMCqHQg9YJiuDVPhaY5jQ@mail.gmail.com>
 <adQsdvPPNviWMCXb@nathan> <5684f479-858e-4c5d-b8f5-bcf05de1f909@gmail.com>
In-Reply-To: <5684f479-858e-4c5d-b8f5-bcf05de1f909@gmail.com>
From: Sami Imseih <samimseih@gmail.com>
Date: Wed, 8 Apr 2026 13:14:42 -0500
Message-ID: 
 <CAA5RZ0vsO9ZtSwoQdug390sLyVhDku9ZLCAWeCdX4KP0rQ1Ghg@mail.gmail.com>
Subject: Re: Add pg_stat_autovacuum_priority
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Nathan Bossart <nathandbossart@gmail.com>,
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>,
 Robert Treat <rob@xzilla.net>,
	satyanarlapuram@gmail.com, pgsql-hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/mixed; boundary="0000000000006ad8da064ef6e06d"
Archived-At: 
 <https://www.postgresql.org/message-id/CAA5RZ0vsO9ZtSwoQdug390sLyVhDku9ZLCAWeCdX4KP0rQ1Ghg%40mail.gmail.com>
Precedence: bulk

--0000000000006ad8da064ef6e06d
Content-Type: text/plain; charset="UTF-8"

> 07.04.2026 00:58, Nathan Bossart wrote:
> > Committed after some more editorialization.
>
> Please look at a new anomaly, I and SQLsmith have discovered:
> SELECT (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1),
>      (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1);
> ERROR:  detected double pfree in PgStat Snapshot 0x5f6fa4d95d50

oops, nice catch!

With the default stats_fetch_consistency setting of
PGSTAT_FETCH_CONSISTENCY_CACHE,
the stats returned by pgstat_fetch_entry() are allocated in the
snapshot's memory context,
so they get free'd when the snapshot is cleared by pgstat_clear_snapshot().
That means inside relation_needs_vacanalyze() we should only
pfree(tabentry) when
pgstat_fetch_consistency is PGSTAT_FETCH_CONSISTENCY_NONE,
as in that mode the stats are palloc'd in the caller's memory context
and must be freed explicitly.

autovacuum.c forces pgstat_fetch_consistency to PGSTAT_FETCH_CONSISTENCY_NONE
for the autovacuum launcher, so the pfree() was never an issue there.
I don't think we should do the same for pg_stat_get_autovacuum_scores
, as we should
not override the users intentions for fetch consistency, and also it
would complicate the
view greatly as we must force a _NONE consistency and a PG_TRY/PG_CATCH
to restore the original mode.

Attached is the fix:

postgres=# set stats_fetch_consistency = NONE;
SET
postgres=# SELECT (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1),
     (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1);
  score  |  score
---------+---------
 8.5e-08 | 8.5e-08
(1 row)

postgres=# set stats_fetch_consistency = snapshot;
SET
postgres=# SELECT (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1),
     (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1);
  score  |  score
---------+---------
 8.5e-08 | 8.5e-08
(1 row)

postgres=# set stats_fetch_consistency = cache;
SET
postgres=# SELECT (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1),
     (SELECT score FROM pg_stat_get_autovacuum_scores() LIMIT 1);
  score  |  score
---------+---------
 8.5e-08 | 8.5e-08
(1 row)

--0000000000006ad8da064ef6e06d
Content-Type: application/octet-stream;
	name="v1-0001-Fix-double-free-in-relation_needs_vacanalyze.patch"
Content-Disposition: attachment;
	filename="v1-0001-Fix-double-free-in-relation_needs_vacanalyze.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_mnqdammc0>
X-Attachment-Id: f_mnqdammc0

RnJvbSBhYmMxOTE1OGRhMDI2ODljM2MxMjA4MjUxOWJmODVmMTFhYjk2ZmViIE1vbiBTZXAgMTcg
MDA6MDA6MDAgMjAwMQpGcm9tOiBTYW1pIEltc2VpaCA8c2FtaW1zZWloQGdtYWlsLmNvbT4KRGF0
ZTogV2VkLCA4IEFwciAyMDI2IDE4OjA2OjQ2ICswMDAwClN1YmplY3Q6IFtQQVRDSCB2MSAxLzFd
IEZpeCBkb3VibGUgZnJlZSBpbiByZWxhdGlvbl9uZWVkc192YWNhbmFseXplCgpXaGVuIHJlbGF0
aW9uX25lZWRzX3ZhY2FuYWx5emUgaXMgZXhlY3V0ZWQgbXVsdGlwbGUgdGltZXMgd2l0aApzdGF0
X2ZldGNoX2NvbnNpc3RlbmN5IG90aGVyIHRoYW4gTk9ORSwgYSBkb3VibGUgZnJlZSBvY2N1cnMK
Zm9yIHRoZSB0YWJsZSBzdGF0cy4gRmlyc3QgaW4gcmVsYXRpb25fbmVlZHNfdmFjYW5hbHl6ZSwg
dGhlbgp3aGVuIHRoZSBzbmFwc2hvdCdzIG1lbW9yeSBjb250ZXh0IGlzIGZyZWVkLiBJbiB0aGUg
Tk9ORSBjYXNlLAp0aGUgc25hcHNob3QgaXMgc3RvcmVkIGluIHRoZSBjYWxsZXIncyBtZW1vcnkg
Y29udGV4dCwgc28gaXQncwp0aGUgY2FsbGVyJ3MgcmVzcG9uc2liaWxpdHkgdG8gZnJlZSB0aGUg
bWVtb3J5LgoKRml4IHRoaXMgYnkgb25seSBjYWxsaW5nIHBmcmVlIGluc2lkZSByZWxhdGlvbl9u
ZWVkc192YWNhbmFseXplCndoZW4gc3RhdF9mZXRjaF9jb25zaXN0ZW5jeSBpcyBOT05FLgotLS0K
IHNyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvYXV0b3ZhY3V1bS5jIHwgOCArKysrKysrLQogMSBmaWxl
IGNoYW5nZWQsIDcgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbigtKQoKZGlmZiAtLWdpdCBhL3Ny
Yy9iYWNrZW5kL3Bvc3RtYXN0ZXIvYXV0b3ZhY3V1bS5jIGIvc3JjL2JhY2tlbmQvcG9zdG1hc3Rl
ci9hdXRvdmFjdXVtLmMKaW5kZXggYmQ2MjZhMTYzNjMuLmIxZDEyZTNhNjFjIDEwMDY0NAotLS0g
YS9zcmMvYmFja2VuZC9wb3N0bWFzdGVyL2F1dG92YWN1dW0uYworKysgYi9zcmMvYmFja2VuZC9w
b3N0bWFzdGVyL2F1dG92YWN1dW0uYwpAQCAtMzMyNyw3ICszMzI3LDEzIEBAIHJlbGF0aW9uX25l
ZWRzX3ZhY2FuYWx5emUoT2lkIHJlbGlkLAogCQkJIGFubHR1cGxlcywgYW5sdGhyZXNoLCBzY29y
ZXMtPmFubCwKIAkJCSBzY29yZXMtPnhpZCwgc2NvcmVzLT5teGlkKTsKIAotCXBmcmVlKHRhYmVu
dHJ5KTsKKwkvKgorCSAqIEluIFBHU1RBVF9GRVRDSF9DT05TSVNURU5DWV9OT05FIG1vZGUsIHN0
YXRzIGFyZSBwYWxsb2MnZCBpbiB0aGUKKwkgKiBjYWxsZXIncyBtZW1vcnkgY29udGV4dCBhbmQg
bXVzdCBiZSBmcmVlZCBleHBsaWNpdGx5LiAgSW4gb3RoZXIgbW9kZXMsCisJICogdGhleSBhcmUg
bWFuYWdlZCBieSB0aGUgc25hcHNob3QncyBtZW1vcnkgY29udGV4dC4KKwkgKi8KKwlpZiAocGdz
dGF0X2ZldGNoX2NvbnNpc3RlbmN5ID09IFBHU1RBVF9GRVRDSF9DT05TSVNURU5DWV9OT05FKQor
CQlwZnJlZSh0YWJlbnRyeSk7CiB9CiAKIC8qCi0tIAoyLjUwLjEKCg==
--0000000000006ad8da064ef6e06d--