Map forks (WIP)
I've been experimenting with different ways to implement the map forks,
for FSM and visibility map. Attached is my first attempt. I've hacked
together an FSM implementation that uses a fork, to have something to
test with, but the FSM implementation is still very much WIP, so please
ignore freespace.c and indexfsm.c for now. The rest of the code is WIP
as well, but it does compile and pass regression tests.
A relation fork is represented by PhysFileNode, which is
RelFileNode+ForkNumber. smgr.c/md.c now deal with PhysFileNodes instead
of RelFileNodes, as well as a bunch of other places related to that.
Each fork gets its own SMgrRelation.
In RelationData, rd_smgr is now an array of SMgrRelations, one per fork.
In bufmgr.c, there's a new variant of ReadBuffer that takes a ForkNumber
as extra argument.
One difficulty is to decide when the extra forks are created and
dropped. The way it works now is that for a heap, all the required forks
(the main fork that contains the actual data and the FSM fork at the
moment) are created in heap_create. In indexes, it's indexam-specific
which forks are required, so only the main fork is created in
heap_create, and it's up to the indexam build-function to create any
additional forks if necessary. There's a few places, like
copy_relation_data, that need to test which forks exist, so I've added a
new smgrexists() function for that.
I was planning to refactor the WAL replay code so that the
redo-functions would deal with SMgrRelations directly, and get rid of
the lightweight relation cache in xlogutils.c. However, it turned out to
be harder than I thought, because we use the same code in the redo
cleanup functions that we do during normal operations. And we also use
the lightweight relcache in BuildFlatFiles().
I'm not completely satisfied with the way this looks, so I'll try a
slightly different approach next: Instead of having one SMgrRelation per
fork, add an extra ForkNumber argument to all the smgr functions.
Any thoughts, ideas?
PS. If you're wondering what's taking me so long: I've been dragged into
other stuff and haven't had much time to work on this :-(.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
Heikki Linnakangas wrote:
I'm not completely satisfied with the way this looks, so I'll try a
slightly different approach next: Instead of having one SMgrRelation per
fork, add an extra ForkNumber argument to all the smgr functions.
Here's an updated patch using the above approach. Looks much better now,
IMO. The FSM implementation is still an incomplete mess, so don't bother
looking into freespace.c or indexfsm.c, but I'm quite happy with the
forks stuff now.
I shall work on the FSM internals next...
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
Attachments:
fsm-forks-3.patch.gzapplication/x-gzip; name=fsm-forks-3.patch.gzDownload+1-2
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
Heikki Linnakangas wrote:
I'm not completely satisfied with the way this looks, so I'll try a
slightly different approach next: Instead of having one SMgrRelation per
fork, add an extra ForkNumber argument to all the smgr functions.
Here's an updated patch using the above approach. Looks much better now,
IMO. The FSM implementation is still an incomplete mess, so don't bother
looking into freespace.c or indexfsm.c, but I'm quite happy with the
forks stuff now.
I took a quick look through this, and concur that this way is better
than one SMgrRelation per fork.
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.)
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.
(On closer look, XLogOpenRelationWithFork seems unused anyway
... maybe this is just a wart left over from the prior version
of the patch? But it seems to me that the replay code is still
doing the wrong thing in using Relation to refer to a physical
rather than logical rel.)
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. And the comment is flat out wrong for the
current usage.
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.
regards, tom lane
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
"Heikki Linnakangas" <heikki@enterprisedb.com> writes:
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.
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.
So? When you have two live versions of a relation, it's still going to
be necessary to track their free state separately.
Besides, Relation contains a bunch of very handy fields.
This just sounds like you're looking for ways to commit layering
violations. The reason we invented SMgrRelation in the first place
was to get the low-level routines out of dealing with Relation, and
I'm not eager to undo that effort.
regards, tom lane
Tom Lane wrote:
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.
I joggled this around for a while, and settled on removing
XLogOpenRelation altogether, and modifying XLogReadBuffer so that it
takes RelFileNode directly as argument. XLogReadBuffer takes care of
calling smgropen and creating missing files, like XLogOpenRelation used
to. This fits nicely with the changes required for relation forks: the
blurring of logical and physical goes away with XLogOpenRelation, as
XLogReadBuffer clearly works at the physical level.
To support functions that really do need a Relation object, like b-tree
cleanup routine which needs to insert a new index pointer to parent
page, which uses the same code that's used during normal operation,
there's a function XLogFakeRelcacheEntry that let's you create a fake
relcache entry just like XLogOpenRelation used to.
There's one ugly modularity violation in the patch: in XLogReadBuffer,
I'm checking "if(smgr->md_fd == NULL)", to avoid calling smgrcreate if a
relation has already been accessed. I'm seeing two possible solutions to
it: push the functionality to smgr.c by adding a new function
smgropenandcreate that opens a relation and creates it if it doesn't
exist, or just document the wart in comments. I'm leaning towards just
documenting it.
Attached is a patch with just these refactorings, no relation fork or
FSM stuff included. I'm planning to commit this first, followed by the
patch to introduce relation forks, followed by FSM rewrite patch. I
think we're almost there with this refactoring patch and relation forks.
FSM still needs a fair amount of closing up loose ends, clean up, and
testing.
Ps. The ReadBuffer interface is getting out of hand. This patch
introduces one more ReadBuffer variant, ReadBufferWithoutRelcache, and
the upcoming relation forks patch will again introduce at least one new
variant.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com