update: sync bssh-russh-sftp with upstream russh-sftp 2.4.0

Cargo.toml

@@ -34,9 +34,10 @@ tokio = { version = "1.52.3", features = ["full"] }
 # handler in src/ssh/tokio_client/connection.rs refuses certificates because
 # bssh verifies no CA signatures.
 russh = "0.63.1"
-# Use our internal russh-sftp fork tracking upstream 2.3.0
-# (adds pipelined File I/O; serde_bytes perf fix is now upstreamed)
-russh-sftp = { package = "bssh-russh-sftp", version = "2.3.0", path = "crates/bssh-russh-sftp" }
+# Use our internal russh-sftp fork tracking upstream 2.4.0
+# (adds pipelined File I/O and the server read-ahead / write-coalescing loop;
+# the serde_bytes perf fix is now upstreamed)
+russh-sftp = { package = "bssh-russh-sftp", version = "2.4.0", path = "crates/bssh-russh-sftp" }
 clap = { version = "4.6.1", features = ["derive", "env"] }
 anyhow = "1.0.102"
 thiserror = "2.0.18"

crates/bssh-russh-sftp/Cargo.toml

@@ -1,8 +1,8 @@
 [package]
 name = "bssh-russh-sftp"
-version = "2.3.0"
+version = "2.4.0"
 authors = ["Jeongkyu Shin <inureyes@gmail.com>"]
-description = "Temporary fork of russh-sftp 2.3.0 adding pipelined SFTP File I/O (write_all_pipelined / read_to_writer_pipelined). These helpers hide per-request RTT for fast bulk transfers and are the only value-add over upstream russh-sftp."
+description = "Temporary fork of russh-sftp 2.4.0 adding pipelined SFTP File I/O (write_all_pipelined / read_to_writer_pipelined). These helpers hide per-request RTT for fast bulk transfers and are the only value-add over upstream russh-sftp."
 documentation = "https://docs.rs/bssh-russh-sftp"
 edition = "2021"
 homepage = "https://github.com/lablup/bssh"
@@ -11,7 +11,7 @@ license = "Apache-2.0"
 readme = "README.md"
 repository = "https://github.com/lablup/bssh"
 
-# Dependency versions mirror upstream russh-sftp 2.3.0 (AspectUnk/russh-sftp).
+# Dependency versions mirror upstream russh-sftp 2.4.0 (AspectUnk/russh-sftp).
 # Update via ./sync-upstream.sh; the only fork addition is the `futures` dep,
 # needed by the forward-ported pipelined helpers in src/client/fs/file.rs.
 [dependencies]

crates/bssh-russh-sftp/README.md

@@ -1,35 +1,49 @@
 # bssh-russh-sftp
 
-**Temporary fork of [russh-sftp](https://crates.io/crates/russh-sftp) (tracking upstream `2.3.0`) adding pipelined SFTP file I/O.**
+**Temporary fork of [russh-sftp](https://crates.io/crates/russh-sftp) (tracking upstream `2.4.0`) adding pipelined SFTP file I/O and a read-ahead server loop.**
 
 This crate exists so bssh can ship faster bulk SFTP transfers independently, while keeping the public crate name usable through Cargo's `package = "bssh-russh-sftp"` dependency alias.
 
 ## The Value-Add
 
-The fork adds two helpers to `client::fs::File` that keep many SFTP requests in flight at once, hiding per-request round-trip latency (mirroring how OpenSSH's `sftp` client keeps ~64 requests outstanding):
+### Client: pipelined file I/O (`src/client/fs/file.rs`)
 
-- `File::write_all_pipelined(reader, max_inflight)` — streams a reader to the remote file with up to `max_inflight` concurrent `SSH_FXP_WRITE`s.
-- `File::read_to_writer_pipelined(writer, max_inflight)` — streams the remote file to a writer with up to `max_inflight` concurrent `SSH_FXP_READ`s, reassembling chunks in offset order so the output matches a sequential read.
+Two helpers on `client::fs::File` keep many SFTP requests in flight at once, hiding per-request round-trip latency (mirroring how OpenSSH's `sftp` client keeps ~64 requests outstanding):
 
-These are the only additions over upstream. They live in `src/client/fs/file.rs` and are re-applied on each sync from `patches/pipelined-file-io.patch`.
+- `File::write_all_pipelined(reader, max_inflight)` streams a reader to the remote file with up to `max_inflight` concurrent `SSH_FXP_WRITE`s.
+- `File::read_to_writer_pipelined(writer, max_inflight)` streams the remote file to a writer with up to `max_inflight` concurrent `SSH_FXP_READ`s, reassembling chunks in offset order so the output matches a sequential read.
+
+Re-applied on sync from `patches/pipelined-file-io.patch`.
+
+### Server: request read-ahead and write coalescing (`src/server/mod.rs`)
+
+The serial request loop is replaced by a byte-bounded intake queue plus a processor, adding two `server::Config` knobs: `max_buffered_request_bytes` (default 8 MiB) and `max_write_coalesce_len` (default 256 KiB). Read-ahead keeps the transport decrypting requests while the handler is blocked on file I/O, and consecutive `SSH_FXP_WRITE`s to the same handle at sequential offsets are merged into one handler call while each request id still gets its own status reply. The unbounded-in-count, bounded-in-bytes intake is deliberate: stalling intake can deadlock against the russh session loop waiting on channel window (see issue lablup/bssh#227, paramiko's unbounded READ prefetch).
+
+Re-applied on sync from `patches/server-readahead-write-coalescing.patch`.
 
 > The `serde_bytes` packet-serialization performance fix that originally motivated this fork was upstreamed in russh-sftp 2.1.2; it is kept for reference under `patches/historical/`.
 
 ## Usage
 
 ```toml
 [dependencies]
-russh-sftp = { package = "bssh-russh-sftp", version = "2.3.0" }
+russh-sftp = { package = "bssh-russh-sftp", version = "2.4.0" }
 ```
 
 ## Sync with Upstream
 
 ```bash
 cd crates/bssh-russh-sftp
-./sync-upstream.sh 2.3.0   # omit the version to use upstream's default branch
+./sync-upstream.sh 2.4.0   # omit the version to use upstream's default branch
 ```
 
-`sync-upstream.sh` copies upstream `src` verbatim and re-applies every patch directly under `patches/` (anything under `patches/historical/` is excluded). Patches already merged upstream are detected via reverse-apply and skipped.
+`sync-upstream.sh` copies upstream `src` verbatim and re-applies every patch directly under `patches/` (anything under `patches/historical/` is excluded), then verifies each patch is present in the result, builds, and runs the fork tests.
+
+Upstream publishes **no git tags**, and marks releases with a `bump to <version>` commit instead, so both scripts resolve a version argument to that commit. An unresolvable version is a hard error listing the available release commits: falling back to the default branch would vendor unreleased code while stamping `Cargo.toml` with the requested version. Resolution happens before anything is copied, so a bad version leaves the tree untouched.
+
+Patch state is detected with `git apply --check`, not `patch --dry-run`. Apple's bundled `patch` silently auto-corrects direction and exits 0 whether a patch applies, is reversed, or is already applied, so its exit status cannot distinguish "already upstream" from "not applied yet".
+
+Because the sync deletes `src/**/*.rs` before copying upstream over it, **a fork change with no patch file is silently lost**. Regenerate the patches with `./create-patch.sh <version>` after editing vendored code; it diffs every file listed in its `PATCH_TARGETS` and warns about any other file that drifts from upstream without an entry.
 
 ## License
 

crates/bssh-russh-sftp/create-patch.sh

@@ -1,13 +1,13 @@
 #!/bin/bash
 # create-patch.sh
-# Regenerates patches/pipelined-file-io.patch by diffing the current vendored
-# source against a fresh checkout of upstream russh-sftp.
+# Regenerates every file in patches/ by diffing the current vendored source
+# against a fresh checkout of upstream russh-sftp.
 #
 # Self-contained: clones upstream into a temp dir (no manually-maintained
 # references/ directory needed), so it always diffs against the exact version.
 #
 # Usage: ./create-patch.sh [version]
-#   version: optional, e.g. "2.3.0" (default: upstream's default branch, since
+#   version: optional, e.g. "2.4.0" (default: upstream's default branch, since
 #            russh-sftp does not publish git tags)
 
 set -e
@@ -16,13 +16,14 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 UPSTREAM_URL="https://github.com/AspectUnk/russh-sftp.git"
 TEMP_DIR="/tmp/russh-sftp-createpatch-$$"
 PATCH_DIR="$SCRIPT_DIR/patches"
-PATCH_FILE="$PATCH_DIR/pipelined-file-io.patch"
 
+RED='\033[0;31m'
 GREEN='\033[0;32m'
 YELLOW='\033[1;33m'
 NC='\033[0m'
 log_info() { echo -e "${GREEN}[INFO]${NC} $1"; }
 log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; }
+log_error() { echo -e "${RED}[ERROR]${NC} $1" >&2; }
 
 cleanup() { [ -d "$TEMP_DIR" ] && rm -rf "$TEMP_DIR"; }
 trap cleanup EXIT
@@ -34,37 +35,79 @@ git clone --quiet "$UPSTREAM_URL" "$TEMP_DIR"
 cd "$TEMP_DIR"
 
 if [ -z "$VERSION" ]; then
-    VERSION=$(git describe --tags --abbrev=0 2>/dev/null || echo "master")
+    VERSION="master"
 fi
+
+# Same resolution as sync-upstream.sh: russh-sftp publishes no git tags, and
+# marks releases with a "bump to <version>" commit. A patch must be generated
+# against the exact base the vendored tree was synced from, so an unresolvable
+# version is an error rather than a silent fall back to the default branch.
 if [ "$VERSION" != "master" ]; then
-    # russh-sftp publishes no git tags, so a version string may not be a ref.
-    if ! { git checkout --quiet "v$VERSION" 2>/dev/null || git checkout --quiet "$VERSION" 2>/dev/null; }; then
-        log_warn "No git ref '$VERSION' (russh-sftp publishes no tags); diffing against the default branch."
-        VERSION="master"
+    if git rev-parse --verify -q "v$VERSION^{commit}" > /dev/null; then
+        REF="v$VERSION"
+    elif git rev-parse --verify -q "$VERSION^{commit}" > /dev/null; then
+        REF="$VERSION"
+    else
+        REF=$(git log --format='%H' --grep="^bump to $VERSION\$" -1)
+        if [ -z "$REF" ]; then
+            log_error "Cannot resolve upstream version '$VERSION': no tag, no ref, and no 'bump to $VERSION' commit."
+            log_error "Available release commits:"
+            git log --oneline --grep='^bump to' | head -10 >&2
+            exit 1
+        fi
     fi
+    git checkout --quiet "$REF"
 fi
 log_info "Diffing against upstream $VERSION ($(git rev-parse --short HEAD))"
 
 UPSTREAM_SRC="$TEMP_DIR/src"
 CURRENT_SRC="$SCRIPT_DIR/src"
 mkdir -p "$PATCH_DIR"
 
-# The only fork change is the pipelined File I/O in client/fs/file.rs
-# (write_all_pipelined / read_to_writer_pipelined). Emit a -p1 patch.
-diff -u \
-    --label a/src/client/fs/file.rs \
-    --label b/src/client/fs/file.rs \
-    "$UPSTREAM_SRC/client/fs/file.rs" \
-    "$CURRENT_SRC/client/fs/file.rs" \
-    > "$PATCH_FILE" || true
+# Every fork change, one patch per file. Keep this list in sync with the fork:
+# a file that drifts from upstream without an entry here is silently deleted by
+# sync-upstream.sh, which wipes src/ before copying upstream over it.
+#   client/fs/file.rs - pipelined File I/O (write_all_pipelined /
+#                       read_to_writer_pipelined)
+#   server/mod.rs     - request read-ahead intake queue and sequential-write
+#                       coalescing (issue lablup/bssh#227)
+PATCH_TARGETS=(
+    "client/fs/file.rs:pipelined-file-io.patch"
+    "server/mod.rs:server-readahead-write-coalescing.patch"
+)
 
-if [ -s "$PATCH_FILE" ]; then
-    LINES=$(wc -l < "$PATCH_FILE" | tr -d ' ')
-    log_info "Patch created: $PATCH_FILE ($LINES lines)"
-    echo ""
-    echo "Patch summary:"
-    echo "=============="
-    grep -E "^@@|^\+\+\+|^---" "$PATCH_FILE" | head -20
-else
-    log_warn "No differences found - patch file is empty"
-fi
+# Guard against exactly the failure this list exists to prevent: any src file
+# that differs from upstream but has no patch entry.
+UNTRACKED=0
+while IFS= read -r REL; do
+    for TARGET in "${PATCH_TARGETS[@]}"; do
+        [ "${TARGET%%:*}" = "$REL" ] && continue 2
+    done
+    log_warn "src/$REL differs from upstream but has no PATCH_TARGETS entry; sync-upstream.sh would discard it"
+    UNTRACKED=1
+done < <(cd "$UPSTREAM_SRC" && find . -name '*.rs' -type f | sed 's|^\./||' | while read -r F; do
+    if [ ! -f "$CURRENT_SRC/$F" ] || ! diff -q "$UPSTREAM_SRC/$F" "$CURRENT_SRC/$F" > /dev/null 2>&1; then
+        echo "$F"
+    fi
+done)
+
+for TARGET in "${PATCH_TARGETS[@]}"; do
+    REL="${TARGET%%:*}"
+    OUT="$PATCH_DIR/${TARGET##*:}"
+
+    diff -u \
+        --label "a/src/$REL" \
+        --label "b/src/$REL" \
+        "$UPSTREAM_SRC/$REL" \
+        "$CURRENT_SRC/$REL" \
+        > "$OUT" || true
+
+    if [ -s "$OUT" ]; then
+        LINES=$(wc -l < "$OUT" | tr -d ' ')
+        log_info "Patch created: $OUT ($LINES lines)"
+    else
+        log_warn "No differences in src/$REL - $OUT is empty (already upstream?)"
+    fi
+done
+
+[ "$UNTRACKED" -eq 0 ] || log_warn "One or more fork changes are untracked; add them to PATCH_TARGETS before syncing."

crates/bssh-russh-sftp/patches/pipelined-file-io.patch

@@ -1,10 +1,9 @@
 --- a/src/client/fs/file.rs
 +++ b/src/client/fs/file.rs
-@@ -91,6 +91,205 @@
-
+@@ -96,6 +96,205 @@
          self.session.fsync(self.handle.as_str()).await.map(|_| ())
      }
-+
+ 
 +    /// Streams `reader` to this remote file with up to `max_inflight` concurrent
 +    /// SFTP `WRITE` requests in flight. Each request carries up to the negotiated
 +    /// `write_len` (or the per-handle packet ceiling when no limit is advertised).
@@ -203,6 +202,7 @@
 +        self.pos = next_to_write;
 +        Ok(total)
 +    }
- }
-
- fn check_write_result(
++
+     /// Closes the file waiting for all pending writes and the close itself
+     /// to be confirmed by the remote party.
+     /// Equivalent to [`shutdown`](tokio::io::AsyncWriteExt::shutdown)