Re: CLUSTER patch

Alvaro Herrera wrote:
> 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.

I sent a new version of your patch out that got farther, and it does
have those swap lines.

> 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);

Yes, that is required.

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

Yes, the problem here is that dirty buffers can't look up the proper
relation to figure out how to complete the transaction.  Reloading the
cache after the swap seems to fix it, but the invalidation itself throws
WARNINGS because the relfilenode's of the old and new relations
structures don't match, and when it tries to delete the first one, it
throws the warning.

> (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.

Yep, I think everything now points to changes needed in relcache.c to
handle the new condition of a relation swapping its relnode.  I looked
at the REINDEX code and it doesn't seem to have this problem.  Not sure
why.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026