From: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: All-zero page in GIN index causes assertion failure |
Date: | 2015-07-20 20:23:41 |
Message-ID: | 55AD58CD.2000607@iki.fi |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 07/20/2015 07:11 PM, Alvaro Herrera wrote:
> Heikki Linnakangas wrote:
>
>> Looking closer, heap vacuum does a similar thing, but there are two
>> mitigating factors that make it safe in practice, I think:
>>
>> 1. An empty heap page is all-zeroes except for the small page header in the
>> beginning of the page. For a torn page to matter, the page would need to be
>> torn in the header, but we rely elsewhere (control file) anyway that a
>> 512-byte sector update is atomic, so that shouldn't happen. Note that this
>> hinges on the fact that there is no special area on heap pages, so you
>> cannot rely on this for index pages.
>>
>> 2. The redo of the first insert/update on a heap page will always
>> re-initialize the page, even when full-page-writes are disabled. This is the
>> XLOG_HEAP_INIT_PAGE optimization. So it's not just an optimization, it's
>> required for correctness.
>
> Hmm, I guess this is a case of an optimization hiding a bug :-( I mean,
> if we didn't have that, we would have noticed this a lot sooner, I
> imagine.
Yeah, probably.
I came up with the attached, for GIN and SP-GiST. Instead of WAL-logging
the page initialization in SP-GiST vacuum, I changed it so that it
simply leaves the page as all-zeroes, and adds it to the FSM. The code
that grabs a target page from the FSM handles that, and initializes the
page anyway, so that was simpler. This made the SP-GiST is-deleted flag
obsolete, it's no longer set, although the code still checks for it for
backwards-compatibility. (even that may actually be unnecessary, as a
page that's marked as deleted must also be empty, and wherever we check
for the deleted-flag, we also check for PageIsEmpty()))
- Heikki
Attachment | Content-Type | Size |
---|---|---|
fix-gin-spgist-vacuum-1.patch | application/x-patch | 2.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andrew Dunstan | 2015-07-20 20:51:13 | "make check" changes have caused buildfarm deterioration. |
Previous Message | Merlin Moncure | 2015-07-20 18:28:55 | Re: First Aggregate Funtion? |