small smgrcreate cleanup patch

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: small smgrcreate cleanup patch
Date: 2010-08-20 02:43:54
Message-ID: AANLkTin2sHEVd5+M9dzJN4T2tLz=swUZ0bJgFYV08U60@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

smgrcreate() currently contains a call to TablespaceCreateDbspace().
As the comment says, this is a rather silly place to put it. The
silliest thing about it, IMHO, is that it forces the following check
to be done in both smgrcreate() and mdcreate():

if (isRedo && reln->md_fd[forknum] != NULL)
return;

So I propose moving the TablespaceCreateDbspace() call to mdcreate(),
removing the redundant check from smgrcreate(), and deleting that
portion of the comment which says this is in the wrong place. You
could argue that perhaps md.c isn't the right place either, but it
certainly makes more sense than smgr.c, and I'd argue it's exactly
right. Moving the TablespaceCreateDbspace() logic into md.c (or
smgr.c) seems like it would be more awkward, though I suppose it could
be done. One less-than-thrilling result would be that the
TablespaceCreateLock logic wouldn't be confined to tablespace.c, not
that that's *necessarily* a disaster.

A related, interesting question is whether there's any purpose to the
smgr layer at all. Would we accept a patch that implemented an
alternative smgr layer, perhaps on a per-tablespace basis? The only
real alternative I can imagine to "magnetic disk" storage would be
network storage. You could imagine using such a thing for temporary
tablespaces, essentially writing out temporary table pages to a RAM
cache on a remote machine; or perhaps more interestingly, using it for
fault tolerance, keeping 2 or 3 copies of each page on different
servers. Maybe someone will say "hey, just use iSCSI" or "hey, just
use AFS" or something like that, but I dunno if I trust them to do the
right thing with fsync, and they might not be as well-optimized as
would be possible if we rolled our own. At any rate, if we DON'T
think we'd ever consider supporting something like this, perhaps we
ought to just fold the md.c stuff into smgr.c and eliminate all the
jumping through hoops.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company

Attachment Content-Type Size
smgrcreate_cleanup.patch application/octet-stream 2.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2010-08-20 02:45:29 Re: security hook on authorization
Previous Message Robert Haas 2010-08-20 01:43:02 Re: BUG #5305: Postgres service stops when closing Windows session