Skip to content

Pull the whole bundle in one streamed request over hgresume - #406

Draft
hahn-kev-bot wants to merge 7 commits into
masterfrom
claude/resumable-request-patches-1725c0
Draft

hahn-kev-bot wants to merge 7 commits into
masterfrom
claude/resumable-request-patches-1725c0

Conversation

@hahn-kev-bot

@hahn-kev-bot hahn-kev-bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Important

Stacked on #404, which fixes restore against libpalaso 18.0.0; retarget to master once that merges.
Waiting on sillsdev/hgresume#31. Each pull request now asks for the rest of the bundle, and until that PR ships the server reads all of it into memory and holds it for as long as the client takes to download it.

AI summary

Summary

Over the resumable transport, a pull used to fetch the bundle in chunks sized to finish inside the 30 second timeout. Now it fetches it in as few requests as the server allows, writing the body to the bundle file as it arrives:

 Pull
   loop
-    ask for chunkSize bytes (sized to take ~7 s on this link)
-    buffer the whole chunk, then append it to the bundle file
+    nothing known yet             -> ask for 1 byte (brings back the bundle size so far)
+    server has more than we do    -> ask for all of it but the last byte (served at once)
+    caught up                     -> ask for the rest, streamed to the bundle file
+    "in progress" reply           -> probe again (it doesn't say how far the server got)
+    connection cut / stall > 30 s -> next request resumes at the end of the file
+    Cancel                        -> checked as each piece of the body arrives
  • The 30 s timeout now bounds each wait for data, not the whole request, so a slow link that keeps delivering is never cut off.
  • IApiServer gains an Execute overload that streams a 200 body to a Stream, with a progress callback.
  • The progress line shows an ETA from the measured rate, updated at most once a second.
  • Push is unchanged.
  • Integration tests can run against a server already running on the machine (HGRESUME_URL, HGRESUME_REPO_ROOT). They add a 3 MB clone, and a clone through a proxy that cuts the connection four times.

Evidence

  • Timing, against hgresume built from source (measured by the author of the second commit):
    Before: a pull of one small change took 22 s, and a 3 MB clone took 37 s.
    After: the small pull takes three quick requests, and the clone takes 9 s.
  • Cancel during a long download:
    Before: Pull_UserCancelsMidBody_StopsAndNextPullResumesFromBytesReceived fails. The cancelled request runs to the end, and the next pull resumes at byte 1049399.
    After: it passes. The download stops half way, and the next pull resumes at byte 524700.
  • "In progress" reply once caught up:
    Before: Pull_InProgressWhenCaughtUp_ProbesNext fails because the next request asks for the rest again (chunkSize 2146434248), which waits out the server's 14 s poll every time.
    After: it passes. The next request is a 1-byte probe.
  • All 71 HgResume tests in LibChorus.Tests pass on net8.0.

Merge Danger

Door: two-way

The client change needs no protocol change, and reverting it restores chunked pulls. A bundle half-downloaded by the new client resumes fine under the old one, because both resume from the end of the file.

Blast Radius: every resumable pull

  • Old PHP server: asking for the rest of the bundle sends a chunkSize of about 2 GB. The C# server limits that to what is left of the bundle. The old PHP server passes it to fread, which allocates the full size, over the 128M memory_limit. Production needs to be on the C# image.
  • Buffering proxies and antivirus: a proxy or antivirus scanner that holds a whole response before passing it on could keep the client from seeing data within 30 s. On a large bundle that could retry forever. A per-request size cap would prevent this; it is left out for now.
  • Retries: failures and timeouts still retry immediately, with no backoff and no limit, as before. Each retry now carries on from the bytes already received.
  • Cancel during a stall: a stalled read notices Cancel only when the 30 s idle timeout fires.

Test plan

🤖 Generated with Claude Code


This change is Reviewable

imnasnainaec and others added 2 commits October 21, 2026 14:24
The floating 18.0.0-* libpalaso references started resolving to stable
18.0.0, whose minimums for L10NSharp, icu.net, System.Resources.Extensions
and NUnit are above what Chorus pinned, so restore failed with NU1605.
Raise those pins and pin libpalaso to 18.0.0.

L10NSharp 11 no longer pulls in System.ServiceModel, so Chorus references
it directly.

Security: System.ServiceModel.Primitives 4.8.1 -> 10.1.0 drops the
vulnerable System.Security.Cryptography.Xml chain from LibChorus's
netstandard2.0 graph (4.10.x and 8.x still carry vulnerable versions).
NuGet.CommandLine 6.7.0 -> 6.14.3 in l10n.proj (GHSA-68w7-72jg-6qpp).

l10n.proj referenced a nonexistent PkgGitVersion_GitVersion_MsBuild
property, so its manual GitVersion import never ran (NuGet already
imports those targets) and RestartBuild was always true.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ChorusHub.wixproj installs its own SIL.BuildTasks via nuget.exe for the
FileUpdate task that stamps the license year, separate from the
PackageReference in ChorusHub.csproj.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Test Results

       9 files  ±  0     353 suites  +6   52m 2s ⏱️ - 3m 34s
1 062 tests +20  1 005 ✔️ +19    56 💤 ±0  1 ❌ +1 
3 345 runs  +56  3 221 ✔️ +55  123 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit b755902. ± Comparison against base commit 050c057.

This pull request removes 1 and adds 21 tests. Note that renamed tests count towards both.
Pull_BundleInMultipleChunks_Success
CalculateEstimatedTimeRemaining_FromRate_ExpectedString(0,1000.0d,"(less than 1 minute)")
CalculateEstimatedTimeRemaining_FromRate_ExpectedString(1000,0.0d,"")
CalculateEstimatedTimeRemaining_FromRate_ExpectedString(300000,1000.0d,"(about 5 minutes)")
CalculateEstimatedTimeRemaining_FromRate_ExpectedString(59000,1000.0d,"(less than 1 minute)")
CalculateEstimatedTimeRemaining_FromRate_ExpectedString(60000,1000.0d,"(about 1 minute)")
ChunkSizeToAskFor_CaughtUp_TheRest
ChunkSizeToAskFor_HaveItAll_TheRest
ChunkSizeToAskFor_NothingKnownButDataOnDisk_OneByteProbe
ChunkSizeToAskFor_NothingKnown_OneByteProbe
ChunkSizeToAskFor_ServerHasMoreThanOffset_AllButTheLastByte
…

♻️ This comment has been updated with latest results.

hahn-kev and others added 3 commits October 23, 2026 17:44
The resumable transport used to pull a bundle in chunks sized to finish
inside a 30 second request timeout, so a slow link spent much of its time
on round trips and a chunk that timed out was thrown away. Now a pull
asks the server for everything after what is already on disk, in one
pullBundleChunk request, and writes the body into the bundle file as it
arrives. The timeout bounds each wait for data rather than the whole
download, and when the connection drops the next request resumes from
the bytes that did arrive. No server change is needed: the server already
serves any offset and caps chunkSize at what is left of the bundle.

IApiServer gains an Execute overload that streams the response body to a
Stream, with a progress callback; non-200 responses are still buffered.
The pull progress line is now rate based and updated at most once a
second. PushStorageManager.GetChunk never allocates more than the bundle
has left, and PullStorageManager can open the bundle for writing from an
offset, truncating anything past it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLSihB4ffNi95ioJQEvsLr
Tested against the hgresume server built from source: asking for the
whole bundle in the first request cost every pull a 14 second wait,
because the server only answers at once when it can satisfy the request
from the bundle file as it stands, and otherwise polls for 14 seconds
before answering "in progress". A pull of one small change took 22 s.

So the client now asks for one byte first (served as soon as the bundle
file starts, and it brings back the size so far), then for all but the
last byte of what the server has (served at once, even while the server
is still adding to the file), and only then for the rest in one streamed
request. The same small pull takes three quick requests, and a 3 MB clone
dropped from 37 s to 9 s. A cut connection still resumes from the bytes
received; the body length of the cut response is kept as the bundle size
seen so far.

The integration tests can now run against a server already running on
the machine (HGRESUME_URL and HGRESUME_REPO_ROOT) instead of the
container, and gain a large-bundle clone and a clone through a proxy
that cuts the connection four times part-way through the download.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LLSihB4ffNi95ioJQEvsLr
While the server is still making a bundle, a request for the rest of it
waits out the server's whole 14 second poll and comes back "in progress"
without saying how far the server has got. The client then asked for the
rest again, so a long clone downloaded nothing more until the bundle was
finished. Now an "in progress" reply sends the client back to a one byte
probe, which the server answers as soon as the bundle has grown, so the
download keeps up with the server again.

A single request can now carry the whole bundle, so Cancel, which was
only checked between requests, could do nothing for the length of the
download. It is now checked as the body arrives; the bytes received stay
in the bundle file and the next pull resumes after them.

Also abort the stalled-body test's response, since HttpListener on
Windows refuses to close a response that is short of its Content-Length.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hahn-kev
hahn-kev force-pushed the claude/resumable-request-patches-1725c0 branch from 76eb8a2 to b755902 Compare October 23, 2026 10:46
@hahn-kev-bot
hahn-kev-bot changed the base branch from master to chore/sil-tasks-4 October 23, 2026 10:46
hahn-kev and others added 2 commits October 26, 2026 11:30
The progress reporter's throttle started _lastReportMs at long.MinValue,
so now - _lastReportMs overflowed to a negative number and every update
was skipped until the whole bundle had arrived. On a slow link the bar
and ETA sat still for the entire download. Start it one interval in the
past instead, so the first report goes out.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
CopyBody turned any IOException into HttpRequestException, including one
from writing the bundle file. On a full disk the pull took that for a
dropped connection and retried forever, asking the server for the rest of
the bundle each time. Only failures reading the response are now treated
as connection errors; a write error goes up to the caller as it is, and
the partial bundle stays on disk for the next pull to resume.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Base automatically changed from chore/sil-tasks-4 to master October 26, 2026 12:35
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.

3 participants