From 42f1c363d772f05fc3c20920ee40880fcb808170 Mon Sep 17 00:00:00 2001 From: Andrew Walker Date: Fri, 6 Sep 2024 05:33:06 -0600 Subject: [PATCH] Be more selective about file metdata in listdir The directory iterator we use is configurable about what file metdata to retrieve. This can in some situations significantly improve performance of generating directory listing. --- .../middlewared/plugins/filesystem.py | 51 ++++++++++++++++++- .../middlewared/utils/filesystem/directory.py | 3 ++ tests/api2/test_listdir_request_mask.py | 30 +++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 tests/api2/test_listdir_request_mask.py diff --git a/src/middlewared/middlewared/plugins/filesystem.py b/src/middlewared/middlewared/plugins/filesystem.py index 94a59f31f668d..655d56cec877e 100644 --- a/src/middlewared/middlewared/plugins/filesystem.py +++ b/src/middlewared/middlewared/plugins/filesystem.py @@ -20,7 +20,7 @@ from middlewared.utils.filesystem import attrs, stat_x from middlewared.utils.filesystem.acl import acl_is_present from middlewared.utils.filesystem.constants import FileType -from middlewared.utils.filesystem.directory import DirectoryIterator +from middlewared.utils.filesystem.directory import DirectoryIterator, DirectoryRequestMask from middlewared.utils.filesystem.utils import timespec_convert from middlewared.utils.mount import getmntinfo from middlewared.utils.nss import pwd, grp @@ -214,6 +214,31 @@ def mkdir(self, data): 'zfs_attrs': ['ARCHIVE'] } + @private + def listdir_request_mask(self, select): + """ create request mask for directory listing """ + if not select: + # request_mask=None means ALL in the directory iterator + return None + + request_mask = 0 + for i in select: + selected = i[0] if isinstance(i, list) else i + + match selected: + case 'realpath': + request_mask |= DirectoryRequestMask.REALPATH + case 'acl': + request_mask |= DirectoryRequestMask.ACL + case 'zfs_attrs': + request_mask |= DirectoryRequestMask.ZFS_ATTRS + case 'is_ctldir': + request_mask |= DirectoryRequestMask.CTLDIR + case 'xattrs': + request_mask |= DirectoryRequestMask.XATTRS + + return request_mask + @accepts( Str('path', required=True), Ref('query-filters'), @@ -248,6 +273,14 @@ def listdir(self, path, filters, options): """ Get the contents of a directory. + The select option may be used to optimize listdir performance. Metadata-related + fields that are not selected will not be retrieved from the filesystem. + + For example {"select": ["path", "type"]} will avoid querying an xattr list and + ZFS attributes for files in a directory. + + NOTE: an empty list for select (default) is treated as requesting all information. + Each entry of the list consists of: name(str): name of the file path(str): absolute path of the entry @@ -274,6 +307,20 @@ def listdir(self, path, filters, options): if not path.is_dir(): raise CallError(f'Path {path} is not a directory', errno.ENOTDIR) + if options.get('count') is True: + # We're just getting count, drop any unnecessary info + request_mask = 0 + else: + request_mask = self.listdir_request_mask(options.get('select', None)) + + # None request_mask means "everything" + if request_mask is None or (request_mask & DirectoryRequestMask.ZFS_ATTRS): + # Make sure this is actually ZFS before issuing FS ioctls + try: + self.get_zfs_attributes(str(path)) + except Exception: + raise CallError(f'{path}: ZFS attributes are not supported.') + file_type = None for filter_ in filters: if filter_[0] not in ['type']: @@ -300,7 +347,7 @@ def listdir(self, path, filters, options): # filter these here. filters.extend([['is_mountpoint', '=', True], ['name', '!=', IX_APPS_DIR_NAME]]) - with DirectoryIterator(path, file_type=file_type) as d_iter: + with DirectoryIterator(path, file_type=file_type, request_mask=request_mask) as d_iter: return filter_list(d_iter, filters, options) @accepts(Str('path'), roles=['FILESYSTEM_ATTRS_READ']) diff --git a/src/middlewared/middlewared/utils/filesystem/directory.py b/src/middlewared/middlewared/utils/filesystem/directory.py index 7a5f72e18d70f..49da034960e00 100644 --- a/src/middlewared/middlewared/utils/filesystem/directory.py +++ b/src/middlewared/middlewared/utils/filesystem/directory.py @@ -36,6 +36,9 @@ class DirectoryRequestMask(enum.IntFlag): XATTR - list of extended attributes (requires listxattr call) ZFS_ATTRS - include ZFS attributes (requires fcntl call per file) + + NOTE: this changes to this should also be reflected in API test + `test_listdir_request_mask.py` """ ACL = enum.auto() CTLDIR = enum.auto() diff --git a/tests/api2/test_listdir_request_mask.py b/tests/api2/test_listdir_request_mask.py new file mode 100644 index 0000000000000..16a4a150d5fb7 --- /dev/null +++ b/tests/api2/test_listdir_request_mask.py @@ -0,0 +1,30 @@ +import enum +import pytest + +from middlewared.test.integration.utils import call + + +class DirectoryRequestMask(enum.IntFlag): + ACL = enum.auto() + CTLDIR = enum.auto() + REALPATH = enum.auto() + XATTRS = enum.auto() + ZFS_ATTRS = enum.auto() + + +@pytest.mark.parametrize('select_key,request_mask', [ + ('realpath', DirectoryRequestMask.REALPATH.value), + ('acl', DirectoryRequestMask.ACL.value), + ('zfs_attrs', DirectoryRequestMask.ZFS_ATTRS.value), + ('is_ctldir', DirectoryRequestMask.CTLDIR.value), + ('xattrs', DirectoryRequestMask.XATTRS.value), + ([], None), + ('name', 0) +]) +def test__select_to_request_mask(select_key, request_mask): + val = call('filesystem.listdir_request_mask', select_key) + + if select_key == []: + assert val is None + + assert val == request_mask