Skip to content

Commit

Permalink
Be more selective about file metdata in listdir (#14434)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
anodos325 authored Sep 6, 2024
1 parent a882ba3 commit d5e0d6e
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 4 deletions.
53 changes: 51 additions & 2 deletions src/middlewared/middlewared/plugins/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -214,6 +214,33 @@ 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:
# select may be list [key, new_name] to allow
# equivalent of SELECT AS.
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'),
Expand Down Expand Up @@ -248,6 +275,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
Expand All @@ -274,6 +309,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']:
Expand All @@ -300,7 +349,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'])
Expand Down
7 changes: 5 additions & 2 deletions src/middlewared/middlewared/utils/filesystem/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -245,9 +248,9 @@ def __next__(self):
attr_mask = fget_zfs_file_attributes(fd)
zfs_attrs = zfs_attributes_dump(attr_mask)
except OSError as e:
# non-ZFS filesystems will fail with ENOTTY
# non-ZFS filesystems will fail with ENOTTY or EINVAL
# In this case we set `None` to indicate non-ZFS
if e.errno != errno.ENOTTY:
if e.errno not in (errno.ENOTTY, errno.EINVAL):
raise e from None

zfs_attrs = None
Expand Down
31 changes: 31 additions & 0 deletions tests/api2/test_listdir_request_mask.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
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),
(['xattrs', 'user_xattrs'], DirectoryRequestMask.XATTRS.value),
([], None),
('name', 0)
])
def test__select_to_request_mask(select_key, request_mask):
if select_key == []:
val = call('filesystem.listdir_request_mask', [])
assert val is None
else:
val = call('filesystem.listdir_request_mask', [select_key])
assert val == request_mask

0 comments on commit d5e0d6e

Please sign in to comment.