Re: Tablespace patch review

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Gavin Sherry <swm(at)linuxworld(dot)com(dot)au>, pgsql-patches(at)postgresql(dot)org
Subject: Re: Tablespace patch review
Date: 2004-06-18 06:53:08
Message-ID: 16291.1087541588@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> OK, I have reviewed the patch. I think Tom is doing the same, but I
> want to report the things I found.

I just came up for air after about two solid days of working on this
patch ... had not seen your message before committing it. The good
news is that I think I did see all the stuff you found.

> What facility is there for moving objects between tablespaces?

None, as yet.

> Seems we should be consistent in having WIN32 defs or not.

Probably. I removed #ifdefs whereever possible --- there are just a few
left in tablespace.c and dbcommands.c now. I was contemplating
replacing HAVE_SYMLINKS with a HAVE_TABLESPACES flag, but with the
occurrences isolated to one file I'm not sure it's worth the trouble.

> Your code in tablespc.c calls realpath(). Do all OS's have that?

It doesn't anymore --- I was concerned about the portability question
too. The only point of that code AFAICS was to prevent creation of
two pg_tablespace entries pointing at the same directory. I felt that
the better way to handle this was to write a PG_VERSION file in the
tablespace directory during CREATE TABLESPACE. A subsequent CREATE
TABLESPACE on the same directory will fail because the directory isn't
empty anymore. And the version file might come in handy someday...

> Does all object creation code put a lock in the tablespace row to
> prevent DROP TABLESPACE from removing a tablespace in use?

I hacked up some logic to deal with this, based on taking out an
ExclusiveLock on pg_tablespace when adding a per-database subdirectory
to a tablespace directory or doing DROP TABLESPACE. It works but it'd
be nice to reduce the strength of the lock ...

> There is interesting code that checks to see of the objects existing in
> a tablespace are about to be dropped by the transaction.

s/interesting/doesn't work/ ... I didn't commit this stuff. Nor the
smgr changes either; those were far from ready for prime time.

> Are we ripping out our initlocation code at the same time?

Not done yet, but it's dead and should be removed as soon as a
decent respect for the deceased permits ;-)

> Where do we need to add mention of tablespaces in the main
> non-reference-page docs? Clearly at least in the section on managing
> disk space.

Yeah. The patch as committed covers the reference pages, but we
desperately need a higher-level discussion of tablespaces for the
administrator's guide.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2004-06-18 06:56:56 Re: Tablespace patch review
Previous Message Christopher Kings-Lynne 2004-06-18 06:48:38 Re: pgsql-server: Tablespaces.

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2004-06-18 06:56:56 Re: Tablespace patch review
Previous Message Christopher Kings-Lynne 2004-06-18 05:35:06 Re: Tablespace patch review