Re: Non-transactional pg_class, try 2 - Mailing list pgsql-patches

Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

> Would it work to do
>
> #define tid    ItemPointerData
> ...
>     tid    relntrans;
> ...
> #undef tid
> ?

Yeah, it probably would.  I'll try.

> The *real* problem with what you've done is that pg_class.h now depends
> on having ItemPointerData defined before it's included, which creates an
> inclusion ordering dependency that should not exist.  If you stick with
> either of these approaches then pg_class.h needs to #include whatever
> defines ItemPointerData.

storage/itemptr.h is #included in pg_class.h (first chunk of the patch).


> > Other two caveats are:
> > 1. During bootstrap, RelationBuildLocalRelation creates nailed relations
> > with hardcoded TID=(0,1).  This is because we don't have access to
> > pg_class yet, so we can't find the real pointer; and furthermore, we are
> > going to fix the entries later in the bootstrapping process.
>
> This seems dangerous; can't you set it to InvalidItemPointer instead?
> If it's not used before fixed, this doesn't matter, and if someone
> *does* try to use it, that will catch the problem.

Doesn't work because the bootstrap system actually _writes_ there :-(  A
workaround could be to disable writing in bootstrapping mode, and store
InvalidItemPointer.  (Actually storing InvalidItemPointer was the first
thing I did, but it crashed on bootstrap.)

I wasn't worried about bootstrap writing invalid values, because the
correct values are written shortly after (at the latest, when vacuum is
run by initdb).  And if I set it to Invalid and have bootstrap mode skip
writing, exactly the same thing will happen ...

> > 2. The whole VACUUM/VACUUM FULL/ANALYZE relation list stuff is pretty
> > ugly as well; and autovacuum is skipping pg_ntclass (really all
> > non-transactional catalogs) altogether.  We could improve the situation
> > by introducing some sort of struct like {relid, relkind}, so that
> > vacuum_rel could know what relkind to expect, and it could skip
> > non-transactional catalogs cleanly in vacuum full and analyze.
>
> Need to do something about this.  pg_ntclass will contain XIDs (of
> inserting/deleting transactions) so it MUST get vacuumed to be sure
> we don't expose ourselves to XID wrap problems.

Oh, certainly it does get vacuumed.  vacuum.c is modified so that
non-transactional catalogs are vacuumed (in database-wide VACUUM for
instance).  The only thing I was stating above was that the way vacuum.c
handles the list of relations, is a bit of a mess, because vacuum_rel
wants to check the relkind but get_oids_list forms only a single list
and it's assumed that they are all RELKIND_RELATION rels.  I had to
modify it a bit so that NON_TRANSACTIONAL rels are also included in that
list, and therefore the check had to be relaxed.

I also made ANALYZE silently skip non-transactional catalogs, in an
similarly ugly way.  I don't remember what was the rationale for this --
certainly there isn't any harm.  But on the other hand, analyzing it
serves no purpose since the statistics are not used for anything.