public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kirill Reshke <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Chao Li <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Wed, 17 Dec 2025 23:27:05 +0500
Message-ID: <CALdSSPhtPK36_sr9yFo3cN9TXQmjvSX3BZXCF8fVBLX-GETD0Q@mail.gmail.com> (raw)
In-Reply-To: <CAAKRu_ayWLg=WDGZZfSPWf0KjPM8u=LBb0D6XaEWyx2_YFFwAQ@mail.gmail.com>
References: <CAAKRu_ZP-3=SaZykpwDBMJOdUKyW3Wm5JZfPFRR3L5Ac8ouq4w@mail.gmail.com>
	<CAAKRu_bgkOQqu3K5n4YLRsNBZqJ9Rjg80ROqgKSr2UGz4b5hUg@mail.gmail.com>
	<2wk7jo4m4qwh5sn33pfgerdjfujebbccsmmlownybddbh6nawl@mdyyqpqzxjek>
	<CAAKRu_YR-COJ9aGnMUQqt5yoWmUBjikqrd4jGNZYouHaXpis9g@mail.gmail.com>
	<CALdSSPhiCwJwWwgJP1NmqRmnp9RS2tGOBY0gQrfLCbB+OS5_KQ@mail.gmail.com>
	<CAAKRu_YS+Ocm=OzMaZnG4egFiE9v4VYfZ25DXd6jbwegqmGYbQ@mail.gmail.com>
	<CAAKRu_ZGZSqhGt-RcmmfiSheC+1fjQdxy6_+oM-1jMn8hyVptQ@mail.gmail.com>
	<CALdSSPg+B8RTzTXhJvCcKJBqgzhPZkq0E2oqxQdv74ZNZOMVzg@mail.gmail.com>
	<CAAKRu_Zha7HcdQBv8tTtQrcry5332J6kHnOc1X=TT03LzUXDow@mail.gmail.com>
	<CAAKRu_amF00f2T_H8N6pbbe75C22EeX1OqA=svpj8LNO1sdUuw@mail.gmail.com>
	<mhf4vkmh3j57zx7vuxp4jagtdzwhu3573pgfpmnjwqa6i6yj5y@sy4ymcdtdklo>
	<CAAKRu_agCLAQ9OjmrTdJe-X=Xr7QnU4d=cfxdQGwc9jNx9w31w@mail.gmail.com>
	<CAAKRu_ayWLg=WDGZZfSPWf0KjPM8u=LBb0D6XaEWyx2_YFFwAQ@mail.gmail.com>

Hi!

in v27-0001:
> Melanie Plageman <melanieplageman(at)gmail(dot)com> wrote:
> > The last vacuum is expected to set vm bits, but the test doesn’t verify that. Should we verify that like:
> > ```
> > evantest=# SELECT blkno, all_visible, all_frozen FROM pg_visibility_map('test_vac_unmodified_heap');
> > blkno | all_visible | all_frozen
> > -------+-------------+------------
> > 0 | t | t
> > (1 row)

> I've done this. I've actually added three such verifications -- one
> after each step where the VM is expected to change. It shouldn't be
> very expensive, so I think it is okay. The way the test would fail if
> the buffer wasn't correctly dirtied is that it would assert out -- so
> the visibility map test wouldn't even have a chance to fail. But, I
> think it is also okay to confirm that the expected things are
> happening with the VM -- it just gives us extra coverage.

+1 on extra coverage. Should we also do sql-level check that the VM
indeed does not need to set PD_ALL_VISIBLE (check header bytes using
pageinspect?).


v27-0003 & v27-0004: I did not get the exact reason we introduced
`identify_and_fix_vm_corruption` in 0003 and moved code in 0004 to
another place. I can see we have this starting v25 of patch set. Well,
maybe this is not an issue at all...


in v27-0005. This patch changes code which is not exercised in
tests[0]. I spent some time understanding the conditions when we
entered this. There is a comment about non-finished relation
extension, but I got no success trying to reproduce this. I ended up
modifying code to lose PageSetAllVisible in proper places and running
vacuum. Looks like everything works as expected. I will spend some
more time on this, maybe I will be successful in writing an
injection-point-based TAP test which hits this...



[0] https://coverage.postgresql.org/src/backend/access/heap/vacuumlazy.c.gcov.html#1902
-- 
Best regards,
Kirill Reshke





view thread (143+ messages)  latest in thread

reply

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: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
  In-Reply-To: <CALdSSPhtPK36_sr9yFo3cN9TXQmjvSX3BZXCF8fVBLX-GETD0Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox