Skip to content

libstore/filetransfer: Pause transfers instead of stalling the download thread#14614

Merged
Ericson2314 merged 5 commits intomasterfrom
libcurl-pause
Nov 22, 2025
Merged

libstore/filetransfer: Pause transfers instead of stalling the download thread#14614
Ericson2314 merged 5 commits intomasterfrom
libcurl-pause

Conversation

@xokdvium
Copy link
Contributor

@xokdvium xokdvium commented Nov 22, 2025

Motivation

Instead of naively stalling the download thread we can instead stop the transfer.
This allows the other multiplexed connections to continue downloading (and unpacking),
if the result of the download gets piped into a GitFileSystemObjectSink.

Prior art in lix project:

This patch is very different from the lix one, since we are using a decompression sink
in the middle of the pipeline but the co-authored-by is there since I was motivated to
implement this by looking at the lix side of things.

Co-authored-by: eldritch horrors pennae@lix.systems

Context

Resolves #11728


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Those can never be nullptr, so we should use the type system
to ensure this invariant.
This is necessary to make pausing/unpausing possible in a follow-up commit.
xokdvium and others added 3 commits November 22, 2025 04:23
…ad thread

Instead of naively stalling the download thread we can instead stop the transfer.
This allows the other multiplexed connections to continue downloading (and unpacking),
if the result of the download gets piped into a GitFileSystemObjectSink.

Prior art in lix project:

- https://git.lix.systems/lix-project/lix/commit/4ae6fb5a8f0d456b8d2ba2aaca3712b4e49057fc
- https://git.lix.systems/lix-project/lix/commit/12156d3beb8a16c0e2e8cf7180e1fbf27280a669

This patch is very different from the lix one, since we are using a decompression sink
in the middle of the pipeline but the co-authored-by is there since I was motivated to
implement this by looking at the lix side of things.

Co-authored-by: eldritch horrors <pennae@lix.systems>
Since the root cause (the lack of backpressure control) has
been fixed in the previous commit we can revert the change from
8ffea0a and make the default size much
smaller.
@xokdvium xokdvium added performance store Issues and pull requests concerning the Nix store fetching Networking with the outside (non-Nix) world, input locking labels Nov 22, 2025
@xokdvium
Copy link
Contributor Author

Needs more testing before merge, but I'm relatively confident this patch is good.

@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 22, 2025
Merged via the queue into master with commit 79dcc09 Nov 22, 2025
23 checks passed
@Ericson2314 Ericson2314 deleted the libcurl-pause branch November 22, 2025 06:22
@phip1611
Copy link
Member

Thanks for working in this! Does that mean that everyone who increased download-buffer-size should/can remove this once this change lands in their system?

@xokdvium
Copy link
Contributor Author

Does that mean that everyone who increased download-buffer-size should/can remove this once this change lands in their system?

Yes, that is no longer necessary. I've also decreased the default so as to not waste memory. There's no benefit to having a big buffer now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation fetching Networking with the outside (non-Nix) world, input locking performance store Issues and pull requests concerning the Nix store

Projects

None yet

Development

Successfully merging this pull request may close these issues.

warning: download buffer is full; consider increasing the 'download-buffer-size' setting (won't work)

3 participants