Quantcast

[PATCH] transaction: enable hardlink backups for non-windows systems

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] transaction: enable hardlink backups for non-windows systems

Jeroen Vaelen
# HG changeset patch
# User Jeroen Vaelen <[hidden email]>
# Date 1487064458 28800
#      Tue Feb 14 01:27:38 2017 -0800
# Node ID c7fb7ac39a12c8683518bb7db7e1a93346e017e0
# Parent  a0e3d808690d57d1c9dff840e0b8ee099526397b
transaction: enable hardlink backups for non-windows systems

07a92bbd02e5 disabled hardlink backups entirely because they can cause trouble
with CIFS on Windows (see issue 4546). This changeset limits that restriction
to Windows systems. Ideally we check for CIFS, because e.g. NTFS does support
hardlinks. But this at least gives us cheaper transactional backups for posix,
which is a step forward.

Note: the test changes were originally introduced in 07a92bbd02e5.

diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1084,9 +1084,9 @@
         if checkambig:
             oldstat = checkambig and filestat(dest)
         unlink(dest)
-    # hardlinks are problematic on CIFS, quietly ignore this flag
-    # until we find a way to work around it cleanly (issue4546)
-    if False and hardlink:
+    # quietly ignore the hardlink flag on Windows due to CIFS limitations
+    # (see discussion on issue 4546)
+    if hardlink and pycompat.osname != 'nt':
         try:
             oslink(src, dest)
             return
diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
--- a/tests/test-hardlinks.t
+++ b/tests/test-hardlinks.t
@@ -166,7 +166,7 @@
   1 r2/.hg/store/00manifest.i
   1 r2/.hg/store/data/d1/f2.i
   2 r2/.hg/store/data/f1.i
-  1 r2/.hg/store/fncache
+  2 r2/.hg/store/fncache
 
   $ hg -R r2 verify
   checking changesets
@@ -191,7 +191,7 @@
   1 r2/.hg/store/00manifest.i
   1 r2/.hg/store/data/d1/f2.i
   1 r2/.hg/store/data/f1.i
-  1 r2/.hg/store/fncache
+  2 r2/.hg/store/fncache
 
 
   $ cd r3
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] transaction: enable hardlink backups for non-windows systems

Augie Fackler-2
On Tue, Feb 14, 2017 at 01:31:02AM -0800, Jeroen Vaelen wrote:

> # HG changeset patch
> # User Jeroen Vaelen <[hidden email]>
> # Date 1487064458 28800
> #      Tue Feb 14 01:27:38 2017 -0800
> # Node ID c7fb7ac39a12c8683518bb7db7e1a93346e017e0
> # Parent  a0e3d808690d57d1c9dff840e0b8ee099526397b
> transaction: enable hardlink backups for non-windows systems
>
> 07a92bbd02e5 disabled hardlink backups entirely because they can cause trouble
> with CIFS on Windows (see issue 4546). This changeset limits that restriction
> to Windows systems. Ideally we check for CIFS, because e.g. NTFS does support
> hardlinks. But this at least gives us cheaper transactional backups for posix,
> which is a step forward.

I'm hesitant to take this because as of 10.12 macOS is pushing people
towards CIFS instead of AFP, so this issue feels more likely to come
up there. Also, isn't it possible to mount CIFS volumes in the
filesystem space on Linux? Do we have any reason for confidence this
is only a problem in the Windows CIFS client, and not a problem in the
server?

Thanks!

>
> Note: the test changes were originally introduced in 07a92bbd02e5.
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -1084,9 +1084,9 @@
>          if checkambig:
>              oldstat = checkambig and filestat(dest)
>          unlink(dest)
> -    # hardlinks are problematic on CIFS, quietly ignore this flag
> -    # until we find a way to work around it cleanly (issue4546)
> -    if False and hardlink:
> +    # quietly ignore the hardlink flag on Windows due to CIFS limitations
> +    # (see discussion on issue 4546)
> +    if hardlink and pycompat.osname != 'nt':
>          try:
>              oslink(src, dest)
>              return
> diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
> --- a/tests/test-hardlinks.t
> +++ b/tests/test-hardlinks.t
> @@ -166,7 +166,7 @@
>    1 r2/.hg/store/00manifest.i
>    1 r2/.hg/store/data/d1/f2.i
>    2 r2/.hg/store/data/f1.i
> -  1 r2/.hg/store/fncache
> +  2 r2/.hg/store/fncache
>
>    $ hg -R r2 verify
>    checking changesets
> @@ -191,7 +191,7 @@
>    1 r2/.hg/store/00manifest.i
>    1 r2/.hg/store/data/d1/f2.i
>    1 r2/.hg/store/data/f1.i
> -  1 r2/.hg/store/fncache
> +  2 r2/.hg/store/fncache
>
>
>    $ cd r3
> _______________________________________________
> Mercurial-devel mailing list
> [hidden email]
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] transaction: enable hardlink backups for non-windows systems

timeless
Augie Fackler wrote:
> I'm hesitant to take this because as of 10.12 macOS is pushing people
> towards CIFS instead of AFP,

Interesting

> so this issue feels more likely to come up there.

> Also, isn't it possible to mount CIFS volumes in the filesystem space on Linux?

I was going to mention CIFS for Linux.

> Do we have any reason for confidence this is only a problem in the Windows CIFS client,
> and not a problem in the server?

I can't imagine this being the case. It's incredibly rare for the
linux CIFS clients to be significantly better than the Windows
implementations.

Out of curiosity, does sshfs handle this stuff correctly? :)
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] transaction: enable hardlink backups for non-windows systems

Jeroen Vaelen
In reply to this post by Augie Fackler-2
Sorry for the delay in reply.

Confidence in stable posix implementations could come from:

- hardlink backups being the default for nearly 10 years before the previously
  mentioned patch disabled them after a series of bug reports coming
  exclusively from Windows users.
- generally more dependencies on hardlinks on such systems by different
  tools which make cache coherency bugs (theory) like issue 4546 less
  likely.
 
Getting a repro for this bug has been tough. Do you have any ideas for
alternative directions here?

Excerpts from Augie Fackler's message of 2017-02-14 10:39:00 -0500:

> On Tue, Feb 14, 2017 at 01:31:02AM -0800, Jeroen Vaelen wrote:
> > # HG changeset patch
> > # User Jeroen Vaelen <[hidden email]>
> > # Date 1487064458 28800
> > #      Tue Feb 14 01:27:38 2017 -0800
> > # Node ID c7fb7ac39a12c8683518bb7db7e1a93346e017e0
> > # Parent  a0e3d808690d57d1c9dff840e0b8ee099526397b
> > transaction: enable hardlink backups for non-windows systems
> >
> > 07a92bbd02e5 disabled hardlink backups entirely because they can cause trouble
> > with CIFS on Windows (see issue 4546). This changeset limits that restriction
> > to Windows systems. Ideally we check for CIFS, because e.g. NTFS does support
> > hardlinks. But this at least gives us cheaper transactional backups for posix,
> > which is a step forward.
>
> I'm hesitant to take this because as of 10.12 macOS is pushing people
> towards CIFS instead of AFP, so this issue feels more likely to come
> up there. Also, isn't it possible to mount CIFS volumes in the
> filesystem space on Linux? Do we have any reason for confidence this
> is only a problem in the Windows CIFS client, and not a problem in the
> server?
>
> Thanks!
>
> >
> > Note: the test changes were originally introduced in 07a92bbd02e5.
> >
> > diff --git a/mercurial/util.py b/mercurial/util.py
> > --- a/mercurial/util.py
> > +++ b/mercurial/util.py
> > @@ -1084,9 +1084,9 @@
> >          if checkambig:
> >              oldstat = checkambig and filestat(dest)
> >          unlink(dest)
> > -    # hardlinks are problematic on CIFS, quietly ignore this flag
> > -    # until we find a way to work around it cleanly (issue4546)
> > -    if False and hardlink:
> > +    # quietly ignore the hardlink flag on Windows due to CIFS limitations
> > +    # (see discussion on issue 4546)
> > +    if hardlink and pycompat.osname != 'nt':
> >          try:
> >              oslink(src, dest)
> >              return
> > diff --git a/tests/test-hardlinks.t b/tests/test-hardlinks.t
> > --- a/tests/test-hardlinks.t
> > +++ b/tests/test-hardlinks.t
> > @@ -166,7 +166,7 @@
> >    1 r2/.hg/store/00manifest.i
> >    1 r2/.hg/store/data/d1/f2.i
> >    2 r2/.hg/store/data/f1.i
> > -  1 r2/.hg/store/fncache
> > +  2 r2/.hg/store/fncache
> >
> >    $ hg -R r2 verify
> >    checking changesets
> > @@ -191,7 +191,7 @@
> >    1 r2/.hg/store/00manifest.i
> >    1 r2/.hg/store/data/d1/f2.i
> >    1 r2/.hg/store/data/f1.i
> > -  1 r2/.hg/store/fncache
> > +  2 r2/.hg/store/fncache
> >
> >
> >    $ cd r3
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] transaction: enable hardlink backups for non-windows systems

Yuya Nishihara
On Wed, 15 Feb 2017 20:34:32 +0000, Jeroen Vaelen wrote:
> Confidence in stable posix implementations could come from:
>
> - hardlink backups being the default for nearly 10 years before the previously
>   mentioned patch disabled them after a series of bug reports coming
>   exclusively from Windows users.

No. This particular code path was introduced in 3.3-rc, and we got the bug
report soon after the release.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] transaction: enable hardlink backups for non-windows systems

Sean Farley-3
In reply to this post by Augie Fackler-2
Augie Fackler <[hidden email]> writes:

> On Tue, Feb 14, 2017 at 01:31:02AM -0800, Jeroen Vaelen wrote:
>> # HG changeset patch
>> # User Jeroen Vaelen <[hidden email]>
>> # Date 1487064458 28800
>> #      Tue Feb 14 01:27:38 2017 -0800
>> # Node ID c7fb7ac39a12c8683518bb7db7e1a93346e017e0
>> # Parent  a0e3d808690d57d1c9dff840e0b8ee099526397b
>> transaction: enable hardlink backups for non-windows systems
>>
>> 07a92bbd02e5 disabled hardlink backups entirely because they can cause trouble
>> with CIFS on Windows (see issue 4546). This changeset limits that restriction
>> to Windows systems. Ideally we check for CIFS, because e.g. NTFS does support
>> hardlinks. But this at least gives us cheaper transactional backups for posix,
>> which is a step forward.
>
> I'm hesitant to take this because as of 10.12 macOS is pushing people
> towards CIFS instead of AFP

Actually, it's since 10.9:

http://appleinsider.com/articles/13/06/11/apple-shifts-from-afp-file-sharing-to-smb2-in-os-x-109-mavericks
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Loading...