From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Noah Misch <noah@leadboat.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Andrey M. Borodin <x4mmm@yandex-team.ru>
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Date: Wed, 22 Oct 2025 12:12:08 -0400
Message-ID: <798102.1761149528@sss.pgh.pa.us> (raw)
In-Reply-To: <CA+hUKGKN3oy0bN_3yv8hd78a4+M1tJC9z7mD8+f+yA+GeoFUwQ@mail.gmail.com>
References: <CAAKRu_ZuYsf4MNGzm7_KvG0SKsu-EtYRhFHy2x3q0heX8KLMTg@mail.gmail.com>
<CAAKRu_bQgf4e9OveCQX++zTEJLKbZbqLeapA7SNkNuMjxcCO1w@mail.gmail.com>
<20240306234733.nd4a636colxkgq2e@liskov>
<CAAKRu_aiaf+CufVA5Rrksz4Lw2fvZ3M-OgdBZ8WpzvYoQY96MQ@mail.gmail.com>
<CA+hUKGKHb3i8Wy72VCKZGA2B5djoj7tAzYAzbeS=Gwr_SdhgRw@mail.gmail.com>
<CAAKRu_byDppRvNJ+p5kq-SmXZDQuL6L8N6D06pHbO6WnBnanWQ@mail.gmail.com>
<CA+hUKGLY4Q4ZY4f1rvnFtv6+PkjNf8MejdPkcju3Qii9DYqqcQ@mail.gmail.com>
<CAAKRu_bbkmwAzSBgnezancgJeXrQZXy4G4kBTd+5=cr86H5yew@mail.gmail.com>
<20240707144944.58.nmisch@google.com>
<CA+hUKGK3sB-T+Ao8EBCOu5M1MsCMPnPpNyXuQGkQYHU4WC35eA@mail.gmail.com>
<20240716015226.60.nmisch@google.com>
<CA+hUKGKN3oy0bN_3yv8hd78a4+M1tJC9z7mD8+f+yA+GeoFUwQ@mail.gmail.com>
[ seizing on this old commit as being most closely related to the issue ]
Thomas Munro <thomas.munro@gmail.com> writes:
> On Tue, Jul 16, 2024 at 1:52 PM Noah Misch <noah@leadboat.com> wrote:
>> On Mon, Jul 15, 2024 at 03:26:32PM +1200, Thomas Munro wrote:
>> That's reasonable. radixtree already forbids mutations concurrent with
>> iteration, so there's no new concurrency hazard. One alternative is
>> per_buffer_data big enough for MaxOffsetNumber, but that might thrash caches
>> measurably. That patch is good to go apart from these trivialities:
> Thanks! I have pushed that patch, without those changes you didn't like.
The security team recently updated our Coverity instance to the latest
version, and it's started complaining as follows:
*** CID 1667418: Memory - corruptions (OVERRUN)
/srv/coverity/git/pgsql-git/postgresql/src/backend/access/heap/vacuumlazy.c: 2812 in lazy_vacuum_heap_rel()
2806 * already have the correct page pinned anyway.
2807 */
2808 visibilitymap_pin(vacrel->rel, blkno, &vmbuffer);
2809
2810 /* We need a non-cleanup exclusive lock to mark dead_items unused */
2811 LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
>>> CID 1667418: Memory - corruptions (OVERRUN)
>>> Overrunning callee's array of size 291 by passing argument "num_offsets" (which evaluates to 2048) in call to "lazy_vacuum_heap_page".
2812 lazy_vacuum_heap_page(vacrel, blkno, buf, offsets,
2813 num_offsets, vmbuffer);
2814
2815 /* Now that we've vacuumed the page, record its available space */
2816 page = BufferGetPage(buf);
2817 freespace = PageGetHeapFreeSpace(page);
The reason it thinks that num_offsets could be as much as 2048 is
presumably the code a little bit above this:
OffsetNumber offsets[MaxOffsetNumber];
...
num_offsets = TidStoreGetBlockOffsets(iter_result, offsets, lengthof(offsets));
Assert(num_offsets <= lengthof(offsets));
However, lazy_vacuum_heap_page blindly assumes that the passed value
will be no more than MaxHeapTuplesPerPage. It seems like we ought
to get these two functions in sync, either both using MaxOffsetNumber
or both using MaxHeapTuplesPerPage for their array sizes.
It looks to me like MaxHeapTuplesPerPage should be sufficient.
Also, after reading TidStoreGetBlockOffsets I wonder if we
should replace that Assert with
num_offsets = Min(num_offsets, lengthof(offsets));
Thoughts?
regards, tom lane
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, thomas.munro@gmail.com, noah@leadboat.com, melanieplageman@gmail.com, vignesh21@gmail.com, andres@anarazel.de, hlinnaka@iki.fi, byavuz81@gmail.com, robertmhaas@gmail.com, x4mmm@yandex-team.ru
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
In-Reply-To: <798102.1761149528@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox