Add option to ignore acl and xattr for performance reasons

[ocofon]

I recently switched my backups on some heavily loaded servers with slow disks from normal rsync to borg v1.1.0rcX.
On some server the backup need much more time than before.
It seems the reason for the slow backup is that borg causes a lot more syscalls for reading an unchanged file than rsync does.
While rsync uses a single lstat for any unchanged file borg uses 10 syscalls.

[pid 26202] lstat("testdir/test7522", {st_mode=S_IFREG|0644, st_size=2, ...}) = 0
[pid 26202] open("testdir/test7522", O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC) = 6
[pid 26202] ioctl(6, FS_IOC32_GETFLAGS or FS_IOC_GETFLAGS, 0x7ffdd66fd110) = 0
[pid 26202] close(6)                    = 0
[pid 26202] llistxattr("testdir/test7522", "", 4096) = 0
[pid 26202] open("testdir/test7522", O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC) = 6
[pid 26202] ioctl(6, FS_IOC32_GETFLAGS or FS_IOC_GETFLAGS, 0x7ffdd66fca70) = 0
[pid 26202] close(6)                    = 0
[pid 26202] getxattr("testdir/test7522", "system.posix_acl_access", 0x0, 0) = -1 ENODATA (No data available)
[pid 26202] getxattr("testdir/test7522", "system.posix_acl_default", 0x0, 0) = -1 ENODATA (No data available)

I patched borg-1.1.0rc3 with the following patch to disable the read of xattr, acl and inode flags:

--- a/src/borg/archive.py
+++ b/src/borg/archive.py
@@ -841,7 +841,7 @@ def stat_ext_attrs(self, st, path):

     def stat_attrs(self, st, path):
         attrs = self.stat_simple_attrs(st)
-        attrs.update(self.stat_ext_attrs(st, path))
+        #attrs.update(self.stat_ext_attrs(st, path))
         return attrs

     @contextmanager
diff --git a/src/borg/archiver.py b/src/borg/archiver.py
index f6b177d3..d9f02352 100644
--- a/src/borg/archiver.py
+++ b/src/borg/archiver.py
@@ -561,10 +561,10 @@ def _process(self, archive, cache, matcher, exclude_caches, exclude_if_present,
             recurse = restrict_dev is None or st.st_dev == restrict_dev
             status = None
             # Ignore if nodump flag is set
-            with backup_io('flags'):
-                if get_flags(path, st) & stat.UF_NODUMP:
-                    self.print_file_status('x', path)
-                    return
+            #with backup_io('flags'):
+            #    if get_flags(path, st) & stat.UF_NODUMP:
+            #        self.print_file_status('x', path)
+            #        return
             if stat.S_ISREG(st.st_mode):
                 if not dry_run:
                     status = archive.process_file(path, st, cache, self.ignore_inode)

With the patched version there is only one syscall for an unchanged file in borg.

[pid 26333] lstat("testdir/test3378", {st_mode=S_IFREG|0644, st_size=2, ...}) = 0
[pid 26333] lstat("testdir/test3379", {st_mode=S_IFREG|0644, st_size=2, ...}) = 0
[pid 26333] lstat("testdir/test3380", {st_mode=S_IFREG|0644, st_size=2, ...}) = 0

The time needed for a backup is now comparable to the old rsync backup.
It would be great if borg would have an option to disable the reading of xattr and acl when there is no need for backup of those.
Maybe it is also possible to skip reading xattr on unchanged files because when xattrs are updated the ctime of the file changes.
Also the get_flags() is called twice on every file which is probably not necessary.

[enkore]

The syscalls themselves are not the problem, rather that the xattr code is very slow. I think it is far better to fix that instead of adding yet-another option. get_flags is also not so good (on Linux, where a synthetic implementation is used).

[ThomasWaldmann]

@ocofon we need to consider that borg always does full backups. That means that the archive created in the repository contains full metadata for all input files and references all the content chunks.

So, even if ctime or mtime, size and inode number indicate that a file did not change, that full metadata information must come from somewhere. Currently, we do not store xattrs, acls or flags in the files cache, so they can only come from the filesystem

If we would store these into files cache also, that would mean:

• we could not use mtime any more, because it only triggers on content change
• we could only use ctime, assuming ctime always changes on flags/xattrs/acls change
• the already big files cache would get even bigger

[ocofon]

@ThomasWaldmann i agree, thats why i think a command line switch or another environment variable to disable it for people who don't need it would be the best solution.
The time needed for a backup of 15 million files on the slowest server decreased from about 20h to 8h with the patched version.
For 15 million files we are talking about 135 millionen additional syscalls without a single xattr or acl set on the filesystem.

Its also seems that calling the acl_get() function would be completly unnecessary (at least on linux) when the exclusion of "system.posix_acl_*" would be removed from xattr.listxattr.
xattr.get_all() would add system.posix_acl_access and system.posix_acl_default to the backup and the values would be restored on extract.
This would save 2 syscalls if no acls are set and caching the result of get_flags() (which is currently called twice on a file) would save another 3.

[enkore]

Syscalls are irrelevant, see above.

[ThomasWaldmann]

One get_flags() call that just checked for the NODUMP flag will be gone in 1.1.1 (by default, except when you give --exclude-nodump). You can get rid of the other one when giving --nobsdflags.

This was needed for other reasons (not this ticket), just wanted to mention it.

[ThomasWaldmann]

@ocofon could you benchmark with borg 1.1.2+ code to demonstrate the difference?

Guess the worst case would be:

• borg create using --nobsdflags (and not using --exclude-nodump, of course)
• 2nd+ backup, so everything already is in borg's files cache only the metadata comes from disk
• with/without getting/processing/archiving xattrs
• with/without getting/processing/archiving acls
• just getting xattrs/acls, but not processing/archiving them
• with lots of small files
• HDD (except if you can already see a vast difference on SSD also)
• with cold OS block / fs cache

[ThomasWaldmann]

Guess the performance tests should be re-done with the code from PR #3918.

[ThomasWaldmann]

I am closing this in favour of #3955.