fix(common): bound the channels Context::map opens, not just its concurrency - #449
Open
mpzFixApplier wants to merge 1 commit into
Open
fix(common): bound the channels Context::map opens, not just its concurrency#449mpzFixApplier wants to merge 1 commit into
mpzFixApplier wants to merge 1 commit into
Conversation
…ncurrency
`Context::map` gives every item its own child context, and therefore its own
multiplexer channel. `buffered(concurrency_limit)` bounds how many items run at
once, but the channel IDs are still per item, so a `map` over N items opens N
channels over its lifetime.
That is enough to exhaust a multiplexer which tracks channels rather than
in-flight work. A mux frees a channel when its stream is dropped, but that
release is processed by the connection task and lags behind the rate at which
the sliding window opens new ones, so the live-channel count keeps climbing on a
large enough workload. Against tlsn's mux (512 streams, `crates/tlsn/src/session.rs`)
an MPC-TLS session with a ~7KB `max_sent_data` budget fails this way in roughly
half of all runs, with `context mux error` — whose source, `maximum number of
streams reached`, is hidden by `ContextError`'s `Display`.
Distribute the items round-robin over at most `concurrency_limit` lanes instead,
each lane owning one child context and processing its items sequentially. The
number of channels a `map` ever opens becomes `min(items.len(), concurrency_limit)`,
independent of the workload, and that is also still the concurrency bound, so
`concurrency_limit` keeps its meaning and its default.
Both parties derive lanes from the item index alone (`index % lanes`), so they
agree on the channel layout and on the per-channel message order without any
extra coordination. Results are still returned in input order.
Measured with tlsn alpha.15 driving loopback MPC-TLS at a given `max_sent_data`,
everything else equal:
buffered window, MAX_SENT=7168: 7/15 runs ok
round-robin lanes, MAX_SENT=7168: 12/12 ok
MAX_SENT=8192: 12/12 ok
MAX_SENT=16384: 10/10 ok
`cargo test --workspace` passes; the existing `test_map_respects_concurrency_limit`
still holds (lanes bound concurrency too), and a two-party recording test pins the
channel count to the lane count.
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.
Fixes the channel exhaustion described in #448
The problem
Context::mapgives every item its own child context, and therefore its own multiplexer channel.buffered(concurrency_limit)bounds how many items run at once, but the channel IDs are still per item, so a map over N items opens N channels over its lifetime.That is enough to exhaust a mux which tracks channels rather than in-flight work. A mux frees a channel when its stream is dropped, but that release is processed by the connection task and lags behind the rate at which the sliding window opens new ones, so the live-channel count keeps climbing. Against tlsn's mux (512 streams) an MPC-TLS session with a ~7 KB
max_sent_databudget fails this way in roughly half of all runs, surfacing ascontext mux errorwhose hidden source ismaximum number of streams reached.The change
Distribute items round-robin over at most
concurrency_limitlanes, each lane owning one child context and processing its items sequentially.min(items.len(), concurrency_limit), independent of workload size.concurrency_limitkeeps its meaning — and its default of 32 is unchanged here.index % lanes, derived from the item index alone, so both parties agree on the channel layout and per-channel message order with no extra coordination.Measurements
tlsn alpha.15 driving loopback MPC-TLS at a given
max_sent_data, everything else equal:bufferedwindowTests
cargo test --workspacepasses. The existingtest_map_respects_concurrency_limitstill holds unchanged (lanes bound concurrency as well), and a two-party recording test is added that maps far more items than the limit and pins the opened-channel count to the lane count while checking both sides stay in lockstep and results come back in input order.Changelog
Added under
## [Unreleased]->### Fixed.