Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin

[Branko Čibej <br...@apache.org>]

On 26.06.2019 17:48, Branko Čibej wrote:
> On 26.06.2019 04:52, Quentin Smith wrote:
>> Hi,
>>
>> svn_io_file_flush_to_disk in subversion/libsvn_subr/io.c fails to
>> flush files on AFS filesystems on Darwin. The user-visible experience is:
>>
>> $ svn commit
>> svn: E000025: Commit failed (details follow):
>> svn: E000025: Can't write '/afs/path/db/txn-current' atomically
>> svn: E000025: Can't flush file '/afs/path/db/svn-CNFY6N' to disk:
>> Inappropriate ioctl for device
>>
>> This is because Darwin defines F_FULLFSYNC, which on AFS (and maybe
>> other filesystems?) returns ENOTTY.
>>
>> The code ignores EINVAL with a comment about filesystems that don't
>> support it; evidently on Darwin this also needs to include ENOTTY.
>>
>> (Also, I suspect it should fall back from fcntl to fsync, since I
>> suspect fsync /does/ work on AFS filesystems.)
>>
>> I repro'd this with svn 1.10.3 (r1842928), but inspection of trunk
>> confirms the code has not changed.
> First of all, thanks for the repor.
>
> Please provide a standalone reproduction script, using our command-line
> client.
>
> Last but not least, I do wish filesystems were consistent in their
> implementation ... what on earth is ENOTTY doing here?

Sometimes it's good to have ages old browser tabs lying around. This is 
now fixed in apr_file_datasync on APR trunk and the 1.7.x branch.

-- Brane

https://svn.apache.org/viewvc?view=revision&revision=1883341

Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin

[Nathan Hartman <ha...@gmail.com>]

On Thu, Nov 12, 2020 at 12:25 PM Branko Čibej <br...@apache.org> wrote:
>
> On 12.11.2020 17:38, Branko Čibej wrote:
> > On 26.06.2019 17:48, Branko Čibej wrote:
> >> On 26.06.2019 04:52, Quentin Smith wrote:
> >>> Hi,
> >>>
> >>> svn_io_file_flush_to_disk in subversion/libsvn_subr/io.c fails to
> >>> flush files on AFS filesystems on Darwin. The user-visible
> >>> experience is:
> >>>
> >>> $ svn commit
> >>> svn: E000025: Commit failed (details follow):
> >>> svn: E000025: Can't write '/afs/path/db/txn-current' atomically
> >>> svn: E000025: Can't flush file '/afs/path/db/svn-CNFY6N' to disk:
> >>> Inappropriate ioctl for device
> >>>
> >>> This is because Darwin defines F_FULLFSYNC, which on AFS (and maybe
> >>> other filesystems?) returns ENOTTY.
> >>>
> >>> The code ignores EINVAL with a comment about filesystems that don't
> >>> support it; evidently on Darwin this also needs to include ENOTTY.
> >>>
> >>> (Also, I suspect it should fall back from fcntl to fsync, since I
> >>> suspect fsync /does/ work on AFS filesystems.)
> >>>
> >>> I repro'd this with svn 1.10.3 (r1842928), but inspection of trunk
> >>> confirms the code has not changed.
> >> First of all, thanks for the repor.
> >>
> >> Please provide a standalone reproduction script, using our command-line
> >> client.
> >>
> >> Last but not least, I do wish filesystems were consistent in their
> >> implementation ... what on earth is ENOTTY doing here?
> >
> > Sometimes it's good to have ages old browser tabs lying around. This
> > is now fixed in apr_file_datasync on APR trunk and the 1.7.x branch.
> >
> > -- Brane
> >
> > https://svn.apache.org/viewvc?view=revision&revision=1883341
>
> And r1883355 makes Subversion use the right APR functions.

I just realized that this nomination:

[[[

 * r1883355
   Use the APR-1.4+ API for flushing file contents to disk.
   Justification:
     Reduce code duplication between APR and SVN.
   Votes:
     +1: brane

]]]

isn't a mere refactoring, as I thought previously, but rather is a fix
for the above flush-to-disk issue on AFS filesystems--assuming, of
course, that SVN is built with APR >= 1.7. (Perhaps this nomination
deserves a Notes section to point that out.)

(I will probably vote +1 for this, pending to check, since we support
building with APR >= 1.4, if there is a chance of some regression
being introduced when SVN is built with APR < 1.7.)

Nathan

Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin

[Branko Čibej <br...@apache.org>]

On 12.11.2020 17:38, Branko Čibej wrote:
> On 26.06.2019 17:48, Branko Čibej wrote:
>> On 26.06.2019 04:52, Quentin Smith wrote:
>>> Hi,
>>>
>>> svn_io_file_flush_to_disk in subversion/libsvn_subr/io.c fails to
>>> flush files on AFS filesystems on Darwin. The user-visible 
>>> experience is:
>>>
>>> $ svn commit
>>> svn: E000025: Commit failed (details follow):
>>> svn: E000025: Can't write '/afs/path/db/txn-current' atomically
>>> svn: E000025: Can't flush file '/afs/path/db/svn-CNFY6N' to disk:
>>> Inappropriate ioctl for device
>>>
>>> This is because Darwin defines F_FULLFSYNC, which on AFS (and maybe
>>> other filesystems?) returns ENOTTY.
>>>
>>> The code ignores EINVAL with a comment about filesystems that don't
>>> support it; evidently on Darwin this also needs to include ENOTTY.
>>>
>>> (Also, I suspect it should fall back from fcntl to fsync, since I
>>> suspect fsync /does/ work on AFS filesystems.)
>>>
>>> I repro'd this with svn 1.10.3 (r1842928), but inspection of trunk
>>> confirms the code has not changed.
>> First of all, thanks for the repor.
>>
>> Please provide a standalone reproduction script, using our command-line
>> client.
>>
>> Last but not least, I do wish filesystems were consistent in their
>> implementation ... what on earth is ENOTTY doing here?
>
> Sometimes it's good to have ages old browser tabs lying around. This 
> is now fixed in apr_file_datasync on APR trunk and the 1.7.x branch.
>
> -- Brane
>
> https://svn.apache.org/viewvc?view=revision&revision=1883341

And r1883355 makes Subversion use the right APR functions.