Horton
Menu

Re: New IndexAM API controlling index vacuum strategies

Peter Geoghegan <pg@bowt.ie>

View thread
From: Peter Geoghegan <pg@bowt.ie>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Noah Misch <noah@leadboat.com>
Date: 2021-03-15T20:58:02Z
Lists: pgsql-hackers 
On Mon, Mar 15, 2021 at 12:58 PM Peter Geoghegan <pg@bowt.ie> wrote:
> > I'm not comfortable with this change without adding more safety
> > checks. If there's ever a case in which the HEAPTUPLE_DEAD case is hit
> > and the xid needs to be frozen, we'll either cause errors or
> > corruption. Yes, that's already the case with params->index_cleanup ==
> > DISABLED, but that's not that widely used.
>
> I noticed that Noah's similar 2013 patch [1] added a defensive
> heap_tuple_needs_freeze() + elog(ERROR) to the HEAPTUPLE_DEAD case. I
> suppose that that's roughly what you have in mind here?

I'm not sure if you're arguing that there might be (either now or in
the future) a legitimate case (a case not involving data corruption)
where we hit HEAPTUPLE_DEAD, and find we have an XID in the tuple that
needs freezing. You seem to be suggesting that even throwing an error
might not be acceptable, but what better alternative is there? Did you
just mean that we should throw a *better*, more specific error right
there, when we handle HEAPTUPLE_DEAD? (As opposed to relying on
heap_prepare_freeze_tuple() to error out instead, which is what would
happen today.)

That seems like the most reasonable interpretation of your words to
me. That is, I think that you're saying (based in part on remarks on
that other thread [1]) that you believe that fully eliminating the
"tupgone = true" special case is okay in principle, but that more
hardening is needed -- if it ever breaks we want to hear about it.
And, while it would be better to do a much broader refactor to unite
heap_prune_chain() and lazy_scan_heap(), it is not essential (because
the issue is not really new, even without VACUUM (INDEX_CLEANUP
OFF)/"params->index_cleanup == DISABLED").

Which is it?

[1] https://www.postgresql.org/message-id/20200724165514.dnu5hr4vvgkssf5p%40alap3.anarazel.de
-- 
Peter Geoghegan

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Don't truncate heap when VACUUM's failsafe is in effect.

  2. Teach VACUUM to bypass unnecessary index vacuuming.

  3. Add wraparound failsafe to VACUUM.

  4. Truncate line pointer array during VACUUM.

  5. Remove tupgone special case from vacuumlazy.c.

  6. Refactor lazy_scan_heap() loop.

  7. Propagate parallel VACUUM's buffer access strategy.

  8. Simplify state managed by VACUUM.

  9. Notice that heap page has dead items during VACUUM.

  10. Adjust lazy_scan_heap() accounting comments.

  11. Use full 64-bit XID for checking if a deleted GiST page is old enough.

  12. Fix some problems with VACUUM (INDEX_CLEANUP FALSE).