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

Re: Map forks (WIP)

From: "Heikki Linnakangas" <heikki(at)enterprisedb(dot)com>
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-patches" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Map forks (WIP)
Date: 2008-05-20 21:05:06
Message-ID: 48333D02.2070907@enterprisedb.com (view raw or flat)
Thread:
Lists: pgsql-patches
Tom Lane wrote:
> One thing I did *not* like was changing the FSM API to refer to Relation
> rather than RelFileNode --- I don't believe that's a good idea at all.
> In particular, consider what happens during TRUNCATE or CLUSTER: it's
> not very clear how you'll tell the versions of the relation apart.
> If you want to push the FSM API up to use SMgrRelation instead of
> RelFileNode, that'd be okay, but not Relation.  (Essentially the
> distinction I'm trying to preserve here is logical vs physical
> relation.)

Oh really? I'm quite fond of the new API. From a philosophical point of
view, in the new world order, the FSM is an integral part of a relation, 
not something tacked on the physical layer. TRUNCATE and CLUSTER will 
need to truncate and truncate+recreate the FSM file, respectively. The 
FSM fork is on an equal footing with the main fork: when TRUNCATE swaps 
the relation file, a new FSM fork is created as well, and there's no way 
or need to access the old file anymore. When a relation is moved to 
another tablespace, the FSM fork is moved as well, and while the 
RelFileNode changes at that point, the logical Relation is the same.

Besides, Relation contains a bunch of very handy fields. pgstat_info in 
particular, which is needed if we want to collect pgstat information 
about FSM, and I think we will. I might also want add a field like 
rd_amcache there, for the FSM: I'm thinking of implementing something 
like the fastroot thing we have in b-tree, and we might need some other 
per-relation information there as well.

> The XLogOpenRelationWithFork stuff needs to be re-thought also,
> as again this is blurring the question of what's a logical and
> what's a physical relation --- and if forknum isn't part of the
> relation ID, that API is wrong either way.  I'm not sure about
> a good solution in this area, but I wonder if the right answer
> might be to make the XLOG replay stuff use SMgrRelations instead
> of bogus Relations.  IIRC the replay code design predates the
> existence of SMgrRelation, so maybe we need a fresh look there.

Agreed, I'm not happy with that part either. I tried to do just what you 
suggest, make XLOG replay stuff deal with SMgrRelations instead of the 
lightweight relcache, and it did look good until I got to refactoring 
btree_xlog_cleanup() (GIN/GiST has the same problem, IIRC). 
btree_xlog_cleanup() uses the same functions as the normal-operation 
code to insert pointers to parent pages, which operates on Relation. 
That started to become really hairy to solve without completely 
bastardizing the normal code paths.

Hmm. One idea would be  to still provide a function to create a fake 
RelationData struct from SMgrRelation, which the redo function can call 
in that kind of situations.

> (On closer look, XLogOpenRelationWithFork seems unused anyway

That's just because FSM WAL-logging hasn't been implemented yet.

> One really trivial thing that grated on me was
> 
> + /*
> +  * In a few places we need to loop through 0..MAX_FORKS to discover which
> +  * forks exists, so we should try to keep this number small.
> +  */
> + #define MAX_FORKS (FSM_FORKNUM + 1)
> 
> I think you should either call it MAX_FORK (equal to the last fork
> number) or NUM_FORKS (equal to last fork number plus one).  As is,
> it's just confusing.  

Agreed, will fix.

> And the comment is flat out wrong for the current usage.

What's described in the comment is done in ATExecSetTableSpace. I grant 
you that there's many other usages for it. I'll work on the comment.

> BTW, it would probably be a good idea to try to get the fork access
> API committed before you work on FSM.  Whenever you can break a
> big patch into successive sections, it's a good idea, IMHO.  I don't
> think there's any doubt that we are going to go in this direction,
> so I see no objection to committing fork-based API revisions in advance
> of having any real use for them.

Yep. I'll develop them together for now, but will separate them when the 
fork stuff is ripe for committing.

-- 
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com


In response to

Responses

pgsql-patches by date

Next:From: Tom LaneDate: 2008-05-20 23:09:37
Subject: Re: Simplify formatting.c
Previous:From: Guillaume LelargeDate: 2008-05-20 20:23:19
Subject: Re: Patch to change psql default banner v6

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