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
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 |