Skip to content

Reuse an SFTP channel for small downloads - #10

Merged
LukeGus merged 2 commits into
Termix-SSH:dev-1.1.0from
deanxbox:perf/small-file-download-channel
Oct 20, 2026
Merged

LukeGus merged 2 commits into
Termix-SSH:dev-1.1.0from
deanxbox:perf/small-file-download-channel

Conversation

@deanxbox

@deanxbox deanxbox commented Oct 19, 2026 •

Copy link
Copy Markdown

Reuse one per-session SFTP download channel for files under 8 MiB, reducing channel setup overhead for bulk downloads while keeping large files on dedicated channels. On one host, a 100-file download improved from about 4.1–4.3 to 14–17 files/s; a user-timed download in the Termix app improved from 24.76 s to 9.7 s.

Overview

This PR reuses an SFTP channel for small downloads to improve bulk-download throughput.

  • Updated: Small files share a lazily opened per-session download channel; files at least 8 MiB use dedicated channels.
  • Fixed: Cancelled small downloads with a stalled close retire the shared channel so a later download can open a fresh one.

Changes Made

  • Kept the shared download channel separate from the session's browsing channel, with a fallback to the browsing channel if a download channel cannot be opened.
  • Added handling for stalled close operations after cancellation, including an identity guard to prevent a late timer from evicting a replacement channel. If a stalled channel is retired, other small downloads running on it fail and need retrying.
  • Reworked backend download-route tests to cover channel reuse, cancellation, fallbacks, disconnects, error events, and the 8 MiB boundary. The SFTP channel-count assertion was updated.
  • Frontend untouched: it still sends one request per file, all at once.
  • TypeScript check completed cleanly; backend Vitest passed 61/61 tests.
  • Measured 100 files / 32 MB through the plugin HTTP endpoint on one host: throughput improved from about 4.1–4.3 to 14–17 files/s at 100 concurrent requests, with all 100 files byte-exact. A user-timed 100-file download in the Termix app improved from 24.76 s to 9.7 s.
  • Known limits: measured on one host only; low-MaxSessions servers, browser saving of 100 files, and UI cancellation were not tested. During a 100-concurrent burst, directory listing took up to about 4.8 s (0.7 s at 16 concurrent, 0.25 s at 4); the old listing latency was not measured, so a regression is not established. Capping frontend concurrency (~16) is a possible separate follow-up.

Related Issues

  • None.

Screenshots / Demos

reuse.an.SFTP.channel.for.small.downloads.Demo.mp4

Checklist

  • Code follows the project style
  • Works on mobile and desktop (if it has UI)
  • Tests pass and new code has tests
  • Added the change to CHANGELOG.md (if the repo has one)
  • I have read the repo's CONTRIBUTING.md

Share one lazily opened download channel across small files, while keeping
large downloads on dedicated channels. Retire the shared channel if a
cancelled download stalls during close, and cover reuse, cancellation,
fallbacks, and the 8 MiB boundary in tests. For 100 files, measured
throughput improved from ~4.1–4.3 to ~14–17 files/s; an app-timed download
improved from 24.76 s to 9.7 s.
@termix-triage-bot

termix-triage-bot Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

AI review | Looks safe to merge

Implements per-session SFTP channel reuse for small files to improve bulk download throughput, with proper fallbacks, cancellation handling, and comprehensive tests. One minor stream-state check may need adjustment for production Node streams. No security or rule violations.

  • minor src/backend/download-routes.ts:412 readStream.closed check may prevent channel retirement on client disconnect. Calling readStream.destroy() sets .closed to true in standard Node streams, so !readStream.closed evaluates to false and skips discardBulkChannel. Consider tracking successful completion explicitly or checking readStream.readableEnded instead.

Reviewed d4afb96. AI can be wrong, a maintainer still decides. Comment /termix review to run it again.

Add a regression test that a finished download keeps the shared channel
past the 2 s grace timer and that the next download reuses it. Clarify
the comments on the no-op release for shared channels and on the
readStream.closed check, after triage-bot review.
@deanxbox

Copy link
Copy Markdown
Author
  • major src/backend/download-routes.ts:412 readStream.closed is not a standard Node.js stream property. It should likely be readStream.destroyed or readStream.readableEnded === false. Because .closed is undefined, the condition !readStream.closed always evaluates to true, which triggers discardBulkChannel after every successful download and defeats the channel reuse optimization.

Readable.closed is a standard Node stream property. It was added in Node 18, and this repo requires Node ≥ 22.12. It is false while the stream is open and becomes true once the close event fires. I confirmed this on Node 24.15: false before close, true at the close event.

So after a normal download !readStream.closed is false and the shared channel is not discarded. Reuse works as intended. The discard path only runs if the stream is still open when the 2 s grace timer fires after a cancel, which is the stalled-close case.

@LukeGus
LukeGus changed the base branch from main to dev-1.1.0 October 20, 2026 03:11
@LukeGus
LukeGus merged commit 6879844 into Termix-SSH:dev-1.1.0 Oct 20, 2026
2 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.

2 participants