On Sat, 21 Mar 2020 at 16:30, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Mar 20, 2020 at 8:24 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
> >
> > On Fri, Mar 20, 2020 at 04:58:08PM +0530, Amit Kapila wrote:
> > > See, how the attached looks? I have written a commit message as well,
> > > see if I have missed anyone is from the credit list?
> >
> > Thanks for looking again.
> >
> > Couple tweaks:
> >
>
> I have addressed your comments in the attached patch. Today, while
> testing error messages from various phases, I noticed that the patch
> fails to display error context if the error occurs during the truncate
> phase. The reason was that we had popped the error stack in
> lazy_scan_heap due to which it never calls the callback. I think we
> need to set up callback at a higher level as is done in the attached
> patch. I have done the testing by inducing errors in various phases
> and it prints the required information. Let me know what you think of
> the attached?
I've looked at the current version patch.
+/* Phases of vacuum during which we report error context. */
+typedef enum
+{
+ VACUUM_ERRCB_PHASE_UNKNOWN,
+ VACUUM_ERRCB_PHASE_SCAN_HEAP,
+ VACUUM_ERRCB_PHASE_VACUUM_INDEX,
+ VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+ VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
+ VACUUM_ERRCB_PHASE_TRUNCATE
+} ErrCbPhase;
I've already commented on earlier patch but I personally think we'd be
better to report freespace map vacuum as a separate phase. The
progress report of vacuum command is used to know the progress but
this error context would be useful for diagnostic of failure such as
disk corruption. For visibility map, I think the visibility map bit
that are processed during vacuum is exactly corresponding to the block
number but since freespace map vacuum processes the range of blocks
I've sometimes had trouble with identifying the cause of the problem.
What do you think?
Regards,
--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services