Hi, Thank you for working on this!
On Fri, 6 Dec 2024 at 03:32, Melanie Plageman <melanieplage...@gmail.com> wrote: > > Here's an example to exercise the new log message: > > create table foo (a int, b int) with (autovacuum_enabled = false); > insert into foo select generate_series(1,1000), 1; > delete from foo where a > 500; > vacuum (verbose) foo; > -- visibility map: 5 pages newly set all-visible, of which 2 set > all-frozen. 0 all-visible pages newly set all-frozen. > insert into foo select generate_series(1,500), 1; > vacuum (verbose, freeze) foo; > --visibility map: 3 pages newly set all-visible, of which 3 set > all-frozen. 2 all-visible pages newly set all-frozen. 0001 and 0002 LGTM. I have a small comment about 0003: + /* + * If the page wasn't already set all-visible and all-frozen in + * the VM, count it as newly set for logging. + */ + if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 && + (old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0) + { + vacrel->vm_new_visible_pages++; + vacrel->vm_new_visible_frozen_pages++; + } + /* + * If the page wasn't already set all-visible and all-frozen in the + * VM, count it as newly set for logging. + */ + if ((old_vmbits & VISIBILITYMAP_ALL_VISIBLE) == 0 && + (old_vmbits & VISIBILITYMAP_ALL_FROZEN) == 0) + { + vacrel->vm_new_visible_pages++; + if (presult.all_frozen) + vacrel->vm_new_visible_frozen_pages++; + } I think there is no need to check VISIBILITYMAP_ALL_FROZEN in these if conditions. If the page is not all-visible; it can not be all-frozen, right? -- Regards, Nazir Bilal Yavuz Microsoft