Thread: Auto create (top level) directory for create tablespace

I thought it made sense for CREATE TABLESPACE to attempt to create the
top level location directory - and also for tablespace redo to do
likwewise during WAL replay.

Tablespace creation then behaves a bit more like intidb with respect to
directory creation, which I found quite nice.

Patch against HEAD, passes regression tests.

Cheers

Mark
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.51
diff -c -r1.51 tablespace.c
*** src/backend/commands/tablespace.c    15 Nov 2007 21:14:34 -0000    1.51
--- src/backend/commands/tablespace.c    15 Dec 2007 19:04:45 -0000
***************
*** 199,204 ****
--- 199,205 ----
      Oid            tablespaceoid;
      char       *location;
      char       *linkloc;
+     struct stat    st;
      Oid            ownerId;

      /* Must be super user */
***************
*** 297,302 ****
--- 298,317 ----
      recordDependencyOnOwner(TableSpaceRelationId, tablespaceoid, ownerId);

      /*
+      * Try to create the target directory if it does not exist.
+      */
+     if (stat(location, &st) < 0)
+     {
+         if (mkdir(location, 0700) != 0)
+                 ereport(ERROR,
+                     (errcode_for_file_access(),
+                  errmsg("could not create location directory \"%s\": %m",
+                         location)));
+
+     }
+
+
+     /*
       * Attempt to coerce target directory to safe permissions.    If this fails,
       * it doesn't exist or has the wrong owner.
       */
***************
*** 1279,1284 ****
--- 1294,1314 ----
          xl_tblspc_create_rec *xlrec = (xl_tblspc_create_rec *) XLogRecGetData(record);
          char       *location = xlrec->ts_path;
          char       *linkloc;
+         struct stat    st;
+
+         /*
+          * Try to create the target directory if it does not exist.
+          */
+         if (stat(location, &st) < 0)
+         {
+             if (mkdir(location, 0700) != 0)
+             {
+                 ereport(ERROR,
+                         (errcode_for_file_access(),
+                      errmsg("could not create location directory \"%s\": %m",
+                             location)));
+             }
+         }

          /*
           * Attempt to coerce target directory to safe permissions.    If this
Index: src/test/regress/output/tablespace.source
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/output/tablespace.source,v
retrieving revision 1.5
diff -c -r1.5 tablespace.source
*** src/test/regress/output/tablespace.source    3 Jun 2007 22:16:03 -0000    1.5
--- src/test/regress/output/tablespace.source    15 Dec 2007 19:04:46 -0000
***************
*** 57,63 ****

  -- Will fail with bad path
  CREATE TABLESPACE badspace LOCATION '/no/such/location';
! ERROR:  could not set permissions on directory "/no/such/location": No such file or directory
  -- No such tablespace
  CREATE TABLE bar (i int) TABLESPACE nosuchspace;
  ERROR:  tablespace "nosuchspace" does not exist
--- 57,63 ----

  -- Will fail with bad path
  CREATE TABLESPACE badspace LOCATION '/no/such/location';
! ERROR:  could not create location directory "/no/such/location": No such file or directory
  -- No such tablespace
  CREATE TABLE bar (i int) TABLESPACE nosuchspace;
  ERROR:  tablespace "nosuchspace" does not exist

Mark Kirkwood <markir@paradise.net.nz> writes:
> I thought it made sense for CREATE TABLESPACE to attempt to create the
> top level location directory -

I thought we had deliberately made it not do that.  Auto-recreate during
replay sounds even worse.  The problem is that a tablespace would
normally be under a mount point, and auto-create has zero chance of
getting such a path right.

Ignoring this point is actually a fine recipe for destroying your data;
see Joe Conway's report a couple years back about getting burnt by a
soft NFS mount.  If the DB directory is not there, auto-creating it is
a horrible idea.

            regards, tom lane

Tom Lane wrote:
> Mark Kirkwood <markir@paradise.net.nz> writes:
>
>> I thought it made sense for CREATE TABLESPACE to attempt to create the
>> top level location directory -
>>
>
> I thought we had deliberately made it not do that.  Auto-recreate during
> replay sounds even worse.  The problem is that a tablespace would
> normally be under a mount point, and auto-create has zero chance of
> getting such a path right.
>
> Ignoring this point is actually a fine recipe for destroying your data;
> see Joe Conway's report a couple years back about getting burnt by a
> soft NFS mount.  If the DB directory is not there, auto-creating it is
> a horrible idea.
>
>

Hmm - ok, unmounted filesystems could bite you. However, they could bite
folks creating the directory manually too...(I guess you could argue it
is less likely though).

On the replay front, the use case I was thinking about is standby
database - the classic foot gun there is to create a tablespace on
source box and forget to add the appropriate directory on the target....
and bang! replay fails.

It does seem to me like there are scenarios where either behavior is
undesirable... a possible option is a configuration parameter to choose
between auto creation or not. However I'm happy to go with the consensus
here - if its universally deemed to be a terrible idea, then let's ditch
the patch :-)

Best wishes

Mark

Tom Lane wrote:
> Ignoring this point is actually a fine recipe for destroying your data;
> see Joe Conway's report a couple years back about getting burnt by a
> soft NFS mount.  If the DB directory is not there, auto-creating it is
> a horrible idea.

Yes, this would be equivalent to the old behavior of the Red Hat RPMs that
automatically performed initdb if the data directory is missing.  Thankfully,
that has been abolished.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/