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

Bruce Momjian wrote:
> Tom Lane wrote:
> > Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> > > Maybe you should check that it points to the right location? Or drop and
> > > recreate the symlink, and ignore failure at mkdir.
> >
> > More specifically, ignore EEXIST failure when replaying mkdir. Anything
> > else is still a problem.
>
> Thanks for the help. I tried to find somewhere else in our recovery
> code that was similar but didn't find anything.
>
> 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:

- /* Create the symlink if not already present */
- linkloc = (char *) palloc(OIDCHARS + OIDCHARS + 1);
- sprintf(linkloc, "pg_tblspc/%u", xlrec->ts_id);
-
- if (symlink(location, linkloc) < 0)
- {
- if (errno != EEXIST)
- ereport(ERROR,
- (errcode_for_file_access(),
- errmsg("could not create symbolic link \"%s\": %m",
- linkloc)));
- }

Still, it seems logical to unlink it before creating it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ None of us is going to be here forever. +