> It makes sense to prefer consistency here, I suppose. The reason why > we're not consistent is because it was easier not to be, which isn't > exactly the best reason (nor the worst).
Consistency is the key point here. It is odd that a serial vacuum may skip the remainder of the indexes if failsafe kicks-in, but in the parallel case it will go through the entire index cycle. > My gut instinct is that the most important thing is to at least call > lazy_check_wraparound_failsafe() once per index scan. I agree. And this should happen in the serial and parallel case. > That said, one thing that does bother me in this area occurs to me: we > call lazy_check_wraparound_failsafe() from lazy_scan_heap() (before we > get to the index scans that you're talking about) at an interval that > is based on how many heap pages we've either processed (and recorded > as a scanned_pages page) *or* have skipped over using the visibility > map. In other words we use blkno here, when we should really be using > scanned_pages instead: > if (blkno - next_failsafe_block >= FAILSAFE_EVERY_PAGES) > { > lazy_check_wraparound_failsafe(vacrel); > next_failsafe_block = blkno; > } > This code effectively treats pages skipped using the visibility map as > equivalent to pages physically scanned (scanned_pages), even though > skipping requires essentially no work at all. That just isn't logical, > and feels like something worth fixing. The fundamental unit of work in > lazy_scan_heap() is a *scanned* heap page. It makes perfect sense to use the scanned_pages instead. Regards, Sami imseih Amazon Web Services (AWS)