Re: vacuum verbose: show pages marked allvisible/frozen/hintbits

2020-09-06 Thread Michael Paquier
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


signature.asc
Description: PGP signature


Re: vacuum verbose: show pages marked allvisible/frozen/hintbits

2020-06-14 Thread Masahiko Sawada
On Sun, 26 Jan 2020 at 23:13, Justin Pryzby  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, ))
{
/*
 * 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 Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services




vacuum verbose: show pages marked allvisible/frozen/hintbits

2020-01-26 Thread Justin Pryzby
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.
>From 57eede7d1158904d6b66532c7d0ce6a59803210f Mon Sep 17 00:00:00 2001
From: Justin Pryzby 
Date: Sun, 29 Dec 2019 14:56:02 -0600
Subject: [PATCH v1 1/2] Report number of pages marked allvisible/frozen..

..as requested by Jeff Janes
---
 src/backend/access/heap/vacuumlazy.c | 37 ++--
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8ce5011..9975699 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -283,7 +283,9 @@ typedef struct LVRelStats
 	double		new_rel_tuples; /* new estimated total # of tuples */
 	double		new_live_tuples;	/* new estimated total # of live tuples */
 	double		new_dead_tuples;	/* new estimated total # of dead tuples */
-	BlockNumber pages_removed;
+	BlockNumber pages_removed;	/* Due to truncation */
+	BlockNumber pages_allvisible;
+	BlockNumber pages_frozen;
 	double		tuples_deleted;
 	BlockNumber nonempty_pages; /* actually, last nonempty page + 1 */
 	LVDeadTuples *dead_tuples;
@@ -602,11 +604,13 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 			 get_namespace_name(RelationGetNamespace(onerel)),
 			 RelationGetRelationName(onerel),
 			 vacrelstats->num_index_scans);
-			appendStringInfo(, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"),
+			appendStringInfo(, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen, %u marked all visible, %u marked frozen\n"),
 			 vacrelstats->pages_removed,
 			 vacrelstats->rel_pages,
 			 vacrelstats->pinskipped_pages,
-			 vacrelstats->frozenskipped_pages);
+			 vacrelstats->frozenskipped_pages,
+			 vacrelstats->pages_allvisible,
+			 vacrelstats->pages_frozen);
 			appendStringInfo(,
 			 _("tuples: %.0f removed, %.0f remain, %.0f are dead but not yet removable, oldest xmin: %u\n"),
 			 vacrelstats->tuples_deleted,
@@ -751,6 +755,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	vacrelstats->scanned_pages = 0;
 	vacrelstats->tupcount_pages = 0;
 	vacrelstats->nonempty_pages = 0;
+	vacrelstats->pages_allvisible = 0;
+	vacrelstats->pages_frozen = 0;
+
 	vacrelstats->latestRemovedXid = InvalidTransactionId;
 
 	/*
@@ -1170,6 +1177,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 visibilitymap_set(onerel, blkno, buf, InvalidXLogRecPtr,
   vmbuffer, InvalidTransactionId,
   VISIBILITYMAP_ALL_VISIBLE | VISIBILITYMAP_ALL_FROZEN);
+vacrelstats->pages_allvisible++;
+vacrelstats->pages_frozen++;
 END_CRIT_SECTION();
 			}
 
@@ -1501,8 +1510,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++;
+			}
+
+			vacrelstats->pages_allvisible++;
 
 			/*
 			 * It should never be the case that the visibility map page is set
@@ -1690,6 +1703,14 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	"%u pages are entirely empty.\n",
 	empty_pages),
 	 empty_pages);
+	appendStringInfo(, ngettext("Marked %u page all visible, ",
+	"Marked %u pages all visible, ",
+	vacrelstats->pages_allvisible),
+	vacrelstats->pages_allvisible);
+	appendStringInfo(, ngettext("%u page frozen.\n",
+	"%u pages frozen.\n",
+	vacrelstats->pages_frozen),
+	vacrelstats->pages_frozen);
 	appendStringInfo(, _("%s."), pg_rusage_show());
 
 	ereport(elevel,
@@ -1912,10 +1933,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++;
+		}
 
 		Assert(BufferIsValid(*vmbuffer));
 		if