Re: CLUSTER patch

Tom Lane dijo:

> Alvaro Herrera <alvherre(at)atentus(dot)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"