You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
PR #388 adds the SFTP storage driver and hardens SSH host key handling, but the current runtime path opens a fresh SSH connection, authenticates, and starts a new SFTP subsystem for each storage operation.
put, get, delete, exists, metadata, copy_object, and put_reader each call self.connect().
get_stream / get_range use open_reader(), which also calls self.connect() and keeps the resulting connection alive inside SftpFileReader for the stream lifetime.
That is correct from a safety/lifetime perspective, but it makes hot-path reads, writes, metadata checks, and range reads pay the SSH handshake and authentication cost repeatedly. The cost is especially visible for folder listings, preview/range requests, WebDAV-style clients, and upload/download flows that perform multiple small operations.
Proposed solution
Introduce a small per-driver SFTP connection pool that reuses authenticated SSH connections while preserving stream safety.
Suggested implementation shape:
Add an internal SftpConnectionPool owned by SftpDriver, likely behind Arc.
Keep SftpSession owned by a checked-out connection lease; do not share SftpSession directly across threads.
Use a bounded pool, for example a small Semaphore plus Mutex<Vec<SftpConnection>>, with a conservative default pool size.
Make simple operations acquire a lease, run the operation, and return the connection to the pool only if it is still healthy.
Make SftpFileReader hold a connection lease so streamed reads/ranges keep their connection until the reader is dropped.
Drop stale/broken connections on SFTP/SSH IO errors and reconnect on the next acquire.
Preserve the PR feat: add SFTP storage backend #388 host key behavior: every newly created SSH connection must still validate the pinned storage_policy.options.sftp_host_key_fingerprint and fail closed on missing/mismatched host keys.
Add focused tests around lease return/drop behavior and the SFTP integration round trip. If practical, instrument the test server or connection factory to prove multiple sequential operations do not reconnect every time.
Alternatives considered
Keep opening one SSH connection per operation. This is simple and safe, but wastes handshake/authentication work on common SFTP workflows.
Store one global connection behind a mutex. This avoids repeated handshakes, but a long stream would block unrelated metadata/upload/delete operations and create poor tail latency.
Share SftpSession directly across operations. Avoid this unless the crate explicitly guarantees the needed concurrency semantics; a lease-owned session is the safer boundary.
Category
Performance
Checklist
I have searched existing issues for similar requests.
Problem
PR #388 adds the SFTP storage driver and hardens SSH host key handling, but the current runtime path opens a fresh SSH connection, authenticates, and starts a new SFTP subsystem for each storage operation.
Current code anchors:
src/storage/drivers/sftp.rs:SftpDriver::connect()performs SSH connect, password authentication, session channel open, and SFTP subsystem initialization.put,get,delete,exists,metadata,copy_object, andput_readereach callself.connect().get_stream/get_rangeuseopen_reader(), which also callsself.connect()and keeps the resulting connection alive insideSftpFileReaderfor the stream lifetime.That is correct from a safety/lifetime perspective, but it makes hot-path reads, writes, metadata checks, and range reads pay the SSH handshake and authentication cost repeatedly. The cost is especially visible for folder listings, preview/range requests, WebDAV-style clients, and upload/download flows that perform multiple small operations.
Proposed solution
Introduce a small per-driver SFTP connection pool that reuses authenticated SSH connections while preserving stream safety.
Suggested implementation shape:
SftpConnectionPoolowned bySftpDriver, likely behindArc.SftpSessionowned by a checked-out connection lease; do not shareSftpSessiondirectly across threads.SemaphoreplusMutex<Vec<SftpConnection>>, with a conservative default pool size.SftpFileReaderhold a connection lease so streamed reads/ranges keep their connection until the reader is dropped.storage_policy.options.sftp_host_key_fingerprintand fail closed on missing/mismatched host keys.Alternatives considered
SftpSessiondirectly across operations. Avoid this unless the crate explicitly guarantees the needed concurrency semantics; a lease-owned session is the safer boundary.Category
Performance
Checklist