From 2c8af3691a4e4cce17c30fdcb6a5acec05e93ae0 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 23 Oct 2026 08:01:16 +0000 Subject: [PATCH 1/3] Send bundle chunks straight from the file instead of buffering them pullBundleChunk read each chunk into a byte array and then wrote that array as the response, so a slow client pinned the whole chunk in server memory for as long as its download took. Now the response carries the file region (path, offset, count) and RestDispatcher sends it with SendFileAsync, which uses sendfile where the platform has it and never holds the chunk in managed memory. Headers, Content-Length and the wire format are unchanged, so existing clients are unaffected; clients that ask for the rest of the bundle in one request no longer cost the server that much memory per download. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01LLSihB4ffNi95ioJQEvsLr --- csharp/src/HgResume.Api/HgResumeApi.cs | 40 ++++++++------------- csharp/src/HgResume.Api/HgResumeResponse.cs | 8 +++++ csharp/src/HgResume.Api/RestDispatcher.cs | 18 +++++++--- 3 files changed, 35 insertions(+), 31 deletions(-) diff --git a/csharp/src/HgResume.Api/HgResumeApi.cs b/csharp/src/HgResume.Api/HgResumeApi.cs index fc214d0..49b00ce 100644 --- a/csharp/src/HgResume.Api/HgResumeApi.cs +++ b/csharp/src/HgResume.Api/HgResumeApi.cs @@ -277,14 +277,14 @@ public async Task PullBundleChunkInternalAsync(string repoId, { if (CanGetChunkBelowBundleSize(bundleFilename, chunkSize, offset)) { - byte[] data = await GetChunkAsync(bundleFilename, chunkSize, offset, ct); + var data = GetChunkRegion(bundleFilename, chunkSize, offset); response.Values = new Dictionary { ["bundleSize"] = new FileInfo(bundleFilename).Length.ToString(), - ["chunkSize"] = data.Length.ToString(), + ["chunkSize"] = data.Count.ToString(), ["transId"] = transId, }; - response.Content = data; + response.BodyFile = data; return response; // break out of loop and switch (PHP: break 2) } await Task.Delay(2000, ct); @@ -293,15 +293,15 @@ public async Task PullBundleChunkInternalAsync(string repoId, break; case BundleHelper.State_Downloading: - byte[] chunk = await GetChunkAsync(bundleFilename, chunkSize, offset, ct); + var chunk = GetChunkRegion(bundleFilename, chunkSize, offset); long size = new FileInfo(bundleFilename).Length; response.Values = new Dictionary { ["bundleSize"] = size.ToString(), - ["chunkSize"] = chunk.Length.ToString(), + ["chunkSize"] = chunk.Count.ToString(), ["transId"] = transId, }; - response.Content = chunk; + response.BodyFile = chunk; if (offset > size) { throw new ValidationException( @@ -329,28 +329,16 @@ private static bool CanGetChunkBelowBundleSize(string filename, int chunkSize, i return fi.Exists && offset + chunkSize < fi.Length; } - private static async Task GetChunkAsync(string filename, int chunkSize, int offset, - CancellationToken ct) + /// + /// The part of the bundle file to send for a chunk request: from offset, the smaller of chunkSize and + /// what the file holds past the offset right now (empty if nothing). It is sent straight from the + /// file (see RestDispatcher), so the chunk is never held in memory, whatever its size. + /// + private static HgResumeResponse.FileRegion GetChunkRegion(string filename, int chunkSize, int offset) { var fi = new FileInfo(filename); - if (!fi.Exists || offset >= fi.Length) - { - return Array.Empty(); - } - await using var fs = new FileStream(filename, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, - bufferSize: 4096, FileOptions.Asynchronous); - fs.Seek(offset, SeekOrigin.Begin); - int toRead = (int)Math.Min(chunkSize, fs.Length - offset); - var buffer = new byte[toRead]; - int read = 0; - while (read < toRead) - { - int n = await fs.ReadAsync(buffer.AsMemory(read, toRead - read), ct); - if (n == 0) break; - read += n; - } - if (read < toRead) Array.Resize(ref buffer, read); - return buffer; + long available = fi.Exists ? fi.Length - offset : 0; + return new HgResumeResponse.FileRegion(filename, offset, Math.Max(0, Math.Min(chunkSize, available))); } // ---- misc ----------------------------------------------------------------------------------- diff --git a/csharp/src/HgResume.Api/HgResumeResponse.cs b/csharp/src/HgResume.Api/HgResumeResponse.cs index 476b9b5..89fd170 100644 --- a/csharp/src/HgResume.Api/HgResumeResponse.cs +++ b/csharp/src/HgResume.Api/HgResumeResponse.cs @@ -28,6 +28,14 @@ public sealed class HgResumeResponse public byte[] Content { get; set; } public int Version { get; set; } + /// + /// A region of a file to send as the body instead of , so a bundle chunk of any + /// size goes out without first being read into memory. Null when the body is . + /// + public FileRegion? BodyFile { get; set; } + + public sealed record FileRegion(string Path, long Offset, long Count); + public HgResumeResponse(int code, Dictionary? values = null, byte[]? content = null, int version = ApiConfig.ApiVersion) { diff --git a/csharp/src/HgResume.Api/RestDispatcher.cs b/csharp/src/HgResume.Api/RestDispatcher.cs index 79042c1..d037e79 100644 --- a/csharp/src/HgResume.Api/RestDispatcher.cs +++ b/csharp/src/HgResume.Api/RestDispatcher.cs @@ -145,12 +145,20 @@ private async Task SendResponseAsync(HttpContext context, HgResumeResponse respo } res.ContentType = "application/octet-stream"; - if (response.Content.Length > 0) + // Set Content-Length explicitly (as the PHP RestServer did). The Chorus client reads the + // body with a hand-rolled reader that returns EMPTY when there is no Content-Length header + // (it never falls back to chunked/Transfer-Encoding), so relying on Kestrel's default + // chunked encoding silently breaks getRevisions and pullBundleChunk for the real client. + if (response.BodyFile is { Count: > 0 } file) + { + // Sent straight from the file (sendfile where the platform has it), so a chunk as big as + // the whole bundle costs no memory for the whole time a slow client takes to download it. + // The file is opened with FileShare.ReadWrite, so a bundle hg is still writing can be read. + res.ContentLength = file.Count; + await res.SendFileAsync(file.Path, file.Offset, file.Count, ct); + } + else if (response.Content.Length > 0) { - // Set Content-Length explicitly (as the PHP RestServer did). The Chorus client reads the - // body with a hand-rolled reader that returns EMPTY when there is no Content-Length header - // (it never falls back to chunked/Transfer-Encoding), so relying on Kestrel's default - // chunked encoding silently breaks getRevisions and pullBundleChunk for the real client. res.ContentLength = response.Content.Length; await res.Body.WriteAsync(response.Content, ct); } From 40d433953d4c4a11237689416aa5c4e61cc496fc Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 23 Oct 2026 15:54:48 +0700 Subject: [PATCH 2/3] Test against pinned, latest and Mercurial 3 Chorus clients in CI The integration tests drove one hard-coded LibChorus. They now take -p:ChorusClient=pinned|latest|hg3, and CI runs each one as a matrix leg against the same image: - pinned: LibChorus 6.0.0-beta0064 with Mercurial 6, the last release before the resumable transport moved from HttpWebRequest to HttpClient (sillsdev/chorus#399). The default, and the only leg that runs the HTTP-level tests too. - latest: the newest LibChorus, Mercurial and SIL.Core on NuGet. - hg3: LibChorus 6.0.0-beta0041 with Mercurial 3.0.3.11, as LexBox tested. That Chorus drops the port from the repo URL (fixed in sillsdev/chorus#343), so ServerFixture gains HGRESUME_HOST_PORT and this leg publishes the server on port 80. Its Linux hg needs python2. Each client gets its own hg drop and output dir, so switching between them locally never mixes Mercurial files. CI now builds the image once, hands it to the test legs as an artifact, and pushes only after every leg passes. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/docker-image.yml | 133 +++++++++++++----- csharp/.gitignore | 3 - .../HgResume.IntegrationTests.csproj | 58 ++++++-- .../ServerFixture.cs | 12 +- 4 files changed, 159 insertions(+), 47 deletions(-) diff --git a/.github/workflows/docker-image.yml b/.github/workflows/docker-image.yml index e6f50c3..e0cd8e2 100644 --- a/.github/workflows/docker-image.yml +++ b/.github/workflows/docker-image.yml @@ -12,13 +12,110 @@ env: IMAGE_NAME: ${{ github.repository }} jobs: - build-test-push: - name: Build image, run tests, then push + # Build the amd64 image once and hand it to every test leg as an artifact (a multi-arch image can't + # be loaded/run directly). The push job reuses this build's cache, so amd64 is not compiled twice. + build: + name: Build image + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v4 + + - name: Build amd64 image + uses: docker/build-push-action@v7 + with: + context: csharp + file: csharp/Dockerfile + platforms: linux/amd64 + tags: hgresume-csharp:test + outputs: type=docker,dest=${{ runner.temp }}/hgresume-image.tar + cache-to: type=gha,mode=max + cache-from: type=gha + + - name: Upload image + uses: actions/upload-artifact@v7 + with: + name: hgresume-image + path: ${{ runner.temp }}/hgresume-image.tar + retention-days: 1 + + # Run the integration tests against the image once per Chorus client (see ChorusClient in + # HgResume.IntegrationTests.csproj). The HTTP-level tests don't depend on the client, so only the + # pinned leg runs them; the other legs run just the tests that drive the real Chorus client. + test: + name: Integration tests (${{ matrix.client }} Chorus) + needs: build + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - client: pinned + filter: '' + - client: latest + filter: FullyQualifiedName~SendReceiveTests|FullyQualifiedName~VerifyHgWorking + # Chorus this old drops the port from the repo URL, so the server has to be on port 80. + - client: hg3 + filter: FullyQualifiedName~SendReceiveTests|FullyQualifiedName~VerifyHgWorking + host-port: '80' + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Download image + uses: actions/download-artifact@v8 + with: + name: hgresume-image + path: ${{ runner.temp }} + + - name: Load image + run: docker load --input "${{ runner.temp }}/hgresume-image.tar" + + - name: Set up .NET + uses: actions/setup-dotnet@v6 + with: + dotnet-version: '10.0.x' + + # The Mercurial 3 bundle's Linux hg is a #!/usr/bin/python2 script, and ubuntu-latest has no + # Python 2 (same setup as LexBox's integration-test.yaml). + - name: Set up Python 2 for Mercurial 3 + id: python + if: matrix.client == 'hg3' + uses: MatteoH2O1999/setup-python@v6 + with: + python-version: '2.7.18' + cache-build: true + allow-build: allow + + - name: Link python2 + if: matrix.client == 'hg3' + run: sudo ln -s "${{ steps.python.outputs.python-path }}" /usr/bin/python2 + + - name: Run integration tests against the image + working-directory: csharp + env: + # The test fixture drives the pre-built image itself via Testcontainers (against the + # runner's local Docker daemon), so we skip the fixture's own build step. + HGRESUME_IMAGE: hgresume-csharp:test + HGRESUME_SKIP_BUILD: '1' + HGRESUME_HOST_PORT: ${{ matrix.host-port }} + TEST_FILTER: ${{ matrix.filter }} + run: | + args=(test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj + "-p:ChorusClient=${{ matrix.client }}" --logger "console;verbosity=normal") + if [ -n "$TEST_FILTER" ]; then args+=(--filter "$TEST_FILTER"); fi + dotnet test "${args[@]}" + + push: + name: Build and push multi-arch image + needs: test runs-on: ubuntu-latest permissions: contents: read packages: write - steps: - name: Checkout uses: actions/checkout@v7 @@ -47,34 +144,6 @@ jobs: type=ref,event=pr type=raw,value=${{ steps.version.outputs.value }},enable=${{ github.event_name != 'pull_request' }} - # Build the amd64 image and load it into the local Docker daemon so the test fixtures can run it - # as a container (a multi-arch image can't be loaded/run directly). The multi-arch push step below - # reuses this build's cache, so amd64 is not compiled twice. - - name: Build and export to Docker (amd64) - uses: docker/build-push-action@v7 - with: - context: csharp - file: csharp/Dockerfile - load: true - platforms: linux/amd64 - tags: hgresume-csharp:test - cache-to: type=gha,mode=max - cache-from: type=gha - - - name: Set up .NET - uses: actions/setup-dotnet@v6 - with: - dotnet-version: '10.0.x' - - - name: Run integration tests (HTTP-level + real Chorus client) against the image - working-directory: csharp - env: - # The test fixture drives the pre-built image itself via Testcontainers (against the - # runner's local Docker daemon), so we skip the fixture's own build step. - HGRESUME_IMAGE: hgresume-csharp:test - HGRESUME_SKIP_BUILD: '1' - run: dotnet test test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj --logger "console;verbosity=normal" - - name: Log in to the Container registry # Fork PRs get a read-only GITHUB_TOKEN; skip login/push so the job still builds without a 403. if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository @@ -84,7 +153,7 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - # Multi-arch build + push. amd64 layers come from the cache populated above (no recompile); only + # Multi-arch build + push. amd64 layers come from the build job's cache (no recompile); only # arm64 is built here. Push is gated the same way as login. - name: Build and push Docker image (C#) uses: docker/build-push-action@v7 diff --git a/csharp/.gitignore b/csharp/.gitignore index c89ffcc..b89a9fb 100644 --- a/csharp/.gitignore +++ b/csharp/.gitignore @@ -4,6 +4,3 @@ obj/ .idea/ .vs/ -# Bundled Mercurial dropped into the integration test project by SIL.Chorus.Mercurial at build time -test/HgResume.IntegrationTests/Mercurial/ -test/HgResume.IntegrationTests/MercurialExtensions/ diff --git a/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj b/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj index 6423148..bdc8196 100644 --- a/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj +++ b/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj @@ -21,16 +21,56 @@ enable false true - - $(MSBuildProjectDirectory) $(NoWarn);NU1701;NU1902;NU1903 + + pinned + bin/$(Configuration)/$(ChorusClient)/ + + $(MSBuildProjectDirectory)/obj/hg/$(ChorusClient) + + + + 6.0.0-beta0064 + 6.5.1.43 + 17.0.0 + + + + + *-* + * + *-* + + + + + 6.0.0-beta0041 + 3.0.3.11 + 12.0.0 + + + + + + + + - - - + + + @@ -47,9 +87,9 @@ - <_HgExtDrop Include="$(MSBuildProjectDirectory)/MercurialExtensions/**" /> - <_HgBinDrop Include="$(MSBuildProjectDirectory)/Mercurial/**" + <_HgExtDrop Include="$(Mercurial4ChorusDestDir)/MercurialExtensions/**" /> + <_HgBinDrop Include="$(Mercurial4ChorusDestDir)/Mercurial/**" Condition="$([MSBuild]::IsOSPlatform('Windows'))" /> public sealed class ServerFixture : IAsyncLifetime { @@ -95,9 +98,12 @@ public async Task InitializeAsync() image = new DockerImage(_image); } - _container = new ContainerBuilder(image) - .WithName(ContainerName) - .WithPortBinding(ContainerPort, assignRandomHostPort: true) + string? hostPort = Environment.GetEnvironmentVariable("HGRESUME_HOST_PORT"); + var builder = new ContainerBuilder(image).WithName(ContainerName); + builder = string.IsNullOrWhiteSpace(hostPort) + ? builder.WithPortBinding(ContainerPort, assignRandomHostPort: true) + : builder.WithPortBinding(ushort.Parse(hostPort), ContainerPort); + _container = builder .WithEnvironment("HGRESUME_MANAGE_SECRET", ManageSecret) .WithCleanUp(!keep) .WithWaitStrategy(Wait.ForUnixContainer() From a3027ca052914133f8e04f8738b254daa830e88f Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 23 Oct 2026 16:00:57 +0700 Subject: [PATCH 3/3] Stage the bundled hg into the test output on every platform Chorus only looks for Mercurial/ in the test assembly's dir and the two above it. On Linux the tests had been finding the hg drop in the project dir, three levels above bin/Debug/net10.0. With the per-client output dir and hg drop dir that no longer lines up, so copy Mercurial/ into the output dir on Linux too, as on Windows. Co-Authored-By: Claude Opus 5.5 --- .../HgResume.IntegrationTests.csproj | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj b/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj index bdc8196..843165b 100644 --- a/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj +++ b/csharp/test/HgResume.IntegrationTests/HgResume.IntegrationTests.csproj @@ -11,9 +11,7 @@ needs the whole FieldWorks stack and never touches the server), so we only depend on LibChorus + the bundled Mercurial. Drives the container via Testcontainers, which needs a Docker-API-compatible endpoint (Docker Desktop/Engine work out of the box; podman needs its API socket exposed and - DOCKER_HOST pointed at it). Runs in Linux CI: the checked-in Windows Mercurial bundle below is - staged on Windows only, so on Linux the cross-platform hg that SIL.Chorus.Mercurial ships is used - instead. + DOCKER_HOST pointed at it). Runs on Windows and in Linux CI. --> net10.0 @@ -89,25 +87,22 @@ SIL.Chorus.Mercurial's CopyFiles target drops the platform hg (Mercurial/) and the cross-platform fixutf8 extension (MercurialExtensions/, from runtimes/any) into $(Mercurial4ChorusDestDir) during Build. That happens after project evaluation, so a static glob misses the files on a - clean build. Stage them into the output dir here, after CopyFiles has populated it, - so Chorus finds MercurialExtensions/fixutf8 at runtime (it resolves it relative to the test - assembly). On Linux the linux-x64 hg is already copied to output by NuGet's runtime-asset handling - (VerifyHgWorking confirms it), so only MercurialExtensions needs staging there; the win hg has no - standard RID folder and is only staged on Windows. + clean build. Stage them into the output dir here, after CopyFiles has populated it. Chorus + resolves MercurialExtensions/fixutf8 relative to the test assembly, and looks for Mercurial/ only + in the assembly's dir, grandparent and great-grandparent (MercurialLocation), so both must be in + the output dir on every platform. --> <_HgExtDrop Include="$(Mercurial4ChorusDestDir)/MercurialExtensions/**" /> - <_HgBinDrop Include="$(Mercurial4ChorusDestDir)/Mercurial/**" - Condition="$([MSBuild]::IsOSPlatform('Windows'))" /> + <_HgBinDrop Include="$(Mercurial4ChorusDestDir)/Mercurial/**" /> + SkipUnchangedFiles="true" />