Content-Type: multipart/alternative;
 boundary="------------vud4dulIcqiNnY5vWk75CpyX"
Message-ID: <9293e623-1d5e-0bd3-9b61-6129821b6f42@dunslane.net>
Date: Sat, 11 Nov 2023 10:18:51 -0500
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: pg_basebackup check vs Windows file path limits
Content-Language: en-US
To: Alexander Lakhin <exclusion@gmail.com>,
 Daniel Gustafsson <daniel@yesql.se>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <666ac55b-3400-fb2c-2cea-0281bf36a53c@dunslane.net>
 <76f33a1f-ddce-80a6-6f39-fefa0668f4a6@dunslane.net>
 <3AC47E65-02A0-42BF-8A3E-B38FD09FE702@yesql.se>
 <94341cbe-9491-60e9-f01a-7ed770295cfa@dunslane.net>
 <81023744-772d-889b-99b8-bd1d58bd761b@dunslane.net>
 <67093EFC-3800-48B0-9E20-82A37A5F2A7B@yesql.se>
 <0c1c289c-f55a-1cc3-4ce1-44d7e8353e70@dunslane.net>
 <3B1D0F8A-DB97-462A-937F-73B3FCFE2713@yesql.se>
 <a7496caf-0e5b-d973-bc60-3a4308afd03c@dunslane.net>
 <8e3130c2-5c51-5cd5-9f0d-b0cd938bcd6b@dunslane.net>
 <de60e1b0-e2c9-c61a-c65d-c5b288348fa2@dunslane.net>
 <13663265-cac2-ec5d-d378-0dd44a895f8e@gmail.com>
From: Andrew Dunstan <andrew@dunslane.net>
In-Reply-To: <13663265-cac2-ec5d-d378-0dd44a895f8e@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/9293e623-1d5e-0bd3-9b61-6129821b6f42%40dunslane.net>
Precedence: bulk

This is a multi-part message in MIME format.
--------------vud4dulIcqiNnY5vWk75CpyX
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


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

--------------vud4dulIcqiNnY5vWk75CpyX
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p><br>
    </p>
    <p><br>
    </p>
    <p>Hi, Alexander</p>
    <p><br>
    </p>
    <div class="moz-cite-prefix">On 2023-11-11 Sa 08:00, Alexander
      Lakhin wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:13663265-cac2-ec5d-d378-0dd44a895f8e@gmail.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">Hello Andrew,<br>
        <br>
        08.07.2023 18:52, Andrew Dunstan wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:de60e1b0-e2c9-c61a-c65d-c5b288348fa2@dunslane.net">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8">
        <blockquote type="cite"
          cite="mid:8e3130c2-5c51-5cd5-9f0d-b0cd938bcd6b@dunslane.net">
          <pre>Since this test is passing on HEAD which has slightly shorter paths, I'm wondering if we should change this:
</pre>
          <blockquote>rename("$pgdata/pg_replslot",
            "$tempdir/pg_replslot")<br>
              or BAIL_OUT "could not move $pgdata/pg_replslot";<br>
            dir_symlink("$tempdir/pg_replslot", "$pgdata/pg_replslot")<br>
              or BAIL_OUT "could not symlink to $pgdata/pg_replslot";<br>
          </blockquote>
          <p>to use the much shorter $sys_tempdir created a few lines
            below.</p>
        </blockquote>
        <p>Pushed a tested fix along those lines.</p>
      </blockquote>
      <br>
      Today I've started up my Windows VM to run some tests and
      discovered a test<br>
      failure caused by that fix (e213de8e7):<br>
      &gt;meson test<br>
      Ok:                 246<br>
      Expected Fail:      0<br>
      Fail:               1<br>
      Unexpected Pass:    0<br>
      Skipped:            14<br>
      Timeout:            0<br>
      <br>
      ...\010_pg_basebackup\log\regress_log_010_pg_basebackup.txt
      contains:<br>
      [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<br>
      <br>
      With a diagnostic print added before rename() in
      010_pg_basebackup.pl, I see:<br>
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")<br>
      That is, I have the postgres source tree and the user tempdir
      placed on<br>
      different disks.<br>
      <br>
      perldoc on rename() says that it usually doesn't work across
      filesystem<br>
      boundaries, so I think it's not a Windows-specific issue.<br>
      <br>
      <br>
    </blockquote>
    <p><br>
    </p>
    <p>Hmm, maybe we should be using File::Copy::move() instead of
      rename(). The docco for that says:</p>
    <pre>        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.</pre>
    <p><br>
    </p>
    <p>Can you try it out?</p>
    <p><br>
    </p>
    <p>cheers</p>
    <p><br>
    </p>
    <p>andrew<br>
    </p>
    <pre class="moz-signature" signature-switch-id="d0437855-2267-4610-80b3-83167ec45b0b" cols="72">--
Andrew Dunstan
EDB: <a class="moz-txt-link-freetext" href="https://www.enterprisedb.com">https://www.enterprisedb.com</a>
</pre>
  </body>
</html>

--------------vud4dulIcqiNnY5vWk75CpyX--