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

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: andres(at)anarazel(dot)de
Cc: michael(at)paquier(dot)xyz, kuntalghosh(dot)2007(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Regression test PANICs with master-standby setup on same machine
Date: 2019-04-24 04:23:04
Message-ID: 20190424.132304.40676137.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Sorry, I was in haste.

At Wed, 24 Apr 2019 13:18:45 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20190424(dot)131845(dot)116224815(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> 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. Another 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.

The second attached is TAP change to support tablespaces using
relative tablespaces.

- 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Paul Guo 2019-04-24 04:46:15 [Patch] Check file type before calling AllocateFile() for files out of pg data directory to avoid potential issues (e.g. hang).
Previous Message Kyotaro HORIGUCHI 2019-04-24 04:18:45 Re: Regression test PANICs with master-standby setup on same machine