Date: Fri, 21 Jul 2023 10:37:12 +0200 From: Peter Eriksson <pen@lysator.liu.se> To: FreeBSD FS <freebsd-fs@freebsd.org> Cc: kevans@freebsd.org, Rick Macklem <rick.macklem@gmail.com> Subject: Re: RFC: Patch for mountd to handle a database for exports Message-ID: <7AC56987-BAB0-4291-8717-2D9EFB2487DB@lysator.liu.se> In-Reply-To: <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se> References: <CAM5tNy7GkMLgy-wjonVu%2BgOMnexoA2W8vSZvMo4NDikrg3-A1A@mail.gmail.com> <2D8B626E-82BB-410C-B7C7-35B4D3834A44@lysator.liu.se>
index | next in thread | previous in thread | raw e-mail
Regarding the potential for speed improvement here is a quick little benchmark… > # wc -l /etc/zfs/exports > 13116 /etc/zfs/exports With the original FreeBSD 13.2 code: > # /usr/bin/time zfs share -a > 40.08 real 34.00 user 6.04 sys > # /usr/bin/time zfs share -a > 40.15 real 34.06 user 6.04 sys With the DB code in use: > # service mountd stop ; mv /usr/sbin/mountd.DB /usr/sbin/mountd ; mv /lib/libzfs.so.4.DB /lib/libzfs.so.4 > # rm -f /etc/zfs/exports.db > # /usr/bin/time zfs share -a > 2.75 real 0.88 user 1.87 sys > # /usr/bin/time zfs share -a > 2.77 real 1.01 user 1.75 sys This translates into a shorter boot time if nothing else... - Peter > On 21 Jul 2023, at 10:22, Peter Eriksson <pen@lysator.liu.se> wrote: > > Hi! > > Here’s a couple of updated patches with some bug fixes. I’ve also moved the database location for ZFS to /etc/zfs/exports.db to mirror the current location. > > I also changed the patch for mountd so that for each “exports” source specified it first tries to open <path>.db which I think makes it easier to use (no need for the “-D” option and it supports multiple sources). Cleaner I think. > > > Btw, you can create the database manually from /etc/zfs/exports using “makemap” like this: > > makemap -f btree /etc/zfs/exports.db </etc/zfs/exports > > > Known “bugs”: > “V4:” > Even though you could create an /etc/exports.db with the contents of /etc/exports it will fail with the “V4:” lines since the BTree database will sort the exported entries so that the V4: key will appear last and then mountd will complain when scanning the DB. > > Also when using the file /etc/exports you can either write “V4:/export -sec=sys” or “V4: /export -sec=sys” (with a space between V4: and the path) so this probably needs some special handling (can’t simply use “makemap -f btree /etc/exports.db </etc/exports” to create it. > > I did try to fix that but the code quickly got hairy so I didn’t like it. If we really want/need that I’m thinking of creating a special case for the V4: handling, sort of like prefixing the database key with a NUL byte or something (so that it will be sorted first). > > > Multiline options - well, the current ZFS code doesn’t support it either so no change from the current setup but it would be nice to have. > Ie, support for things like: > /export -sec=krb5 > /export -sec=sys ro > > > > <mountd-sharedb.patch><libshare-nfs-sharedb.patch><share.c> > > > (I also agree that the USE_SHAREDB probably should be removed, I just have that here for now so that I can quickly disable the code) > > Regarding the patch to the zfs part - I’m not sure which way to go there - the current patch switches to always use the DB. But one could argue that the code could check for an existing /etc/zfs/exports.db and only use the DB-writing code if that already exists. That way it will support both the old way and the new way, but it requires an empty /etc/zfs/exports.db to be pre-created at initial boot time for it to start using it. > > - Peter > > >> On 21 Jul 2023, at 00:50, Rick Macklem <rick.macklem@gmail.com> wrote: >> >> Hi, >> >> Peter Eriksson has submitted an interesting patch that adds support >> for a database file for exports. My understanding is that this improves >> performance when used with the ZFS share property for exporting a >> large number of file systems. >> >> There are a couple of user visible issues that I'd like feedback from >> others. (Hopefully Peter can correct me if I get any of these wrong.) >> >> - The patch uses a single database file and a new "-D" option to >> specify it on the command line. >> --> I think it might be less confusing to just put the database file(s) >> in the exports list and identify them via a ".db" filename suffix. >> What do others think? >> >> The changes are #ifdef'd on USE_SHAREDB. I'm thinking that this >> change will be always built, so maybe USE_SHAREDB is not needed? >> (Obviously mountd(8)'s semantics will only changed if/when database >> file(s) are provided.) >> >> Once I have feedback on the above, I will put a patch up on >> phabricator. >> >> rick >> ps: I believe kevans@ has volunteered to shepperd the ZFS share >> property changes. >> >home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7AC56987-BAB0-4291-8717-2D9EFB2487DB>