Conversation
HgResumeRestApiServer.Execute is the single choke point for every call the resumable transport makes — getRevisions, pushBundleChunk, pullBundleChunk, finishPushBundle — and it was the one uninstrumented step on the send/receive path: hg launches and chunk handling already open spans, so anything profiling a push saw the HTTP time only as an unattributed remainder. Open a span here too, tagged with the method, the bytes each way and the HTTP status. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RevisionNumber's constructor called `hg log -rN --template "{node}"` to widen
the 12-character hash it had just been handed into the full 40-character node.
GetRevisionsFromQueryResultText builds one RevisionNumber per changeset, so a
single `hg log` listing N revisions was followed by N more hg processes that
told us nothing the first one couldn't have. Profiling a push of a 596-revision
FLEx project, those launches were 597 spans and 47% of the wall clock.
The detailed log template now asks for `{node}` alongside `{node|short}` and
the parser reads it, so the hash arrives with everything else. Two paths hand
us a full node to begin with — a parent's {p1node}, and hg debugancestor — and
those now use it directly instead of asking hg to repeat it. Whatever is left
(the untemplated `hg parent -r`, and external callers of the public parsing
method) falls back to the old lookup, but lazily, on first read of LongHash
rather than in the constructor. Nothing on the push, pull or check-sync path
reads LongHash without the template having filled it in, so in practice the
fallback never runs.
LongHash stays a public get/set property and the repository reference is a
private field, so revisioncache.json round-trips exactly as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`hg bundle` with no -t uses bzip2, the slowest compressor Mercurial ships. Measured on a 635MB FLEx project it took 108s; zstd at level 5 took 37s for 11% more bytes, and zstd at its own default level took 20s for 14% more. That trade pays for itself on any link faster than about 1MB/s and costs below it, which is why the spec and level are ordinary settable fields: a caller whose users are on genuinely poor connections can raise the level, or set the spec to null and get hg's default back. Both ends need the engine. zstd bundles have been readable since hg 4.1, and the hgresume server's `hg incoming`/`hg unbundle` handle them without knowing anything about how the bundle was made. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things were costing round trips per chunk on a resumable push. HttpWebRequest defaults to the 100-continue handshake, so every chunk body waited for the server to say go ahead. Measured on a 635MB push that was about 17% of the time spent in pushBundleChunk. Write buffering is deliberately left alone: turning it off made no measurable difference, and it would stop HttpWebRequest replaying the body when a server answers the first POST with a 401. PushStorageManager.GetChunk took whatever a single FileStream.Read returned. A short read there is legal even when the bundle has more to give, and it costs a whole extra request, since the client sends less than it asked the server to expect. Read until the buffer is full or the file ends. Chunk count on the same push fell from 66 to 41. InitialChunkSize is left at 5000 and now says why: the first chunk goes out before anything is known about the link and has to finish inside TimeoutInSeconds on the slow connections this protocol exists for, and CalculateChunkSize ramps away from it within a couple of round trips anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace the per-request HttpWebRequest in HgResumeRestApiServer with a process-wide static HttpClient, so the many small requests a resumable push/pull makes reuse keep-alive connections (the real win on modern .NET, where the client runs over SocketsHttpHandler). - Credentials are sent pre-emptively as a Basic Authorization header, so we never pay for a 401 challenge round trip. The resumable server accepts Basic auth on the first request, so PreAuthenticate's challenge-then-cache dance was pure overhead. - Per-call timeout is enforced with a CancellationTokenSource rather than HttpClient.Timeout, since each API method carries its own secondsBeforeTimeout; a timeout returns null as before. - ExpectContinue=false and ByteArrayContent (which sets Content-Length) keep the no-100-continue optimization and the Content-Length the server needs. - HttpClient does not throw on a non-2xx status, so RESET/FAIL responses flow through HandleResponse the way the old ProtocolError branch did. Only the x-hgr-* headers are copied into the WebHeaderCollection the response type expects, leaving HgResumeApiResponseHeaders and the test doubles untouched. HgResumeTransport's WebException safety nets now also catch HttpRequestException, which is what HttpClient throws for network failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hahn-kev
marked this pull request as draft
September 18, 2026 10:04
HttpClient drops our pre-emptive Authorization header when it follows a redirect. Give the handler credentials that read the current session, so it can answer the 401 challenge at the new location. They are only handed to hosts we were constructed for, and never on an https to http downgrade. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…rt-httpclient # Conflicts: # src/LibChorus/VcsDrivers/Mercurial/HgResumeRestApiServer.cs # src/LibChorus/VcsDrivers/Mercurial/PushStorageManager.cs
New LibChorus.IntegrationTests project runs ghcr.io/sillsdev/hgresume in a container via Testcontainers and drives push, pull and clone through HgRepository and the real HgResumeTransport. CI runs it on Linux only, since the Windows runners can't host Linux containers; locally the tests are ignored when Docker isn't available. Also use ASCII credentials in the redirect unit test: on .NET Framework the handler encodes challenge answers as Latin-1, as HttpWebRequest did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A process-wide HttpClient keeps connections, and their DNS answers, forever. HgRepository makes a new HgResumeRestApiServer for each push, pull or clone, so give each instance its own client: connections are still reused across one operation's chunks. The transport now disposes its server, and HgRepository disposes the transport. Redirect credentials are scoped to the instance's own server. Requests ask for HTTP/2, which an https server can accept via ALPN; plain http and servers without it fall back to 1.1. Not on .NET Framework, whose handler throws for 2.0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
hahn-kev
marked this pull request as ready for review
October 1, 2026 06:37
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.
Use the modern http client instead of the old HttpWebRequest. This should also enable the client to use http 2. But this is mostly around modernizing.
AI-generated summary
Replaces the per-request
HttpWebRequestinHgResumeRestApiServerwith a process-wide staticHttpClient:SocketsHttpHandler).Authorizationheader (UTF-8 encoded) — no 401 challenge round trip.CancellationTokenSource(each API method carries its ownsecondsBeforeTimeout); a timeout still returnsnull.ExpectContinue=false+ByteArrayContentkeep the no-100-continue optimization and theContent-Lengththe server needs.HttpClientdoesn't throw on a non-2xx status, so RESET/FAIL responses flow throughHandleResponsethe way the oldProtocolErrorbranch did. Only thex-hgr-*headers are copied into theWebHeaderCollectionthe response type expects, soHgResumeApiResponseHeadersand the test doubles are untouched.HgResumeTransport'sWebExceptionsafety nets now also catchHttpRequestException, which is whatHttpClientthrows for network failures.Redirects.
HttpClientdrops theAuthorizationheader when it follows a redirect, which the oldNetworkCredentialapproach handled by answering the 401 challenge. The handler now has aSessionCredentialsobject that does the same:HgResumeRestApiServerwas constructed for, and never on a downgrade from https to http, so a redirect elsewhere can't collect the password.CredentialCacheonly because both .NET and .NET Framework refuse any otherICredentialsafter a redirect; the cache stays empty and ourICredentials.GetCredentialanswers.HttpWebRequestalways did), so a non-ASCII password only works on the first, pre-emptive request there.Integration tests. New
LibChorus.IntegrationTestsproject runsghcr.io/sillsdev/hgresume:v2026-08-24via Testcontainers and drives push, multi-chunk push, clone and pull throughHgRepositoryand the real transport. CI runs it on the Ubuntu job only, since the Windows runners can't host Linux containers. Locally it's ignored when Docker isn't available;HGRESUME_IMAGEoverrides the image.Test plan
net462andnetstandard2.0; CI green on Ubuntu net8.0, Windows net462 and Windows net8.0.HgResumeRestApiServerTestspass on net8.0 and net462, including new tests for the UTF-8 Basic header, a 307 redirect that re-authenticates and keeps the POST body, and the host/https credential rules.LibChorus.IntegrationTests(5 tests) pass against the hgresume container locally and in CI.api pushBundleChunk/pullBundleChunktrace spans. No regression against the known baseline; cost is dominated byhg bundle/hg unbundleand raw chunk transfer, not the per-request overhead this change reduces.🤖 Generated with Claude Code
This change is