Обсуждение: vacuum verbose: show pages marked allvisible/frozen/hintbits

I'm forking this thread since it's separate topic, and since keeping in a
single branch hasn't made maintaining the patches any easier.
https://www.postgresql.org/message-id/CAMkU%3D1xAyWnwnLGORBOD%3Dpyv%3DccEkDi%3DwKeyhwF%3DgtB7QxLBwQ%40mail.gmail.com
On Sun, Dec 29, 2019 at 01:15:24PM -0500, Jeff Janes wrote:
> Also, I'd appreciate a report on how many hint-bits were set, and how many
> pages were marked all-visible and/or frozen.  When I do a manual vacuum, it
> is more often for those purposes than it is for removing removable rows
> (which autovac generally does a good enough job of).

The first patch seems simple enough but the 2nd could use critical review.

On Sun, 26 Jan 2020 at 23:13, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> I'm forking this thread since it's separate topic, and since keeping in a
> single branch hasn't made maintaining the patches any easier.
> https://www.postgresql.org/message-id/CAMkU%3D1xAyWnwnLGORBOD%3Dpyv%3DccEkDi%3DwKeyhwF%3DgtB7QxLBwQ%40mail.gmail.com
> On Sun, Dec 29, 2019 at 01:15:24PM -0500, Jeff Janes wrote:
> > Also, I'd appreciate a report on how many hint-bits were set, and how many
> > pages were marked all-visible and/or frozen.  When I do a manual vacuum, it
> > is more often for those purposes than it is for removing removable rows
> > (which autovac generally does a good enough job of).
>
> The first patch seems simple enough but the 2nd could use critical review.

Here is comments on 0001 patch from a quick review:

-   BlockNumber pages_removed;
+   BlockNumber pages_removed;  /* Due to truncation */
+   BlockNumber pages_allvisible;
+   BlockNumber pages_frozen;

Other codes in vacuumlazy.c uses ‘all_frozen', so how about
pages_allfrozen instead of pages_frozen?

---
@@ -1549,8 +1558,12 @@ lazy_scan_heap(Relation onerel, VacuumParams
*params, LVRelStats *vacrelstats,
        {
            uint8       flags = VISIBILITYMAP_ALL_VISIBLE;

-           if (all_frozen)
+           if (all_frozen) {
                flags |= VISIBILITYMAP_ALL_FROZEN;
+               vacrelstats->pages_frozen++;
+           }

@@ -1979,10 +2000,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber
blkno, Buffer buffer,
        uint8       flags = 0;

        /* Set the VM all-frozen bit to flag, if needed */
-       if ((vm_status & VISIBILITYMAP_ALL_VISIBLE) == 0)
+       if ((vm_status & VISIBILITYMAP_ALL_VISIBLE) == 0) {
            flags |= VISIBILITYMAP_ALL_VISIBLE;
-       if ((vm_status & VISIBILITYMAP_ALL_FROZEN) == 0 && all_frozen)
+           vacrelstats->pages_allvisible++;
+       }
+       if ((vm_status & VISIBILITYMAP_ALL_FROZEN) == 0 && all_frozen) {
            flags |= VISIBILITYMAP_ALL_FROZEN;
+           vacrelstats->pages_frozen++;
+       }

The above changes need to follow PostgreSQL code format (a newline is
required after if statement).

---
        /*
         * If the all-visible page is all-frozen but not marked as such yet,
         * mark it as all-frozen.  Note that all_frozen is only valid if
         * all_visible is true, so we must check both.
         */
        else if (all_visible_according_to_vm && all_visible && all_frozen &&
                 !VM_ALL_FROZEN(onerel, blkno, &vmbuffer))
        {
            /*
             * We can pass InvalidTransactionId as the cutoff XID here,
             * because setting the all-frozen bit doesn't cause recovery
             * conflicts.
             */
            visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
                              vmbuffer, InvalidTransactionId,
                              VISIBILITYMAP_ALL_FROZEN);
        }

We should also count up vacrelstats->pages_frozen here.

For 0002 patch, how users will be able to make any meaning out of how
many hint bits were updated by vacuumu?

Regards,

--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Mon, Jun 15, 2020 at 01:30:58PM +0900, Masahiko Sawada wrote:
> For 0002 patch, how users will be able to make any meaning out of how
> many hint bits were updated by vacuum?

The patch has not been updated for the last three months, though it
looks kind of interesting to have more stats for frozen and
all-visible pages around here.

Please note that the patch uses C99-style comments, which is not a
format allowed.  The format of some of the lines coded is incorrect as
well.  I have marked the patch as returned with feedback for now.
--
Michael