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