Skip site navigation (1) Skip section navigation (2)

Re: Non-transactional pg_class, try 2

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Non-transactional pg_class, try 2
Date: 2006-06-12 01:26:19
Message-ID: 20060612012619.GB25809@alvh.no-ip.org (view raw or flat)
Thread:
Lists: pgsql-hackerspgsql-patches
Tom Lane wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)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.

In response to

Responses

pgsql-hackers by date

Next:From: Tom LaneDate: 2006-06-12 01:31:32
Subject: Re: Non-transactional pg_class, try 2
Previous:From: Tom LaneDate: 2006-06-12 01:21:16
Subject: Re: The corresponding relminxid patch; try 1

pgsql-patches by date

Next:From: Tom LaneDate: 2006-06-12 01:31:32
Subject: Re: Non-transactional pg_class, try 2
Previous:From: Tom LaneDate: 2006-06-12 01:21:16
Subject: Re: The corresponding relminxid patch; try 1

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group