Performance improvements on distributed file systems by nh2 · Pull Request #43 · bup/bup

nh2 · 2018-06-01T15:22:35Z

Here are 4 patches that I've been using in the past 8 months to dramatically speed up bup when run against a distributed file system like GlusterFS (and actually makes it possible to use it in such scenarios).

I've shortly discussed them on the bup IRC channel in the past.

Please see the commit descriptions for details.

The use of scandir and the fix for st_ino are reasonably well tested (over thousands of bup runs), the no-nonstat-metadata flag has not been tested as extensively.

One question:

Right now, when I run make test on Ubuntu 16.04, it fails with:

/home/niklas/src/bup/bup ftp
Warning: scandir support missing; install python-scandir to speed up directory traversals.
Makefile:197: recipe for target 'tmp-target-run-test-ftp' failed
make[1]: *** [tmp-target-run-test-ftp] Error 1
make[1]: Leaving directory '/home/niklas/src/bup'
! Program returned non-zero exit code (2)                           FAILED
WvTest: result code 2, total time 17.455s
./wvtest report t/tmp/test-log/*.log

/home/niklas/src/bup/bup ftp
Warning: scandir support missing; install python-scandir to speed up directory traversals.
Commands: ls cd pwd cat get mget help quit
! test-ftp:63   'Commands: ls cd pwd cat get mget help quit\n' == 'Warning: scandir support missing; install python-scandir to speed up directory traversals.\nCommands: ls cd pwd cat get mget help quit\n' FAILED

! Program returned non-zero exit code (2)                           FAILED

This is because I'm emitting the Warning: scandir support missing; install python-scandir to speed up directory traversals when scandir couldn't be found. I think having this warning is good because scandir really helps a huge deal with performance.

But Ubuntu 16.04 doesn't have python-scandir available, only >= 17.10 has it.

(How) should I modify the two failing tests so that they ignore if this warning is emitted?

Thanks!

This speeds up indexing and is useful when you don't care about xattr or ACL metadata, and is especially effective on distributed network file system mounts like GlusterFS, where each such syscall is a network roundtrip. Signed-off-by: Niklas Hambüchen <mail@nh2.me>

scandir() is a generator, so this makes bup index have getdents() and stat() syscalls interleaved just like `ls` and `rsync` do. Many file systems have optimisations for doing stat() on the results of getdents(); especially on networked file systems this can make a big performance difference. On GlusterFS, this change, together with turning off metadata (previous commit) results in a ~20x speedup of bup index. Signed-off-by: Niklas Hambüchen <mail@nh2.me>

Python < 3.6 thinks it's signed when in fact it's unsigned. This blows up in bup when scandir() is used on systems with large inodes (such as glusterfs). Signed-off-by: Niklas Hambüchen <mail@nh2.me>

Signed-off-by: Niklas Hambüchen <mail@nh2.me>

rlbdv · 2018-09-30T20:59:26Z

Thanks for the help. In accordance with HACKING I've redirected this pull request to the list for discussion: https://groups.google.com/d/msg/bup-list/SNNxvGtFtaA/j38p7jmcAgAJ

jmberg · 2020-11-18T21:34:08Z

+scandir = None
+try:
+    # scandir allows much faster directory traversals, but is
+    # only included in the stanard library from Python 3.3 upwards.


typo - 'standard'

jmberg · 2020-11-18T21:35:07Z

+    # only included in the stanard library from Python 3.3 upwards.
+    import scandir
+except ImportError:
+    log('Warning: scandir support missing; install python-scandir'


not really sure it's worth it ... after all, people who have happily been using bup would not really care too much, and there's no way to suppress this other than to actually install it ... that's a bit annoying?

OTOH, I guess there's precedent for it since we do similar things with xattr

However, then again, python 3.5 has os.scandir() with all the performance improvements incorporated, I think, so maybe just use that instead of the extra library?

jmberg · 2020-11-18T21:35:41Z

+                # We need to use xstat instead of entry.stat() because
+                # entry.stat() uses Python's normal floating-point mtimes.
+                st = xstat.lstat(n)
+                yield (n,st)


nit: maybe add a space after the , in the tuple

jmberg · 2020-11-18T21:36:00Z

-        except OSError as e:
-            add_error(Exception('%s: %s' % (resolve_parent(n), str(e))))
-            continue
+    for (n,st) in _filename_stat_generator('.'):


jmberg · 2020-11-18T21:36:09Z

+        for n in os.listdir(path):
+            try:
+                st = xstat.lstat(n)
+                yield (n,st)


jmberg · 2020-11-18T21:37:20Z

+                st = xstat.lstat(n)
+                yield (n,st)
+            except OSError as e:
+                add_error(Exception('%s: %s' % (resolve_parent(n), str(e))))


this is strange - you copied it from the one place that was already weird, with the Exception() think? normally add_error just takes a string

jmberg · 2020-11-18T21:40:13Z

+                st = xstat.lstat(n)
+                yield (n,st)
+            except OSError as e:
+                add_error(Exception('%s: %s' % (resolve_parent(n), str(e))))


same note here

jmberg · 2020-11-18T21:40:24Z

-        try:
-            st = xstat.lstat(n)
-        except OSError as e:
-            add_error(Exception('%s: %s' % (resolve_parent(n), str(e))))


yeah here was the odd use you removed ...

jmberg · 2020-11-18T21:42:22Z

+#   https://github.com/python/cpython/commit/0f6d73343d342c106cda2219ebb8a6f0c4bd9b3c
+#
+# To work around it, any `st_ino` as obtained from `os.stat()`
+# should be cast to unsigned using `thestat.st_ino & INO_FIX`.


wouldn't it make more sense to use it like a function?

ino_fix(st_ino)
and in the uncommon cases,

st_ino = ino_fix(st_ino)

nh2 added 4 commits June 1, 2018 17:16

Work around st_ino being wrong in Python.

701953d

Python < 3.6 thinks it's signed when in fact it's unsigned. This blows up in bup when scandir() is used on systems with large inodes (such as glusterfs). Signed-off-by: Niklas Hambüchen <mail@nh2.me>

helpers: Fix misleading variable name in fstat()

07d25ce

Signed-off-by: Niklas Hambüchen <mail@nh2.me>

rlbdv added the redirected label Sep 30, 2018

jmberg reviewed Nov 18, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance improvements on distributed file systems#43

Performance improvements on distributed file systems#43
nh2 wants to merge 4 commits into
bup:masterfrom
nh2:network-fs-improvements

nh2 commented Jun 1, 2018

Uh oh!

rlbdv commented Sep 30, 2018

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

jmberg Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nh2 commented Jun 1, 2018

Uh oh!

rlbdv commented Sep 30, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants