Re: Adding vacuum test case of setting the VM when heap page is unmodified
| От | Srinath Reddy Sadipiralla |
|---|---|
| Тема | Re: Adding vacuum test case of setting the VM when heap page is unmodified |
| Дата | |
| Msg-id | CAFC+b6oS-amAUG8ZPe6VWNLH0vqD2NURrcNTQMzh_pQ13cGXvw@mail.gmail.com обсуждение исходный текст |
| Ответ на | Adding vacuum test case of setting the VM when heap page is unmodified (Melanie Plageman <melanieplageman@gmail.com>) |
| Ответы |
Re: Adding vacuum test case of setting the VM when heap page is unmodified
|
| Список | pgsql-hackers |
Hi Melanie,
On Wed, Dec 10, 2025 at 11:21 PM Melanie Plageman <melanieplageman@gmail.com> wrote:
Hi,
While working on a patch to set the VM in the same WAL record as
pruning and freezing [1], I discovered we have no test coverage of the
case where vacuum phase I sets the VM but no modifications are made to
the heap buffer (not even setting PD_ALL_VISIBLE). This can only
happen when the VM was somehow removed or destroyed.
+1 for adding the test, but IIUC PD_ALL_VISIBLE is being set in this
case during the "vacuum test_vac_unmodified_heap;" because
VM bit is not set (as we truncated VM) and presult.all_visible is true as well ,
so it goes in if (!all_visible_according_to_vm && presult.all_visible), where its
doing these, this was the flow i observed while trying to understand the
patch by running the given test, please correct me if I'm wrong.
PageSetAllVisible(page);
MarkBufferDirty(buf);
old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
InvalidXLogRecPtr,
vmbuffer, presult.vm_conflict_horizon,
flags);
case during the "vacuum test_vac_unmodified_heap;" because
VM bit is not set (as we truncated VM) and presult.all_visible is true as well ,
so it goes in if (!all_visible_according_to_vm && presult.all_visible), where its
doing these, this was the flow i observed while trying to understand the
patch by running the given test, please correct me if I'm wrong.
PageSetAllVisible(page);
MarkBufferDirty(buf);
old_vmbits = visibilitymap_set(vacrel->rel, blkno, buf,
InvalidXLogRecPtr,
vmbuffer, presult.vm_conflict_horizon,
flags);
Currently, we require the heap buffer to be marked dirty even if it is
unmodified because we add it to the WAL chain and do not pass
REGBUF_NO_CHANGES. (And we require adding it to the WAL chain because
we update the freespace map using the heap buffer in recovery). The VM
being gone is an uncommon case, so I don't think it makes sense to add
special logic to pass REGBUF_NO_CHANGES. However, I do think we should
have a test for this case.
makes sense, i think this below comment supports your final decision
of not optimizing it.
* NB: If the heap page is all-visible but the VM bit is not set, we
* don't need to dirty the heap page. However, if checksums are
* enabled, we do need to make sure that the heap page is dirtied
* before passing it to visibilitymap_set(), because it may be logged.
* Given that this situation should only happen in rare cases after a
* crash, it is not worth optimizing.
*/
В списке pgsql-hackers по дате отправления: