pg_basebackup check vs Windows file path limits

Started by Andrew Dunstanover 2 years ago20 messages
#1Andrew Dunstan
andrew@dunslane.net

The buildfarm animal fairywren has been failing the tests for
pg_basebackup because it can't create a file with a path longer than 255
chars. This has just been tripped because for release 16 it's running
TAP tests, and the branch name is part of the file path, and
"REL_16_STABLE" is longer than "HEAD". I did think of chdir'ing into the
directory to create the file, but experimentation shows that doesn't
solve matters. I also adjusted the machine's settings related to long
file names, but to no avail, so for now I propose to reduce slightly the
name of the long file so it still exercises the check for file names
longer than 100 but doesn't trip this up on fairywren. But that's a
bandaid. I don't have a good solution for now.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#1)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-02 Su 09:15, Andrew Dunstan wrote:

The buildfarm animal fairywren has been failing the tests for
pg_basebackup because it can't create a file with a path longer than
255 chars. This has just been tripped because for release 16 it's
running TAP tests, and the branch name is part of the file path, and
"REL_16_STABLE" is longer than "HEAD". I did think of chdir'ing into
the directory to create the file, but experimentation shows that
doesn't solve matters. I also adjusted the machine's settings related
to long file names, but to no avail, so for now I propose to reduce
slightly the name of the long file so it still exercises the check for
file names longer than 100 but doesn't trip this up on fairywren. But
that's a bandaid. I don't have a good solution for now.

I've pushed a better solution, which creates the file via a short
symlink. Experimentation on fairywren showed this working.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Andrew Dunstan (#2)
Re: pg_basebackup check vs Windows file path limits

On 3 Jul 2023, at 16:12, Andrew Dunstan <andrew@dunslane.net> wrote:

I've pushed a better solution, which creates the file via a short symlink. Experimentation on fairywren showed this working.

The buildfarm seems a tad upset after this?

--
Daniel Gustafsson

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Daniel Gustafsson (#3)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-03 Mo 10:16, Daniel Gustafsson wrote:

On 3 Jul 2023, at 16:12, Andrew Dunstan<andrew@dunslane.net> wrote:
I've pushed a better solution, which creates the file via a short symlink. Experimentation on fairywren showed this working.

The buildfarm seems a tad upset after this?

Yeah :-(

I think it should be fixing itself now.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#5Daniel Gustafsson
daniel@yesql.se
In reply to: Andrew Dunstan (#4)
Re: pg_basebackup check vs Windows file path limits

On 3 Jul 2023, at 17:18, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2023-07-03 Mo 10:16, Daniel Gustafsson wrote:

On 3 Jul 2023, at 16:12, Andrew Dunstan <andrew@dunslane.net>
wrote:

I've pushed a better solution, which creates the file via a short symlink. Experimentation on fairywren showed this working.

The buildfarm seems a tad upset after this?

Yeah :-(

I think it should be fixing itself now.

Yeah, thanks for speedy fix!

--
Daniel Gustafsson

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#4)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-03 Mo 11:18, Andrew Dunstan wrote:

On 2023-07-03 Mo 10:16, Daniel Gustafsson wrote:

On 3 Jul 2023, at 16:12, Andrew Dunstan<andrew@dunslane.net> wrote:
I've pushed a better solution, which creates the file via a short symlink. Experimentation on fairywren showed this working.

The buildfarm seems a tad upset after this?

Yeah :-(

I think it should be fixing itself now.

But sadly we're kinda back where we started. fairywren is failing on
REL_16_STABLE. Before the changes the failure occurred because the test
script was unable to create the file with a path > 255. Now that we have
a way to create the file the test for pg_basebackup to reject files with
names > 100 fails, I presume because the server can't actually see the
file. At this stage I'm thinking the best thing would be to skip the
test altogether on windows if the path is longer than 255.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#7Daniel Gustafsson
daniel@yesql.se
In reply to: Andrew Dunstan (#6)
Re: pg_basebackup check vs Windows file path limits

On 4 Jul 2023, at 20:19, Andrew Dunstan <andrew@dunslane.net> wrote:

But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

--
Daniel Gustafsson

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Daniel Gustafsson (#7)
1 attachment(s)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-04 Tu 16:54, Daniel Gustafsson wrote:

On 4 Jul 2023, at 20:19, Andrew Dunstan<andrew@dunslane.net> wrote:
But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

Not sure it's such a big hammer. Here's a patch.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

Attachments:

pg_basebackup-long-file-fix.patchtext/x-patch; charset=UTF-8; name=pg_basebackup-long-file-fix.patchDownload
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index e0009c8531..ee0d03512c 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -311,20 +311,22 @@ $node->command_fails(
 	'-T with invalid format fails');
 
 # Tar format doesn't support filenames longer than 100 bytes.
-# Create the test file via a short name directory so it doesn't blow the
-# Windows path limit.
-my $lftmp = PostgreSQL::Test::Utils::tempdir_short;
-dir_symlink "$pgdata", "$lftmp/pgdata";
-my $superlongname = "superlongname_" . ("x" x 100);
-my $superlongpath = "$lftmp/pgdata/$superlongname";
+SKIP:
+{
+	my $superlongname = "superlongname_" . ("x" x 100);
+	my $superlongpath = "$pgdata/$superlongname";
 
-open my $file, '>', "$superlongpath"
-  or die "unable to create file $superlongpath";
-close $file;
-$node->command_fails(
-	[ @pg_basebackup_defs, '-D', "$tempdir/tarbackup_l1", '-Ft' ],
-	'pg_basebackup tar with long name fails');
-unlink "$superlongpath";
+	skip "File path too long", 1
+	  if $windows_os && length($superlongpath) > 255;
+
+	open my $file, '>', "$superlongpath"
+	  or die "unable to create file $superlongpath";
+	close $file;
+	$node->command_fails(
+		[ @pg_basebackup_defs, '-D', "$tempdir/tarbackup_l1", '-Ft' ],
+		'pg_basebackup tar with long name fails');
+	unlink "$superlongpath";
+}
 
 # The following tests are for symlinks.
 
#9Daniel Gustafsson
daniel@yesql.se
In reply to: Andrew Dunstan (#8)
Re: pg_basebackup check vs Windows file path limits

On 5 Jul 2023, at 14:49, Andrew Dunstan <andrew@dunslane.net> wrote:
On 2023-07-04 Tu 16:54, Daniel Gustafsson wrote:

On 4 Jul 2023, at 20:19, Andrew Dunstan <andrew@dunslane.net>
wrote:

But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

Not sure it's such a big hammer. Here's a patch.

No objections to the patch, LGTM.

--
Daniel Gustafsson

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Daniel Gustafsson (#9)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-06 Th 09:50, Daniel Gustafsson wrote:

On 5 Jul 2023, at 14:49, Andrew Dunstan<andrew@dunslane.net> wrote:
On 2023-07-04 Tu 16:54, Daniel Gustafsson wrote:

On 4 Jul 2023, at 20:19, Andrew Dunstan<andrew@dunslane.net>
wrote:

But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

Not sure it's such a big hammer. Here's a patch.

No objections to the patch, LGTM.

Thanks. pushed with a couple of tweaks.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#10)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-06 Th 12:38, Andrew Dunstan wrote:

On 2023-07-06 Th 09:50, Daniel Gustafsson wrote:

On 5 Jul 2023, at 14:49, Andrew Dunstan<andrew@dunslane.net> wrote:
On 2023-07-04 Tu 16:54, Daniel Gustafsson wrote:

On 4 Jul 2023, at 20:19, Andrew Dunstan<andrew@dunslane.net>
wrote:

But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

Not sure it's such a big hammer. Here's a patch.

No objections to the patch, LGTM.

Thanks. pushed with a couple of tweaks.

Unfortunately, skipping this has now exposed a further problem in this test.

Here's the relevant log extracted from
<https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2023-07-07%2022%3A03%3A06&gt;,
starting with the skip mentioned above:

[23:29:21.661](0.002s) ok 98 # skip File path too long
### Stopping node "main" using mode fast
# Running: pg_ctl -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/pgdata -m fast stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "main"
Junction created for C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\t_010_pg_basebackup_main_data\\pgdata\\pg_replslot <<===>> C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\tmp_test_pjj2\\pg_replslot
### Starting node "main"
# Running: pg_ctl -w -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/pgdata -l C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/log/010_pg_basebackup_main.log -o --cluster-name=main start
waiting for server to start.... done
server started
# Postmaster PID for node "main" is 5184
Junction created for C:\\tools\\nmsys64\\tmp\\6zkMt003MF\\tempdir <<===>> C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\tmp_test_pjj2
# Taking pg_basebackup tarbackup2 from node "main"
# Running: pg_basebackup -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/backup/tarbackup2 -h C:/tools/nmsys64/tmp/63ohSgsh21 -p 54699 --checkpoint fast --no-sync -Ft
WARNING: aborting backup due to backend exiting before pg_backup_stop was called
pg_basebackup: error: could not initiate base backup: ERROR: could not get junction for "./pg_replslot": More data is available.

It's worth pointing out that the path for the replslot junction is almost as long as the original path.

Since this test is passing on HEAD which has slightly shorter paths, I'm wondering if we should change this:

rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
  or BAIL_OUT "could not move $pgdata/pg_replslot";
dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";

to use the much shorter $sys_tempdir created a few lines below.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#12Andrew Dunstan
andrew@dunslane.net
In reply to: Andrew Dunstan (#11)
Re: pg_basebackup check vs Windows file path limits

On 2023-07-08 Sa 09:15, Andrew Dunstan wrote:

On 2023-07-06 Th 12:38, Andrew Dunstan wrote:

On 2023-07-06 Th 09:50, Daniel Gustafsson wrote:

On 5 Jul 2023, at 14:49, Andrew Dunstan<andrew@dunslane.net> wrote:
On 2023-07-04 Tu 16:54, Daniel Gustafsson wrote:

On 4 Jul 2023, at 20:19, Andrew Dunstan<andrew@dunslane.net>
wrote:

But sadly we're kinda back where we started. fairywren is failing on REL_16_STABLE. Before the changes the failure occurred because the test script was unable to create the file with a path > 255. Now that we have a way to create the file the test for pg_basebackup to reject files with names > 100 fails, I presume because the server can't actually see the file. At this stage I'm thinking the best thing would be to skip the test altogether on windows if the path is longer than 255.

That does sound like a fairly large hammer for a nail small enough that we
should be able to fix it, but I don't have any other good ideas off the cuff.

Not sure it's such a big hammer. Here's a patch.

No objections to the patch, LGTM.

Thanks. pushed with a couple of tweaks.

Unfortunately, skipping this has now exposed a further problem in this
test.

Here's the relevant log extracted from
<https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=fairywren&amp;dt=2023-07-07%2022%3A03%3A06&gt;,
starting with the skip mentioned above:

[23:29:21.661](0.002s) ok 98 # skip File path too long
### Stopping node "main" using mode fast
# Running: pg_ctl -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/pgdata -m fast stop
waiting for server to shut down.... done
server stopped
# No postmaster PID for node "main"
Junction created for C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\t_010_pg_basebackup_main_data\\pgdata\\pg_replslot <<===>> C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\tmp_test_pjj2\\pg_replslot
### Starting node "main"
# Running: pg_ctl -w -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/pgdata -l C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/log/010_pg_basebackup_main.log -o --cluster-name=main start
waiting for server to start.... done
server started
# Postmaster PID for node "main" is 5184
Junction created for C:\\tools\\nmsys64\\tmp\\6zkMt003MF\\tempdir <<===>> C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build\\testrun\\pg_basebackup\\010_pg_basebackup\\data\\tmp_test_pjj2
# Taking pg_basebackup tarbackup2 from node "main"
# Running: pg_basebackup -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/backup/tarbackup2 -h C:/tools/nmsys64/tmp/63ohSgsh21 -p 54699 --checkpoint fast --no-sync -Ft
WARNING: aborting backup due to backend exiting before pg_backup_stop was called
pg_basebackup: error: could not initiate base backup: ERROR: could not get junction for "./pg_replslot": More data is available.

It's worth pointing out that the path for the replslot junction is almost as long as the original path.

Since this test is passing on HEAD which has slightly shorter paths, I'm wondering if we should change this:

rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
  or BAIL_OUT "could not move $pgdata/pg_replslot";
dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";

to use the much shorter $sys_tempdir created a few lines below.

Pushed a tested fix along those lines.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#13Alexander Lakhin
exclusion@gmail.com
In reply to: Andrew Dunstan (#12)
Re: pg_basebackup check vs Windows file path limits

Hello Andrew,

08.07.2023 18:52, Andrew Dunstan wrote:

Since this test is passing on HEAD which has slightly shorter paths, I'm wondering if we should change this:

rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
  or BAIL_OUT "could not move $pgdata/pg_replslot";
dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";

to use the much shorter $sys_tempdir created a few lines below.

Pushed a tested fix along those lines.

Today I've started up my Windows VM to run some tests and discovered a test
failure caused by that fix (e213de8e7):

meson test

Ok:                 246
Expected Fail:      0
Fail:               1
Unexpected Pass:    0
Skipped:            14
Timeout:            0

...\010_pg_basebackup\log\regress_log_010_pg_basebackup.txt contains:
[04:42:45.321](0.291s) Bail out!  could not move
T:\postgresql\build/testrun/pg_basebackup/010_pg_basebackup\data/t_010_pg_basebackup_main_data/pgdata/pg_replslot

With a diagnostic print added before rename() in 010_pg_basebackup.pl, I see:
rename("T:\postgresql\build/testrun/pg_basebackup/010_pg_basebackup\data/t_010_pg_basebackup_main_data/pgdata/pg_replslot",
"C:\Users\User\AppData\Local\Temp\fGT76tZUWr/pg_replslot")
That is, I have the postgres source tree and the user tempdir placed on
different disks.

perldoc on rename() says that it usually doesn't work across filesystem
boundaries, so I think it's not a Windows-specific issue.

Best regards,
Alexander

#14Andrew Dunstan
andrew@dunslane.net
In reply to: Alexander Lakhin (#13)
Re: pg_basebackup check vs Windows file path limits

Hi, Alexander

On 2023-11-11 Sa 08:00, Alexander Lakhin wrote:

Hello Andrew,

08.07.2023 18:52, Andrew Dunstan wrote:

Since this test is passing on HEAD which has slightly shorter paths, I'm wondering if we should change this:

rename("$pgdata/pg_replslot", "$tempdir/pg_replslot")
  or BAIL_OUT "could not move $pgdata/pg_replslot";
dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")
  or BAIL_OUT "could not symlink to $pgdata/pg_replslot";

to use the much shorter $sys_tempdir created a few lines below.

Pushed a tested fix along those lines.

Today I've started up my Windows VM to run some tests and discovered a
test
failure caused by that fix (e213de8e7):

meson test

Ok:                 246
Expected Fail:      0
Fail:               1
Unexpected Pass:    0
Skipped:            14
Timeout:            0

...\010_pg_basebackup\log\regress_log_010_pg_basebackup.txt contains:
[04:42:45.321](0.291s) Bail out!  could not move
T:\postgresql\build/testrun/pg_basebackup/010_pg_basebackup\data/t_010_pg_basebackup_main_data/pgdata/pg_replslot

With a diagnostic print added before rename() in 010_pg_basebackup.pl,
I see:
rename("T:\postgresql\build/testrun/pg_basebackup/010_pg_basebackup\data/t_010_pg_basebackup_main_data/pgdata/pg_replslot",
"C:\Users\User\AppData\Local\Temp\fGT76tZUWr/pg_replslot")
That is, I have the postgres source tree and the user tempdir placed on
different disks.

perldoc on rename() says that it usually doesn't work across filesystem
boundaries, so I think it's not a Windows-specific issue.

Hmm, maybe we should be using File::Copy::move() instead of rename().
The docco for that says:

        If possible, move() will simply rename the file. Otherwise, it
        copies the file to the new location and deletes the original. If an
        error occurs during this copy-and-delete process, you may be left
        with a (possibly partial) copy of the file under the destination
        name.

Can you try it out?

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#15Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Andrew Dunstan (#11)
Re: pg_basebackup check vs Windows file path limits

On 2023-Jul-08, Andrew Dunstan wrote:

# Running: pg_basebackup -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/backup/tarbackup2 -h C:/tools/nmsys64/tmp/63ohSgsh21 -p 54699 --checkpoint fast --no-sync -Ft
WARNING: aborting backup due to backend exiting before pg_backup_stop was called
pg_basebackup: error: could not initiate base backup: ERROR: could not get junction for "./pg_replslot": More data is available.

Why not patch pgreadlink to use the method recommended by Microsoft,
that DeviceIoControl() is called first with a NULL reparseBuffer to
determine the size needed, then a second time with a buffer of that
size?

https://learn.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-deviceiocontrol

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

#16Alexander Lakhin
exclusion@gmail.com
In reply to: Andrew Dunstan (#14)
Re: pg_basebackup check vs Windows file path limits

11.11.2023 18:18, Andrew Dunstan wrote:

Hmm, maybe we should be using File::Copy::move() instead of rename(). The docco for that says:

        If possible, move() will simply rename the file. Otherwise, it
        copies the file to the new location and deletes the original. If an
        error occurs during this copy-and-delete process, you may be left
        with a (possibly partial) copy of the file under the destination
        name.

Unfortunately, I've stumbled upon inability of File::Copy::move()
to move directories across filesystems, exactly as described here:
https://stackoverflow.com/questions/17628039/filecopy-move-directories-accross-drives-in-windows-not-working

(I'm sorry for not looking above rename() where this stated explicitly:
# On Windows use the short location to avoid path length issues.
# Elsewhere use $tempdir to avoid file system boundary issues with moving.
So this issue affects Windows only.)

Best regards,
Alexander

#17Andrew Dunstan
andrew@dunslane.net
In reply to: Alexander Lakhin (#16)
Re: pg_basebackup check vs Windows file path limits

On 2023-11-11 Sa 12:00, Alexander Lakhin wrote:

11.11.2023 18:18, Andrew Dunstan wrote:

Hmm, maybe we should be using File::Copy::move() instead of rename().
The docco for that says:

        If possible, move() will simply rename the file. Otherwise, it
        copies the file to the new location and deletes the original. If an
        error occurs during this copy-and-delete process, you may be left
        with a (possibly partial) copy of the file under the destination
        name.

Unfortunately, I've stumbled upon inability of File::Copy::move()
to move directories across filesystems, exactly as described here:
https://stackoverflow.com/questions/17628039/filecopy-move-directories-accross-drives-in-windows-not-working

(I'm sorry for not looking above rename() where this stated explicitly:
# On Windows use the short location to avoid path length issues.
# Elsewhere use $tempdir to avoid file system boundary issues with moving.
So this issue affects Windows only.)

*sigh*

A probable workaround is to use a temp directory on the same device the
test is building on. Just set it up and set your environment TEMPDIR to
point to it, and I think it will be OK (i.e. I havent tested it).

But that doesn't mean I'm not searching for a better solution. Maybe
Alvaro's suggestion nearby will help.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#18Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#15)
Re: pg_basebackup check vs Windows file path limits

On 2023-11-11 Sa 11:31, Alvaro Herrera wrote:

On 2023-Jul-08, Andrew Dunstan wrote:

# Running: pg_basebackup -D C:\\tools\\nmsys64\\home\\pgrunner\\bf\\root\\REL_16_STABLE\\pgsql.build/testrun/pg_basebackup/010_pg_basebackup/data/t_010_pg_basebackup_main_data/backup/tarbackup2 -h C:/tools/nmsys64/tmp/63ohSgsh21 -p 54699 --checkpoint fast --no-sync -Ft
WARNING: aborting backup due to backend exiting before pg_backup_stop was called
pg_basebackup: error: could not initiate base backup: ERROR: could not get junction for "./pg_replslot": More data is available.

Why not patch pgreadlink to use the method recommended by Microsoft,
that DeviceIoControl() is called first with a NULL reparseBuffer to
determine the size needed, then a second time with a buffer of that
size?

https://learn.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-deviceiocontrol

Hmm, here's what that page says - I can't see it saying what you're
suggesting here - am I missing something?:

|[in] nOutBufferSize|

The size of the output buffer, in bytes.

|[out, optional] lpBytesReturned|

A pointer to a variable that receives the size of the data stored in the
output buffer, in bytes.

If the output buffer is too small to receive any data, the call fails,
GetLastError
<https://learn.microsoft.com/en-us/windows/desktop/api/errhandlingapi/nf-errhandlingapi-getlasterror&gt;
returns *ERROR_INSUFFICIENT_BUFFER*, and /lpBytesReturned/ is zero.

If the output buffer is too small to hold all of the data but can hold
some entries, some drivers will return as much data as fits. In this
case, the call fails, GetLastError
<https://learn.microsoft.com/en-us/windows/desktop/api/errhandlingapi/nf-errhandlingapi-getlasterror&gt;
returns *ERROR_MORE_DATA*, and /lpBytesReturned/ indicates the amount of
data received. Your application should call *DeviceIoControl* again with
the same operation, specifying a new starting point.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#19Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Andrew Dunstan (#18)
Re: pg_basebackup check vs Windows file path limits

On 2023-Nov-13, Andrew Dunstan wrote:

size?

https://learn.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-deviceiocontrol

Hmm, here's what that page says - I can't see it saying what you're
suggesting here - am I missing something?:

I don't think so. I think I just confused myself. Reading the docs it
appears that other Windows APIs work as I described, but not this one.

Anyway, after looking at it a bit more, I realized that this code uses
MAX_PATH as basis for its buffer's length limit -- and apparently on
Windows that's only 260, much shorter than MAXPGPATH (1024) which our
own code uses to limit the buffers given to readlink(). So maybe fixing
this is just a matter of doing s/MAX_PATH/MAXPGPATH/ in dirmod.c.

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/

#20Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#19)
Re: pg_basebackup check vs Windows file path limits

On 2023-11-15 We 06:34, Alvaro Herrera wrote:

On 2023-Nov-13, Andrew Dunstan wrote:

size?

https://learn.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-deviceiocontrol

Hmm, here's what that page says - I can't see it saying what you're
suggesting here - am I missing something?:

I don't think so. I think I just confused myself. Reading the docs it
appears that other Windows APIs work as I described, but not this one.

Anyway, after looking at it a bit more, I realized that this code uses
MAX_PATH as basis for its buffer's length limit -- and apparently on
Windows that's only 260, much shorter than MAXPGPATH (1024) which our
own code uses to limit the buffers given to readlink(). So maybe fixing
this is just a matter of doing s/MAX_PATH/MAXPGPATH/ in dirmod.c.

I'll test it.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com