Re: pgsql: Fix pg_basebackup with in-place tablespaces.

From: David Steele <david(at)pgmasters(dot)net>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, thomas(dot)munro(at)gmail(dot)com
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, tmunro(at)postgresql(dot)org, pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Fix pg_basebackup with in-place tablespaces.
Date: 2022-03-16 14:29:17
Message-ID: 8893318e-5778-ebf4-71b0-afb210d71a52@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

On 3/15/22 23:42, Kyotaro Horiguchi wrote:
> At Wed, 16 Mar 2022 11:13:53 +1300, Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote in
>> On Wed, Mar 16, 2022 at 10:28 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> David Steele <david(at)pgmasters(dot)net> writes:
>>>> On 3/14/22 19:31, Thomas Munro wrote:
>>>>> Fix pg_basebackup with in-place tablespaces.
>>>
>>>> Perhaps I'm being picky, but seems like this logic should be wrapped in:
>>>> if (allow_in_place_tablespaces)
>>>> {
>>>> <...>
>>>> }
>>>> I worry about strange effects when this GUC is not enabled.
>>>
>>> What would happen if someone created an in-place tablespace and
>>> then turned off the GUC?
>>
>> Then it would break with unhelpful error messages. I suppose we could
>> error out explicitly, "in-place tablespace detected, but
>> allow_in_place_tablespaces not enabled". I'm not sure why we should
>> suddenly choose to enforce that *here* though.
>
> +1. The GUC is only to prevent non-developer users from accidentally
> creating in-place tablespaces that is not officieally suported. We
> have done nothing about in-place tablespaces other than the things
> needed to perform regression test, and I think we won't do that in
> future.

Sure, but there is a behavioral change whether the GUC is enabled or
not. Before, if there was clutter in pg_tblspc there would at least be a
warning in the log. Now that logging does not happen.

It seems that the warning at line 8313 is essentially dead code now. I
don't expect test code to have an impact on production systems, even if
the effect is minor.

I'm less worried about what happens when the flag is flipped on then off
because that's not likely to happen in production.

Regards,
-David

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Justin Pryzby 2022-03-16 15:08:12 Re: pgsql: Allow extensions to add new backup targets.
Previous Message Robert Haas 2022-03-16 13:52:41 Re: pgsql: Allow extensions to add new backup targets.