On Sat, 25 Jul 2020 at 02:49, Mahendra Singh Thalor <[email protected]> wrote:
>
> Hi hackers,
> We discussed in another email thread[1], that it will be helpful if we can
> display offset along with block number in vacuum error. Here, proposing a
> patch to add offset along with block number in vacuum errors.
>
> In commit b61d161(Introduce vacuum errcontext to display additional
> information), we added vacuum errcontext to display additional
> information(block number) so that in case of vacuum error, we can identify
> which block we are getting error. Addition to block number, if we can
> display offset, then it will be more helpful for users. So to display offset,
> here proposing two different methods(Thanks Robert for suggesting these 2
> methods):
>
> Method 1: We can report the TID as well as the block number in errcontext.
> - errcontext("while scanning block %u of relation \"%s.%s\"",
> - errinfo->blkno, errinfo->relnamespace, errinfo->relname);
> + errcontext("while scanning block %u and offset %u of relation \"%s.%s\"",
> + errinfo->blkno, errinfo->offnum, errinfo->relnamespace, errinfo->relname);
>
> Above fix requires more calls to update_vacuum_error_info(). Attaching
> v01_0001 patch for this method.
>
> Method 2: We can improve the error messages by passing the relevant TID to
> heap_prepare_freeze_tuple and having it report the TID as part of the error
> message or in the error detail.
> ereport(ERROR,
> (errcode(ERRCODE_DATA_CORRUPTED),
> - errmsg_internal("found xmin %u from before relfrozenxid %u",
> + errmsg_internal("for block %u and offnum %u, found xmin %u from before
> relfrozenxid %u",
> + ItemPointerGetBlockNumber(tid),
> + ItemPointerGetOffsetNumber(tid),
> xid, relfrozenxid)));
>
> Attaching v01_0002 patch for this method.
>
> Please let me know your thoughts.
>
+1 for adding offset in error messages.
I had a look at 0001 patch. You've set the vacuum error info but I
think an error won't happen during setting itemids unused:
@@ -1924,14 +1932,22 @@ lazy_vacuum_page(Relation onerel, BlockNumber
blkno, Buffer buffer,
BlockNumber tblk;
OffsetNumber toff;
ItemId itemid;
+ LVSavedErrInfo loc_saved_err_info;
tblk =
ItemPointerGetBlockNumber(&dead_tuples->itemptrs[tupindex]);
if (tblk != blkno)
break; /* past end of
tuples for this block */
toff =
ItemPointerGetOffsetNumber(&dead_tuples->itemptrs[tupindex]);
+
+ /* Update error traceback information */
+ update_vacuum_error_info(vacrelstats,
&loc_saved_err_info, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+ blkno, toff);
itemid = PageGetItemId(page, toff);
ItemIdSetUnused(itemid);
unused[uncnt++] = toff;
+
+ /* Revert to the previous phase information for error
traceback */
+ restore_vacuum_error_info(vacrelstats, &loc_saved_err_info);
}
PageRepairFragmentation(page);
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
