From f007e9001cbd9cf71532b4761403874c0b1f76f2 Mon Sep 17 00:00:00 2001 From: mxmlnkn Date: Wed, 5 Aug 2026 10:20:12 -0700 Subject: [PATCH 1/2] Use server-reported block sizes for SSHFile when none is specified When the server implements limits@openssh.com, use its max_read_len / max_write_len as the block size for opened files (OpenSSH reports 255 KiB, roughly 3x-5x the previous defaults, measured ~3x faster reads in #49). When the extension is not supported, asyncssh synthesizes 16 KiB floors with a zero max_packet_len -- keep the tuned 48 KiB / 240 KiB defaults there instead of degrading to the floors. An explicitly passed block_size wins as before. Also drop a shadowed duplicate seekable() definition left over from merging #50 and #51. Fixes #49 Co-Authored-By: Claude Fable 5 --- sshfs/file.py | 48 +++++++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/sshfs/file.py b/sshfs/file.py index bc1c1e9..4a9ee6b 100644 --- a/sshfs/file.py +++ b/sshfs/file.py @@ -25,32 +25,41 @@ def __init__( self.mode = mode self.max_requests = max_requests or _MAX_SFTP_REQUESTS - if block_size is None: - # "The OpenSSH SFTP server will close the connection - # if it receives a message larger than 256 KB, and - # limits read requests to returning no more than - # 64 KB." - # - # We are going to use the maximum block_size possible - # with a 16KB margin (so instead of sending 256 KB data, - # we'll send 240 KB + headers for write requests) - - if self.readable(): - block_size = READ_BLOCK_SIZE - else: - block_size = WRITE_BLOCK_SIZE - # The blocksize is often used with constructs like # shutil.copyfileobj(src, dst, length=file.blocksize) and since we are # using pipelining, we are going to reflect the total size rather than # a size of chunk to our limits. - self.blocksize = block_size * self.max_requests + self.blocksize = ( + None if block_size is None else block_size * self.max_requests + ) self.kwargs = kwargs self._file = sync(self.loop, self._open_file) self._closed = False + def _determine_block_size(self, channel): + # Use the limits reported by the server (limits@openssh.com) to + # get the best performance. A zero max_packet_len means the + # server never reported limits and the read/write lengths are + # asyncssh's synthesized 16 KiB floors -- fall through to the + # larger defaults below instead of degrading to them. + limits = getattr(channel, "limits", None) + if limits and limits.max_packet_len: + if self.readable(): + return limits.max_read_len + return limits.max_write_len + + # "The OpenSSH SFTP server will close the connection + # if it receives a message larger than 256 KB, and + # limits read requests to returning no more than + # 64 KB." + # + # We are going to use the maximum block_size possible + # with a 16KB margin (so instead of sending 256 KB data, + # we'll send 240 KB + headers for write requests) + return READ_BLOCK_SIZE if self.readable() else WRITE_BLOCK_SIZE + @wrap_exceptions async def _open_file(self): # TODO: this needs to keep a reference to the @@ -60,6 +69,10 @@ async def _open_file(self): # it's operations but the pool it thinking this # channel is freed. async with self.fs._pool.get() as channel: + if self.blocksize is None: + self.blocksize = ( + self._determine_block_size(channel) * self.max_requests + ) return await channel.open( self.path, self.mode, @@ -80,9 +93,6 @@ async def _open_file(self): def readable(self): return "r" in self.mode or "+" in self.mode - def seekable(self): - return "r" in self.mode or "w" in self.mode - def seekable(self): return True From 1292a153b5a2b7f771c4af0fae5c9e277f6308de Mon Sep 17 00:00:00 2001 From: Ivan Shcheklein Date: Wed, 5 Aug 2026 10:20:12 -0700 Subject: [PATCH 2/2] tests: cover SSHFile block size selection test_determine_block_size pins all three cases (server-reported limits, synthesized floors with zero max_packet_len, asyncssh without the limits API); test_open_block_size checks end to end that a server without limits@openssh.com keeps the tuned defaults and that an explicit block_size wins. Co-Authored-By: Claude Fable 5 --- tests/test_sshfs.py | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/test_sshfs.py b/tests/test_sshfs.py index 2892674..f571d7a 100644 --- a/tests/test_sshfs.py +++ b/tests/test_sshfs.py @@ -6,6 +6,7 @@ from concurrent import futures from datetime import datetime, timedelta, timezone from pathlib import Path +from types import SimpleNamespace import fsspec import pytest @@ -13,6 +14,8 @@ from importlib_metadata import entry_points from sshfs import SSHFileSystem +from sshfs.file import SSHFile +from sshfs.utils import READ_BLOCK_SIZE, WRITE_BLOCK_SIZE _STATIC = (Path(__file__).parent / "static").resolve() USERS = {"user": _STATIC / "user.key"} @@ -336,6 +339,50 @@ def test_exceptions(fs, remote_dir): fs.makedirs(remote_dir + "/dir/a/b/c") +def test_open_block_size(fs, remote_dir): + # mockssh (paramiko) does not implement limits@openssh.com, so the + # tuned defaults must survive asyncssh's synthesized 16 KiB floors. + fs.touch(remote_dir + "/a.txt") + with fs.open(remote_dir + "/a.txt", "rb") as file: + assert file.blocksize == READ_BLOCK_SIZE * file.max_requests + with fs.open(remote_dir + "/b.txt", "wb") as file: + assert file.blocksize == WRITE_BLOCK_SIZE * file.max_requests + # An explicit block_size always wins. + with fs.open(remote_dir + "/c.txt", "wb", block_size=4096) as file: + assert file.blocksize == 4096 * file.max_requests + + +def test_determine_block_size(): + reader = SimpleNamespace(readable=lambda: True) + writer = SimpleNamespace(readable=lambda: False) + determine = SSHFile._determine_block_size + + # The server reported its limits: use them. + reported = SimpleNamespace( + limits=SimpleNamespace( + max_packet_len=262144, + max_read_len=261120, + max_write_len=131072, + ) + ) + assert determine(reader, reported) == 261120 + assert determine(writer, reported) == 131072 + + # No limits@openssh.com support: asyncssh synthesizes 16 KiB + # read/write floors with max_packet_len == 0; keep the defaults. + synthesized = SimpleNamespace( + limits=SimpleNamespace( + max_packet_len=0, max_read_len=16384, max_write_len=16384 + ) + ) + assert determine(reader, synthesized) == READ_BLOCK_SIZE + assert determine(writer, synthesized) == WRITE_BLOCK_SIZE + + # asyncssh without the limits API at all. + assert determine(reader, SimpleNamespace()) == READ_BLOCK_SIZE + assert determine(writer, SimpleNamespace()) == WRITE_BLOCK_SIZE + + def test_open_rw(fs, remote_dir): data = b"dvc.org"