On Thu, Nov 10, 2022 at 10:20 AM Imseih (AWS), Sami <sims...@amazon.com> wrote: > 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.
Yeah, it's a little inconsistent. > > 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. I meant that there should definitely be a check between each round of index scans (one index scan here affects each and every index). Doing more than a single index scan is presumably rare, but are likely common among VACUUM operations that take an unusually long time -- which is where the failsafe is relevant. I'm just highlighting that multiple index scans (rather than just 0 or 1 index scans) is by far the primary risk factor that leads to a VACUUM that takes way longer than is typical. (The other notable risk comes from aggressive VACUUMs that freeze a great deal of heap pages all at once, which I'm currently addressing by getting rid of the whole concept of discrete aggressive mode VACUUM operations.) > > 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. Want to have a go at writing a patch for that? -- Peter Geoghegan
