On Thu, Mar 26, 2020 at 09:50:53AM +0530, Amit Kapila wrote: > > > after count_nondeletable_pages, and then revert it back to > > > VACUUM_ERRCB_PHASE_SCAN_HEAP phase and the number of blocks of > > > relation before truncation, after RelationTruncate(). It can be > > > repeated until no more truncating can be done. Why do we need to > > > revert back to the scan heap phase? If we can use > > > vacrelstats->nonempty_pages in the error context message as the > > > remaining blocks after truncation I think we can update callback > > > arguments once at the beginning of lazy_truncate_heap() and don't > > > revert to the previous phase, and pop the error context after exiting. > > > > Perhaps. We need to "revert back" for the vacuum phases, which can be > > called > > multiple times, but we don't need to do that here. > > Yeah, but I think it would be better if are consistent because we have > no control what the caller of the function intends to do after > finishing the current phase. I think we can add some comments where > we set up the context (in heap_vacuum_rel) like below so that the idea > is more clear. > > "The idea is to set up an error context callback to display additional > information with any error during vacuum. During different phases of > vacuum (heap scan, heap vacuum, index vacuum, index clean up, heap > truncate), we update the error context callback to display appropriate > information. > > Note that different phases of vacuum overlap with each other, so once > a particular phase is over, we need to revert back to the old phase to > keep the phase information up-to-date."
Seems fine. Rather than saying "different phases" I, would say: "The index vacuum and heap vacuum phases may be called multiple times in the middle of the heap scan phase." But actually I think the concern is not that we unnecessarily "Revert back to the old phase" but that we do it in a *loop*. Which I agree doesn't make sense, to go back and forth between "scanning heap" and "truncating". So I think we should either remove the "revert back", or otherwise put it after/outside the "while" loop, and change the "return" paths to use "break". -- Justin
