Re: CLUSTER patch - Mailing list pgsql-patches

From Alvaro Herrera
Subject Re: CLUSTER patch
Date
Msg-id Pine.LNX.4.44.0207141310060.17260-100000@cm-lcon1-46-187.cm.vtr.net
Whole thread Raw
In response to Re: CLUSTER patch  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: CLUSTER patch  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-patches
Tom Lane dijo:

> Alvaro Herrera <alvherre@atentus.com> writes:
> > The way the cluster is done is still the same: first cluster the heap
> > and swap relfilenodes, then drop old heap; then rebuild each index, swap
> > relfilenodes with old index and drop new.
>
> I do not see anything in this patch that touches relfilenode.  Perhaps
> the patch is incomplete?

No, this is the old version, corrected according your comments, for
inclusion in case the new version doesn't make it.

Perhaps you missed the patch I posted some days ago that did swap
relfilenodes (there was even a function named swap_relfilenode, so it
wasn't easy to miss).  It's archived in
http://archives.postgresql.org/pgsql-patches/2002-07/msg00079.php.  The
code in question is this:

    tempRFNode = ((Form_pg_class) GETSTRUCT(reltup[0]))->relfilenode;
    ((Form_pg_class) GETSTRUCT(reltup[0]))->relfilenode =
        ((Form_pg_class) GETSTRUCT(reltup[1]))->relfilenode;
    ((Form_pg_class) GETSTRUCT(reltup[1]))->relfilenode = tempRFNode;

    /* Update the RelationRelationName tuples */
    simple_heap_update(relRelation, &reltup[1]->t_self, reltup[1]);
    simple_heap_update(relRelation, &reltup[0]->t_self, reltup[0]);

Now, if I do CommandCounterIncrement() right after this, I get "Relation
deleted while still in use".  So I add

    CatalogOpenIndices(Num_pg_class_indices, Name_pg_class_indices, irels);
    CatalogIndexInsert(irels, Num_pg_class_indices, relRelation, reltup[1]);
    CatalogIndexInsert(irels, Num_pg_class_indices, relRelation, reltup[0]);
    CatalogCloseIndices(Num_pg_class_indices, irels);

and only then the CommandCounterIncrement.  But the server says
TRAP: Failed Assertion("!(bufrel != ((void *)0)):", File: "localbuf.c",
Line: 242)

(I think it's line 241 in pristine source).  I tried doing
CatalogIndexInsert and CommandCounterIncrement after each
simple_heap_update, but the result is the same.  This assertion is
failed at transaction commit, when LocalBufferSync is called.

> > But as soon as I try to do anything to it (the new,
> > clustered filenode) before transaction commit (building an index, say),
> > the local buffer manager fails an assertion (actually,
> > RelationNodeCacheGetRelation returns 0 for the given rnode), and the
> > transaction aborts.
>
> Hmm.  If you do the swap in the correct way (viz, update the relation's
> pg_class entry and then CommandCounterIncrement) I'd expect the relcache
> to respond correctly.  This does involve re-indexing the relcache entry
> under a new relfilenode value, but that's not significantly different
> from the case of renaming a relation.

That's what I think I'm doing, but I don't know what's the relcache
doing; other than I expect, I pressume.  I tried using
RelationForgetRelation on both relations (following Bruce's idea of
invalidating the relcache entries), but I get lost in the relcache, so
I'm now in the dark.

--
Alvaro Herrera (<alvherre[a]atentus.com>)
"The ability to monopolize a planet is insignificant
next to the power of the source"



pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: CLUSTER patch
Next
From: Tom Lane
Date:
Subject: Re: CLUSTER patch