Re: Regression test PANICs with master-standby setup on same machine

At Tue, 23 Apr 2019 10:05:03 -0700, Andres Freund <andres(at)anarazel(dot)de> wrote in <20190423170503(dot)uw5jxrujqlozg23l(at)alap3(dot)anarazel(dot)de>
> Hi,
>
> On 2019-04-23 16:08:18 +0900, Michael Paquier wrote:
> > On Mon, Apr 22, 2019 at 11:00:03PM -0700, Andres Freund wrote:
> > > FWIW, I think the right fix for this is to simply drop the requirement
> > > that tablespace paths need to be absolute. It's not buying us anything,
> > > it's just making things more complicated. We should just do a simple
> > > check against the tablespace being inside PGDATA, and leave it at
> > > that. Yes, that can be tricked, but so can the current system.
> >
> > convert_and_check_filename() checks after that already, mostly. For
> > TAP tests I am not sure that this would help much though as all the
> > nodes of a given test use the same root path for their data folders,
> > so you cannot just use "../hoge/" as location.
>
> I don't see the problem here. Putting the primary and standby PGDATAs
> into a subdirectory that also can contain a relatively referenced
> tablespace seems trivial?
>
> > I'm not We already generate a warning when a tablespace is in a data
> > folder, as this causes issues with recursion lookups of base backups.
> > What do you mean in this case? Forbidding the behavior? -- Michael
>
> I mostly am talking about replacing
>
> Oid
> CreateTableSpace(CreateTableSpaceStmt *stmt)
> {
> ...
> /*
> * Allowing relative paths seems risky
> *
> * this also helps us ensure that location is not empty or whitespace
> */
> if (!is_absolute_path(location))
> ereport(ERROR,
> (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
> errmsg("tablespace location must be an absolute path")));
>
> with a check that forces relative paths to be outside of PGDATA (baring
> symlinks). As far as I can tell convert_and_check_filename() would check
> just about the opposite.

We need to adjust relative path between PGDATA-based and
pg_tblspc based. The attached first patch does that.

- I'm not sure it is OK to use getcwd this way.

The second attached is TAP change to support tablespaces using
relative tablespaces. One issue here is is_in_data_directory
canonicalizes DataDir on-the-fly. It is needed when DataDir
contains '/./' or such. I think the canonicalization should be
done far earlier.

- This is tentative or sample. I'll visit the current discussion thread.

The third is test for this issue.

- Tablespace handling gets easier.

The fourth is the fix for the issue here.

- Not all possible simliar issue is not checked.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center