Allow cancelling VACUUM of nbtrees with corrupted right links - Mailing list pgsql-hackers

From Andres Freund
Subject Allow cancelling VACUUM of nbtrees with corrupted right links
Date
Msg-id 20180627191629.wkunw2qbibnvlz53@alap3.anarazel.de
Whole thread Raw
Responses Re: Allow cancelling VACUUM of nbtrees with corrupted right links
Re: Allow cancelling VACUUM of nbtrees with corrupted right links
List pgsql-hackers
Hi,

A couple times, one very recently, I've encountered btrees that somehow
had corrupted right links. The links formed a cycle, involving a number
of pages.  As of yet it's unclear to me where the corruption is
originating from - could be a storage issue or a postgres issue.


What makes that kind of corruption annoying is not so much lookups or
insertsion not working, but that it can lead to VACUUM being stuck. Page
deletion codepaths have to follow right links, and if there's a cycle
they'll do so forever. That'd be bad enough, but there's no
CHECK_FOR_INTERRUPTS() in those codepaths, which means autovacuum can't
be cancelled. And thus the index can't easily be dropped / reindexed.

In an older case that lead to significant difficulty for the user to
ever get out of the situation, because even after a shutdown autovacuum
quickly latched onto the table, IIRC due to an impeding wraparound.

I think it'd be a good minimal fix if we added a bunch of CFIs to the
likely instances of such loops. I've done that in the attached patch.
Unfortunately it's entirely trivial, because CFI will not trigger when
an lwlock is held (as LWLockAcquire() does a HOLD_INTERRUPTS()).  Any
comments about the patch?


I couldn't see how to fix the _bt_unlink_halfdead_page() right-sib loop,
because we always hold a lock. But given that that loop appears to
mostly be dead code, that doesn't seem too bad?


I think we should backpatch those checks - it's a fairly nasty situation
for users to not be able to even drop an index without going to single
user mode.


I wonder if we additionally should put a CFI() in _bt_relandgetbuf(), as
it's otherwise impossible to check interrupts at the callsites.


Alternatively we could also invent a CFI version that allows
cancellation even if locks held - but that seems nontrivial.

Greetings,

Andres Freund

Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: [HACKERS] Support for Secure Transport SSL library on macOS asOpenSSL alternative
Next
From: Alvaro Herrera
Date:
Subject: Re: Remove mention in docs that foreign keys on partitioned tablesare not supported