On Tue, Mar 17, 2020 at 11:51 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Mar 17, 2020 at 9:52 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > Another minor point, don't we need to remove the error stack by doing > > "error_context_stack = errcallback.previous;" in parallel_vacuum_main? > > > > Few other comments: > 1. The error in lazy_vacuum_heap can either have phase > VACUUM_ERRCB_PHASE_INDEX_* or VACUUM_ERRCB_PHASE_VACUUM_HEAP depending > on when it occurs. If it occurs the first time it enters that > function before a call to lazy_vacuum_page, it will use phase > VACUUM_ERRCB_PHASE_INDEX_*, otherwise, it would use > VACUUM_ERRCB_PHASE_VACUUM_HEAP. The reason is lazy_vacuum_index or > lazy_cleanup_index won't reset the phase after leaving that function. > > 2. Also once we set phase as VACUUM_ERRCB_PHASE_VACUUM_HEAP via > lazy_vacuum_page, it doesn't seem to be reset to > VACUUM_ERRCB_PHASE_SCAN_HEAP even when we do scanning of the heap. I > think you need to set phase VACUUM_ERRCB_PHASE_SCAN_HEAP inside loop. > > I think we need to be a bit more careful in setting/resetting the > phase information correctly so that it doesn't display the wrong info > in the context in an error message. >
Justin, are you planning to work on the pending comments? If you want, I can try to fix some of these. We have less time left for this CF, so we need to do things a bit quicker. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
