Horton

Re: pg_rewind fails on Windows where tablespaces are used

Alexandra Wang <alexandra.wang.oss@gmail.com>

View thread
From: Alexandra Wang <alexandra.wang.oss@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andrew Dunstan <andrew@dunslane.net>, Chris Travers <chris.travers@stormatics.tech>, Farooq Rashed <farooq.rashed@desc.gov.ae>, pgsql-bugs@lists.postgresql.org
Date: 2024-10-23T16:19:14Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix readlink() for non-PostgreSQL junction points on Windows.

  2. Fix stat() for recursive junction points on Windows.

  3. Fix pg_rewind with in-place tablespaces when source is remote

  4. Make unlink() work for junction points on Windows.

  5. Use in-place tablespaces in regression test.

Attachments

Hi,

I’ve attached fixes in 4 tarballs for branches 12 to 15.

On Wed, Oct 23, 2024 at 9:38 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Tue, Jul 09, 2024 at 12:01:17PM -0400, Andrew Dunstan wrote:
>
> > The solution I came up with was to backpatch commits c5cb8f3b, 387803d8 and
> > 5fc88c5d53.
>
> The lstat() wrapper for Windows, noted.

In addition to the three commits from Andrew’s solution, I also
backpatched commit f357233c. Without this commit, DROP TABLESPACE for
a non-in-place tablespace would fail with the following error:

drop tablespace regress_tblspacewith;
ERROR:  42501: could not remove symbolic link "pg_tblspc/83531":
Permission denied
LOCATION:  destroy_tablespace_directories, tablespace.c:829

I encountered this issue while working on the fix for branch 14 and
running the tablespace regress test. This simple test is not covered
in branch 15’s regress tests, as we started setting
allow_in_place_tablespaces = true since commit d6d317db.

> > The only question in my mind is whether those patches should be
> > backpatched.
> >
> > It's a couple of hundred lines, and I think it's safe, but I'd welcome other
> > opinions. If we are going to backpatch them we should also look at adding to
> > adding tests for use of tablespaces with pg_rewind on the back branches.
> > Ideally we'd get this done in time for the next maintenance release.
>
> Seeing that the commits all go down to v16, meaning that these have
> brewed across 3 minor releases already, I'd like to assume that we
> would have already heard about problems related to them.  So that
> seems like a rather safe thing to do at this stage.

I also had to backpatch additional commits for branches 12 to 14, as
follows:

branch 14: e2f0f8ed, af9e6331, and the commits for branch 15
[f357233c, c5cb8f3b, 387803d8, and 5fc88c5d53].
branches 12 & 13: bed90759, 54fb8c7d, de8feb1f, 101c37cd, and the
commits for branch 14.

With these additional commits for branches 12 to 14, I’m not sure if
it’s worth backpatching, or should we backpatch only to branch 15?

I agree with Andrew that if we decide to backpatch these changes, we
should add tests for the pg_rewind issue. Would the pg_rewind TAP
tests be the place for them?

Best,
Alex