Horton

Thread

Commits

  1. Skip pg_baseback long filename test if path too long on Windows

  1. pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-02T13:15:17Z

    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

  2. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-03T14:12:49Z

    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

  3. Re: pg_basebackup check vs Windows file path limits

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-03T14:16:02Z

    > 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

  4. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-03T15:18:25Z

    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

  5. Re: pg_basebackup check vs Windows file path limits

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-03T15:19:51Z

    > 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

  6. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-04T18:19:46Z

    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

  7. Re: pg_basebackup check vs Windows file path limits

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-04T20:54:54Z

    > 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

  8. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-05T12:49:39Z

    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

  9. Re: pg_basebackup check vs Windows file path limits

    Daniel Gustafsson <daniel@yesql.se> — 2023-07-06T13:50:57Z

    > 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

  10. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-06T16:38:03Z

    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

  11. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-08T13:15:21Z

    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&dt=2023-07-07%2022%3A03%3A06>, 
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

  12. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-07-08T15:52:11Z

    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&dt=2023-07-07%2022%3A03%3A06>, 
> 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

  13. Re: pg_basebackup check vs Windows file path limits

    Alexander Law <exclusion@gmail.com> — 2023-11-11T13:00:00Z

    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

  14. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-11-11T15:18:51Z

    
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

  15. Re: pg_basebackup check vs Windows file path limits

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-11-11T16:31:39Z

    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/

  16. Re: pg_basebackup check vs Windows file path limits

    Alexander Law <exclusion@gmail.com> — 2023-11-11T17:00:01Z

    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

  17. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-11-12T14:09:27Z

    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

  18. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-11-13T13:27:39Z

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

  19. Re: pg_basebackup check vs Windows file path limits

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2023-11-15T11:34:58Z

    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/

  20. Re: pg_basebackup check vs Windows file path limits

    Andrew Dunstan <andrew@dunslane.net> — 2023-11-15T17:15:32Z

    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