Re: GiST insert algorithm rewrite

On 30.11.2010 16:23, Robert Haas wrote:
> On Tue, Nov 30, 2010 at 5:02 AM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>> On 30.11.2010 11:55, Heikki Linnakangas wrote:
>>>
>>> On 27.11.2010 21:31, Bruce Momjian wrote:
>>>>
>>>> Heikki Linnakangas wrote:
>>>>>
>>>>> There's no on-disk format changes, except for the additional flag in the
>>>>> page headers, so this does not affect pg_upgrade. However, if there's
>>>>> any "invalid" keys in the old index because of an incomplete insertion,
>>>>> the new code will not understand that. So you should run vacuum to
>>>>> ensure that there's no such invalid keys in the index before upgrading.
>>>>> Vacuum will print a message in the log if it finds any, and you will
>>>>> have to reindex. But that's what it suggests you to do anyway.
>>>>
>>>> OK, pg_upgrade has code to report invalid gin and hash indexes because
>>>> of changes between PG 8.3 and 8.4. Is this something we would do for
>>>> 9.0 to 9.1?
>>>
>>> 9.1. The problem that started this whole thing is there in older
>>> versions, but given the lack of real-life reports and the scale of the
>>> changes required it doesn't seem wise to backport.
>>
>> Oh sorry, I read your question as "9.0 *or* 9.1".
>>
>> Only GiST indexes that have any "invalid" tuples in them n, as a result of a
>> crash, need to be reindexed. That's very rare in practice, so we shouldn't
>> invalidate all GiST indexes. I don't think there's any simple way to check
>> whether reindex is required, so I think we have to just document this.
>
> It seems odd to say, the indexes are corrupted, but they're probably
> not, so let's not worry about it.
>
> I assume there's no way to make the new code cope with any
> pre-existing corruption?
>
> Does the current code cope with the corruption?

It's not corruption, but "intended degradation". Yes, the current code 
copes with it, that's how GiST survives a crash. However, even with the 
current code, VACUUM will nag if it finds any invalid tuples with this 
message:

ereport(NOTICE,(errmsg("index \"%s\" needs VACUUM FULL or REINDEX to finish crash 
recovery",

That's harmless, in the sense that all scans and inserts work fine, but 
scans might need to do more work than if the invalid tuple wasn't there.

I don't think we need to go out of our way to support such degraded 
indexes in 9.1. If you see such notices in your logs, you should REINDEX 
anyway, before of after pg_upgrade. Let's just make sure that you get a 
reasonable error message in 9.1 if a scan or insert encounters such a tuple.

There is a section on this in the docs, BTW: 
http://www.postgresql.org/docs/9.0/static/gist-recovery.html

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com