Re: crash-recovery replay of CREATE TABLESPACE is broken in HEAD

Bruce Momjian wrote:
> > The attached patch does as suggested.  I added the recovery code to the
> > create tablespace function so I didn't have to duplicate all the code
> > that computes the path names.
> >
> > Attached.
>
> Uh, another question.  Looking at the createdb recovery, I see:
>
>         /*
>          * Our theory for replaying a CREATE is to forcibly drop the target
>          * subdirectory if present, then re-copy the source data. This may be
>          * more work than needed, but it is simple to implement.
>          */
>         if (stat(dst_path, &st) == 0 && S_ISDIR(st.st_mode))
>         {
>             if (!rmtree(dst_path, true))
>                 ereport(WARNING,
>                         (errmsg("some useless files may be left behind in old database directory \"%s\"",
>                                 dst_path)));
>         }
>
> Should I be using rmtree() on the mkdir target?
>
> Also, the original tablespace recovery code did not drop the symlink
> first.  I assume that was not a bug only because we don't support moving
> tablespaces:

For consistency with CREATE DATABASE recovery and for reliablity, I
coded the rmtree() call instead.  Patch attached.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + None of us is going to be here forever. +
Index: src/backend/commands/tablespace.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.77
diff -c -c -r1.77 tablespace.c
*** src/backend/commands/tablespace.c    18 Jul 2010 04:47:46 -0000    1.77
--- src/backend/commands/tablespace.c    19 Jul 2010 04:59:03 -0000
***************
*** 538,543 ****
--- 538,544 ----
      char       *linkloc = palloc(OIDCHARS + OIDCHARS + 1);
      char       *location_with_version_dir = palloc(strlen(location) + 1 +
                                     strlen(TABLESPACE_VERSION_DIRECTORY) + 1);
+     struct stat st;

      sprintf(linkloc, "pg_tblspc/%u", tablespaceoid);
      sprintf(location_with_version_dir, "%s/%s", location,
***************
*** 562,567 ****
--- 563,584 ----
                           location)));
      }

+     if (InRecovery)
+     {
+         /*
+          * Our theory for replaying a CREATE is to forcibly drop the target
+          * subdirectory if present, and then recreate it. This may be
+          * more work than needed, but it is simple to implement.
+          */
+         if (stat(location_with_version_dir, &st) == 0 && S_ISDIR(st.st_mode))
+         {
+             if (!rmtree(location_with_version_dir, true))
+                 ereport(WARNING,
+                         (errmsg("some useless files may be left behind in old database directory \"%s\"",
+                                 location_with_version_dir)));
+         }
+     }
+
      /*
       * The creation of the version directory prevents more than one tablespace
       * in a single location.
***************
*** 580,585 ****
--- 597,612 ----
                              location_with_version_dir)));
      }

+     /* Remove old symlink in recovery, in case it points to the wrong place */
+     if (InRecovery)
+     {
+         if (unlink(linkloc) < 0 && errno != ENOENT)
+             ereport(ERROR,
+                     (errcode_for_file_access(),
+                      errmsg("could not remove symbolic link \"%s\": %m",
+                             linkloc)));
+     }
+
      /*
       * Create the symlink under PGDATA
       */