Re: fix tablespace handling in pg_combinebackup

From: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
To: Alexander Lakhin <exclusion(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)anarazel(dot)de>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fix tablespace handling in pg_combinebackup
Date: 2024-04-21 21:49:47
Message-ID: CA+hUKGLnPyTfSFV7zquu1YSNLLjkPT3r0eDTZ+PWKQ4XU3DW3A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 22, 2024 at 12:00 AM Alexander Lakhin <exclusion(at)gmail(dot)com> wrote:
> From what I can see, the following condition (namely, -l):
> if ($path =~ /^pg_tblspc\/(\d+)$/ && -l "$backup_path/$path")
> {
> push @tsoids, $1;
> return 0;
> }
>
> is false for junction points on Windows (cf [1]), but the target path is:

Ah, yes, right, -l doesn't like junction points. Well, we're already
using the Win32API::File package (it looks like a CPAN library, but I
guess the Windows perl distros like Strawberry are all including it
for free?). See PostgreSQL::Test::Utils::is_symlink(), attached.
That seems to work as expected, but someone who actually knows perl
can surely make it better. Then I hit the next problem:

readlink C:\cirrus\build/testrun/pg_combinebackup/002_compare_backups\data/t_002_compare_backups_primary_data/backup/backup1/pg_tblspc/16415:
Inappropriate I/O control operation at
C:/cirrus/src/test/perl/PostgreSQL/Test/Cluster.pm line 927.

https://cirrus-ci.com/task/5162332353986560

I don't know where exactly that error message is coming from, but
assuming that Strawberry Perl contains this code:

https://github.com/Perl/perl5/blob/f936cd91ee430786a1bb6068a4a7c8362610dd5f/win32/win32.c#L2041
https://github.com/Perl/perl5/blob/f936cd91ee430786a1bb6068a4a7c8362610dd5f/win32/win32.c#L1976

... then it's *very* similar to what we're doing in our own
pgreadlink() code. I wondered if the buffer size might be too small
for our path, but it doesn't seem so:

https://github.com/Perl/perl5/blob/f936cd91ee430786a1bb6068a4a7c8362610dd5f/win32/win32.c#L1581C1-L1581C35

(I think MAX_PATH is 256 on Windows.)

If there is some unfixable problem with what they're doing in their
readlink(), then I guess it should be possible to read the junction
point directly in Perl using Win32API::File::DeviceIoControl()... but
I can't see what's wrong with it! Maybe it's not the same code?

Attached are the new test support functions, and the fixup to Robert's
6bf5c42b that uses them. To be clear, this doesn't work, yet. It has
got to be close though...

Attachment Content-Type Size
0001-More-Windows-pseudo-symlink-support-for-Perl-code.patch application/octet-stream 2.2 KB
0002-fixup.patch application/octet-stream 1.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Imseih (AWS), Sami 2024-04-21 23:26:59 Re: allow changing autovacuum_max_workers without restarting
Previous Message Tom Lane 2024-04-21 21:08:08 Re: DROP OWNED BY fails to clean out pg_init_privs grants