chore(bssh-russh-sftp): sync vendored fork to upstream 2.3.0, drop write wrapper#211
Closed
Yaminyam wants to merge 3 commits into
Closed
chore(bssh-russh-sftp): sync vendored fork to upstream 2.3.0, drop write wrapper#211Yaminyam wants to merge 3 commits into
Yaminyam wants to merge 3 commits into
Conversation
The high-level `AsyncWrite`/`AsyncRead` impls on `File` issue exactly one SFTP `WRITE`/`READ` at a time and `await` its `STATUS`/`DATA` reply before sending the next. Sustained throughput is therefore bounded by `chunk_size / RTT` — at 50 ms RTT with the default 256 KiB chunk that caps a single transfer at ~5 MiB/s no matter how fast the link is. Add two pipelined helpers on `File` that keep up to N SFTP requests in flight concurrently, mirroring how OpenSSH's `sftp(1)` client behaves (`-R 64` by default): * `File::write_all_pipelined<R: AsyncRead>(reader, max_inflight)` — reads chunks from `reader` and dispatches `session.write(...)` futures via `FuturesUnordered`, refilling the pipeline as in-flight writes complete. Memory bounded by `max_inflight * write_len`. * `File::read_to_writer_pipelined<W: AsyncWrite>(writer, max_inflight)` — symmetric for downloads. Out-of-order responses are buffered in a `BTreeMap` keyed by offset and flushed to `writer` as soon as the next-expected chunk arrives. Wire `Client::upload_file`/`download_file`/`upload_dir_recursive`/ `download_dir_recursive` to use the new helpers with `MAX_INFLIGHT_REQUESTS = 64`. Measured on macOS arm64 against `bssh-server` v2.1.3 on loopback with a 1 GiB file: | op | build | real | RSS | |----------|------------------------|---------|----------| | upload | vanilla v2.1.3 | 39.30s | 3.23 GB | | upload | streaming-only | 3.47s | 20 MB | | upload | streaming + pipelined | 2.27s | 49 MB | | download | vanilla v2.1.3 | 3.93s | 2.17 GB | | download | streaming-only | 3.41s | 16 MB | | download | streaming + pipelined | 1.34s | 288 MB | Pipelining adds ~+53% on upload and ~+155% on download throughput on top of the streaming patch (which already eliminated the whole-file load). Peak RSS stays well below the unpatched levels: download holds at most ~`max_inflight` chunks pending in the reorder map, and upload caps at `max_inflight * chunk_size + reader buffer`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…apper Upstream russh-sftp 2.3.0 picked up both perf landings that motivated this fork's prior delta: - AspectUnk/russh-sftp#83 — `serde_bytes` perf for `WRITE`/`DATA` payloads (the original reason for `sftp-serde-bytes-perf.patch`). - AspectUnk/russh-sftp#85 — pipelined `AsyncWrite for File` with a `write_acks` queue and dynamic chunk sizing, plus `Config::max_concurrent_writes`. This subsumes our `write_all_pipelined` helper. The only remaining local delta is the read-side helper `File::read_to_writer_pipelined`, which is now proposed upstream as AspectUnk/russh-sftp#91. Until that merges and lands in a release, we keep it in the vendored crate. Changes: - `crates/bssh-russh-sftp/src/` synced to upstream `master` (2.3.0), then the `read_to_writer_pipelined` helper added on top of upstream `File`. - `crates/bssh-russh-sftp/Cargo.toml` mirrors upstream deps (notably `dashmap` replaced our `flurry`, plus pinned point versions). - `patches/sftp-serde-bytes-perf.patch` removed (obsolete). - `sync-upstream.sh` / `create-patch.sh` simplified — no longer reapply the obsolete patch. - `src/ssh/tokio_client/file_transfer.rs`: drop `write_all_pipelined` callers in favor of plain `tokio::io::copy`, and plumb our 64-deep pipeline through `SftpSession::new_with_config(stream, sftp_config())` (was: implicit default of 8). - `src/server/sftp.rs`: add `impl From<SftpError> for StatusReply` to satisfy upstream PR lablup#76's tightened `Handler::Error: Into<StatusReply>` bound. When lablup#91 merges and a russh-sftp release goes out, drop the vendored fork entirely and consume upstream directly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
Closing — fork is already cleaned up upstream. #207 (merged 2026-05-25 by @inureyes) did the same vendored sync work this PR proposes, and more:
I opened this against an outdated Sorry for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sync the vendored
bssh-russh-sftpcrate up to upstream russh-sftp 2.3.0. The fork's previous deltas have either landed upstream or been narrowed to a single helper that's now in review.serde_bytesperf fix forWRITE/DATA(~+29% on large transfers)AsyncWrite for Filepipelined (dynamic chunk sizes,write_acksqueue)File::read_to_writer_pipelined(high-RTT read helper)AsyncRead for Fileis still sequential upstream (oneREADperpoll_read, wait for reply), so we keep the read-side helper in the vendored crate until #91 merges. Everything else this fork carried is gone.What changed
crates/bssh-russh-sftp/— vendoredsrc/re-synced to upstreammaster(2.3.0 =dcc0c06 bump to 2.3.0), then theread_to_writer_pipelinedhelper added on top of upstreamFile.Cargo.tomlupdated to match upstream deps (notablydashmapreplaces ourflurry). README + scripts refreshed to reflect the simplified state.patches/sftp-serde-bytes-perf.patchdeleted (obsolete).Cargo.toml— pin bumped2.1.1 → 2.3.0, comment refreshed.src/ssh/tokio_client/file_transfer.rs—file.write_all_pipelined(reader, 64).awaitcalls replaced withtokio::io::copy(&mut reader, &mut file).awaitnow thatAsyncWriteis natively pipelined upstream. We plumb the 64-deep pipeline depth throughSftpSession::new_with_config(stream, sftp_config())(Config::max_concurrent_writes = 64), since upstream's default is 8.src/server/sftp.rs— addimpl From<SftpError> for StatusReplyto satisfy upstream PR Terminal escape sequences displayed on screen when running Neovim in SSH session #76's tightenedHandler::Error: Into<StatusReply>bound. Existingimpl From<SftpError> for StatusCodekept; both coexist.Why
sync-upstream.sh masterinstead of a patch-rebase exercise.Test plan
cargo build→ cleancargo build -p bssh-russh-sftp→ cleancargo test --lib -p bssh-russh-sftp→ 0/0 (baseline unchanged)cargo test --lib→ existing tests passConfig::max_concurrent_writesplumbing works end-to-end on a real network.Related
File::read_to_writer_pipelinedfor high-RTT SFTP reads AspectUnk/russh-sftp#91