You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Quentin Smith <qu...@mit.edu> on 2019/06/26 02:52:28 UTC
svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
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.
Cheers,
--Quentin
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by Nathan Hartman <ha...@gmail.com>.
On Thu, Jun 27, 2019 at 5:20 AM Branko Čibej <br...@apache.org> wrote:
> > On 26.06.2019 20:57, Branko Čibej wrote:
> > Turns out that APR is already doing this _almost_ correctly. The kind of
> > change linked to above would fit in APR quite well. The "problem" is
> > that Subversion currently doesn't use the "new" APR functions for
> > sync/datasync. So my suggestion for the fix would be twofold:
> >
> > * Enhance the sync functionality in APR (trunk, 1.7 and possibly 1.6)
> > * Change Subversion to use APR's sync functions
> >
> > I'll look into this.
>
> Interesting ... "man fcntl" on macOS does *not* mention ENOTTY at all.
> So everyone who's currently implementing this fix is relying on an
> undocumented feature of the filesystem. :)
Sounds like a BUG, either in macOS, Darwin, APFS, or the man page.
Sadly it's not obvious where to report it.
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by Branko Čibej <br...@apache.org>.
On 27.06.2019 10:56, Branko Čibej wrote:
> On 26.06.2019 20:57, Branko Čibej wrote:
>> On 26.06.2019 18:41, Nathan Hartman wrote:
>>> On Wed, Jun 26, 2019 at 11:49 AM Branko Čibej <brane@apache.org
>>> <ma...@apache.org>> wrote:
>>>
>>> On 26.06.2019 04:52, Quentin Smith wrote:
>>> > 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:
>>>
>>>
>>> (snip)
>>>
>>> > 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.)
>>>
>>>
>>> (snip)
>>>
>>> Last but not least, I do wish filesystems were consistent in their
>>> implementation ... what on earth is ENOTTY doing here?
>>>
>>> -- Brane
>>>
>>>
>>> Looks like libuv encountered similar issues and applied similar
>>> handling to that in SQLite:
>>>
>>> https://github.com/libuv/libuv/commit/5b0e1d75a207bb1c662f4495c0d8ba1e81a5bb7d
>>>
>> Something tell me this should be solved in APR ...
>
> Turns out that APR is already doing this _almost_ correctly. The kind of
> change linked to above would fit in APR quite well. The "problem" is
> that Subversion currently doesn't use the "new" APR functions for
> sync/datasync. So my suggestion for the fix would be twofold:
>
> * Enhance the sync functionality in APR (trunk, 1.7 and possibly 1.6)
> * Change Subversion to use APR's sync functions
>
> I'll look into this.
Interesting ... "man fcntl" on macOS does *not* mention ENOTTY at all.
So everyone who's currently implementing this fix is relying on an
undocumented feature of the filesystem. :)
That's fun.
-- Brane
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by Branko Čibej <br...@apache.org>.
On 26.06.2019 20:57, Branko Čibej wrote:
> On 26.06.2019 18:41, Nathan Hartman wrote:
>> On Wed, Jun 26, 2019 at 11:49 AM Branko Čibej <brane@apache.org
>> <ma...@apache.org>> wrote:
>>
>> On 26.06.2019 04:52, Quentin Smith wrote:
>> > 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:
>>
>>
>> (snip)
>>
>> > 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.)
>>
>>
>> (snip)
>>
>> Last but not least, I do wish filesystems were consistent in their
>> implementation ... what on earth is ENOTTY doing here?
>>
>> -- Brane
>>
>>
>> Looks like libuv encountered similar issues and applied similar
>> handling to that in SQLite:
>>
>> https://github.com/libuv/libuv/commit/5b0e1d75a207bb1c662f4495c0d8ba1e81a5bb7d
>>
> Something tell me this should be solved in APR ...
Turns out that APR is already doing this _almost_ correctly. The kind of
change linked to above would fit in APR quite well. The "problem" is
that Subversion currently doesn't use the "new" APR functions for
sync/datasync. So my suggestion for the fix would be twofold:
* Enhance the sync functionality in APR (trunk, 1.7 and possibly 1.6)
* Change Subversion to use APR's sync functions
I'll look into this.
-- Brane
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by Branko Čibej <br...@apache.org>.
On 26.06.2019 18:41, Nathan Hartman wrote:
> On Wed, Jun 26, 2019 at 11:49 AM Branko Čibej <brane@apache.org
> <ma...@apache.org>> wrote:
>
> On 26.06.2019 04:52, Quentin Smith wrote:
> > 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:
>
>
> (snip)
>
> > 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.)
>
>
> (snip)
>
> Last but not least, I do wish filesystems were consistent in their
> implementation ... what on earth is ENOTTY doing here?
>
> -- Brane
>
>
> Looks like libuv encountered similar issues and applied similar
> handling to that in SQLite:
>
> https://github.com/libuv/libuv/commit/5b0e1d75a207bb1c662f4495c0d8ba1e81a5bb7d
>
Something tell me this should be solved in APR ...
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Jun 26, 2019 at 11:49 AM Branko Čibej <br...@apache.org> wrote:
> On 26.06.2019 04:52, Quentin Smith wrote:
> > 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:
(snip)
> 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.)
(snip)
Last but not least, I do wish filesystems were consistent in their
> implementation ... what on earth is ENOTTY doing here?
>
> -- Brane
Looks like libuv encountered similar issues and applied similar handling to
that in SQLite:
https://github.com/libuv/libuv/commit/5b0e1d75a207bb1c662f4495c0d8ba1e81a5bb7d
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by 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
Posted by 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.
Re: svn_io_file_flush_to_disk fails on AFS filesystems on Darwin
Posted by 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
Posted by Branko Čibej <br...@apache.org>.
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?
-- Brane