Skip to content

fix(sftp): 目录上传改为受限并发,消除小文件串行往返(#265) - #269

Merged
feigeCode merged 1 commit into
mainfrom
fix/issue-265-sftp-upload-concurrency
Sep 21, 2026
Merged

feigeCode merged 1 commit into
mainfrom
fix/issue-265-sftp-upload-concurrency

Conversation

@feigeCode

@feigeCode feigeCode commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Refs #265

Description

Directory uploads were fully serial: upload_dir_with_progress uploaded one file at a time. Combined with the fixed per-file control round trips that with_remote_replace requires (stat / open / fsync / fstat / close / rename), small-file throughput was bound by RTT instead of bandwidth — roughly 180 ms per file at 30 ms RTT, i.e. over 3 minutes for 1000 small files.

Changes

  • New module crates/sftp/src/upload_batch.rs:
    • run_bounded — bounded concurrency built on futures::stream::FuturesUnordered. Deliberately not JoinSet: JoinSet::spawn requires 'static futures, but with_remote_replace borrows &SftpSession from the caller's stack, so the borrow cannot be erased.
    • ConcurrencyBudget — slot-based budget. A small file takes one slot; a file larger than SMALL_FILE_MAX_BYTES takes the whole budget.
    • ProgressHighWater — monotonic high-water mark so interleaved per-file callbacks cannot pull the progress bar backwards.
  • upload_dir_with_progress now runs file uploads through run_bounded with a concurrency of 6. Files above SMALL_FILE_MAX_BYTES (512 KiB, aligned with the existing PIPELINE_THRESHOLD) still take the entire budget, so large-file behaviour is unchanged: the write path already saturates the 64 × 256 KiB pipeline window, and overlapping large files would only increase unacknowledged bytes in memory without gaining throughput.
  • Progress transferred is now accumulated in a global AtomicU64 instead of "completed files + current file".
  • Failure semantics are preserved on purpose: after the first error no new job is admitted, but in-flight jobs run to completion before the error is returned. Dropping them early would leave RemoteReplaceTemp staging files (.name.navop-part-<uuid>) on the remote, because cleanup() would never run.

AI assistance: the code in this PR was generated with AI assistance. It was reviewed, formatted with rustfmt and tested locally. It follows the module organisation and naming conventions already used in crates/sftp/src/.

Screenshot

No UI change — this is a transfer performance fix.

How to Test

  • Red → Green: 8 new unit tests in upload_batch.rs; 5 of them fail before the implementation (job weighting, concurrency peak, non-monotonic progress, no drain-on-error).
  • cargo test -p sftp → 93 passed, 0 failed.
  • cargo clippy -p sftp --all-targets → no new warnings.
  • rustfmt --check clean on the touched files.
  • cargo check -p sftp_transfer -p sftp_view -p remote_file_editor -p ftp → passes.

Reproducing the original scenario (optional): run a local SFTP server behind a TCP latency proxy, then upload 100 × 5 KB files. Sequentially over a 40 ms proxy this takes about 18.5 s; the concurrent path is expected to land in the 3–4 s range.

Note: this PR does not include an end-to-end throughput number over a real WAN link. The unit tests pin the scheduling semantics (real overlap, large files stay serial, in-flight jobs drain on error, monotonic progress), not real network speed.

Checklist

  • I have read the CONTRIBUTING document and followed the guidelines.
  • Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate.
  • Passed cargo run for story tests related to the changes. — N/A, this change touches no UI or component code.
  • Tested macOS, Windows and Linux platforms performance (if the change is platform-specific) — build and tests verified on macOS only.

- 上传每个文件固定要付 6 次控制往返(stat/open/fsync/fstat/close/rename),
  串行执行时小文件吞吐由 RTT 而不是带宽决定,且任何时刻只有一个文件在传。
- 新增 upload_batch 模块:run_bounded 用 FuturesUnordered 做有界并发调度,
  ConcurrencyBudget 按槽位分配预算(小文件 1 槽、大文件独占全部槽位),
  ProgressHighWater 保证并发上报的进度单调不回跳。
- upload_dir_with_progress 小文件并发度 6;超过 SMALL_FILE_MAX_BYTES(512 KiB)
  的文件独占预算,行为与串行一致,不放大内存中的未确认字节。
- 失败语义保持不变:首个错误后不再接纳新任务,但在飞任务跑完再返回错误,
  避免 RemoteReplaceTemp 的暂存文件残留。
- 进度 transferred 改为全局原子累计(原先为「已完成文件 + 本文件」)。
- 验证:cargo test -p sftp 93 passed(新增 8 个);cargo clippy -p sftp
  --all-targets 无新告警;sftp_transfer/sftp_view/remote_file_editor 编译通过。
@feigeCode
feigeCode merged commit 14370eb into main Sep 21, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant