pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Justin Pryzby <pryzby@telsasoft.com>
To: Mahendra Singh Thalor <mahi6run@gmail.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: display offset along with block number in vacuum errors
Date: Tue, 28 Jul 2020 14:39:19 -0500
Message-ID: <20200728193919.GK20393@telsasoft.com> (raw)
In-Reply-To: <CAKYtNApLJjAaRw0UEBBY6G1o0LRZKS7rA5n46BFh+NfwSOycdg@mail.gmail.com>
References: <CAKYtNApK488TDF4bMbw+1QH8HJf9cxdNDXquhU50TK5iv_FtCQ@mail.gmail.com>
	<20200727111352.GN4286@telsasoft.com>
	<CAKYtNApLJjAaRw0UEBBY6G1o0LRZKS7rA5n46BFh+NfwSOycdg@mail.gmail.com>

On Wed, Jul 29, 2020 at 12:35:17AM +0530, Mahendra Singh Thalor wrote:
> Apart from these, I fixed comments given by Sawada and Michael in the
> latest patch. Attaching v2 patch for review.

Thanks.

lazy_check_needs_freeze iterates over blocks and this patch changes it to
update vacrelstats.  I think it should do what
lazy_{vacuum/cleanup}_heap/page/index do and call update_vacuum_error_info() at
its beginning (even though only the offset is changed), and then
restore_vacuum_error_info() at its end (to "revert back" to the item number it
started with).

The same is true of heap_page_is_all_visible(), except it's only called by
lazy_vacuum_page(), which already calls restore_vacuum_error_info() a few lines
later.

As for the message:

+                               if (OffsetNumberIsValid(errinfo->offnum))
+                                       errcontext("while scanning block %u and offset %u of relation \"%s.%s\"",
+                                                          errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
+                               else
+                                       errcontext("while scanning block %u of relation \"%s.%s\"",
+                                                          errinfo->blkno, errinfo->relnamespace, errinfo->relname);

I think that may be confusing.  A DBA should know what a "block" is, but
"offset" sounds like a byte offset, rather than an item number.  Here's what
I'd written.  Maybe it should say "offset number".

+                               errcontext("while vacuuming block %u of relation \"%s.%s\", item offset %u",


-- 
Justin





view thread (39+ messages)  latest in thread

Message-ID: <20200728193919.GK20393@telsasoft.com>
Permalink:  ../20200728193919.GK20393@telsasoft.com/
Also on:    postgresql.org/message-id/20200728193919.GK20393@telsasoft.com

 · 

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: pgsql-hackers@postgresql.org
  Cc: pryzby@telsasoft.com, mahi6run@gmail.com
  Subject: Re: display offset along with block number in vacuum errors
  In-Reply-To: <20200728193919.GK20393@telsasoft.com>

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

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