Re: pgsql: Skip full index scan during cleanup of B-tree indexes when possi - Mailing list pgsql-committers

From Alexander Korotkov
Subject Re: pgsql: Skip full index scan during cleanup of B-tree indexes when possi
Date
Msg-id CAPpHfduX1CmUbC9i=0fOORAcExJ0bdFqdvPTZ5v4=0vMM7DY4g@mail.gmail.com
Whole thread Raw
In response to Re: pgsql: Skip full index scan during cleanup of B-tree indexes when possi  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
Responses Re: pgsql: Skip full index scan during cleanup of B-tree indexes when possi  (Alexander Korotkov <a.korotkov@postgrespro.ru>)
List pgsql-committers
On Thu, Apr 5, 2018 at 2:26 PM, Alexander Korotkov <a.korotkov@postgrespro.ru> wrote:
On Thu, Apr 5, 2018 at 6:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Peter Geoghegan <pg@bowt.ie> writes:
>>> TRAP: FailedAssertion("!(metad->btm_version == 3)", File:
>>> "/home/pg/postgresql/root/build/../source/src/backend/access/nbtree/nbtpage.c",
>>> Line: 619)

>> Hm, buildfarm's not complaining --- what's the test case?

> This was discovered while testing/reviewing the latest version of the
> INCLUDE covering indexes patch. It now seems to be unrelated.

Oh, wait ... I wonder if you saw that because you were running a new
backend without having re-initdb'd?  Once you had re-initdb'd, then
of course there would be no old-format btree indexes anywhere.  But
if you hadn't, then anyplace that was not prepared to cope with the
old header format would complain about pre-existing indexes.

In short, this sounds like a place that did not get the memo about
how to cope with un-upgraded indexes.

That's an issue, because meta-page should be upgraded "on the fly".
That was tested, but perhaps without assertions.  I'll investigate more on
this an propose a fix.

So, "Assert(metad->btm_version == BTREE_VERSION)" was failed, because
metadata was copied from meta page to rel->rd_amcache "as is" with
metad->btm_version == BTREE_MIN_VERSION.

Without assert everything works fine, because extended metadata fields are
never used from rel->rd_amcache.  So my first intention was to relax this
assert to Assert(metad->btm_version >= BTREE_MIN_VERSION && metad->btm_version <= BTREE_VERSION).
But then I still have concern that we copy metadata beyond pd_lower
when metapage is in old format.

memcpy(rel->rd_amcache, metad, sizeof(BTMetaPageData));

This is why I decided to write separate function to handle caching of metadata,
which takes care about filling unavailable fields of metadata with default values.
I also made same fix for pageinspect.  Patch is attached.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 
Attachment

pgsql-committers by date:

Previous
From: Simon Riggs
Date:
Subject: pgsql: MERGE minor errata
Next
From: Alexander Korotkov
Date:
Subject: Re: pgsql: Skip full index scan during cleanup of B-tree indexes when possi