feat: [DSM-148] Larger XNet pool - #11343
Conversation
Increase the XNet pool soft byte limit from 10 MB to 128 MB. Apply a dynamic pooled slice size limit of 1/16 of the space left (8 MB or two XNet payloads' worth initially, less as it fills up), so per-slice limits taper. This is necessary in order to provide real fairness across XNet peers: with the earlier limits (10 MB pool size, fixed 4 MB max per slice) a couple of slices could fill the pool, particularly once we switched to adverts (the existing sweep at least gives everyone equal chances).
|
✅ No security or compliance issues detected. Reviewed everything up to 4442c87. Security Overview
Detected Code Changes
|
| // Maximum byte size of any one pooled slice, applied uniformly across slices: | ||
| // as the pool fills up, we aim to fill what is left of it with (more, but) | ||
| // smaller slices. | ||
| let slice_byte_size_max = pool_bytes_left / POOLED_SLICE_BYTE_SIZE_DIVISOR; |
There was a problem hiding this comment.
If I understand correctly, this means whenever bytes from one subnet are included in a block, the capacity in the pool increases for all other subnets. As a result, we will pull new data from all subnets and append them to their existing slices. Throughout the call to append we are holding the same lock that the payload builder needs to acquire.
How expensive is this append? I imagine if it takes very long then that would impact the finalization rate. Or is the assumption that, in general, cross-net traffic should be low?
There was a problem hiding this comment.
The limit for the next pull increases, yes. But I'm pretty sure that we rarely reach even the 10 MB limit, leave alone 128 MB.
As for how expensive it is, we decode a protobuf-encoded slice (although we could do that before grabbing the lock), take the messages (as blobs) and move them next to the messages already in the pool, then do some basic validation. Overall, not particularly cheap, but also not expensive enough to really matter. It's all in-memory shuffling of pointers.
Looking at it differently, given our maximum throughput of ~10 MB/s and a maximum of 2.5 new slices per second per subnet, this is far from being a bottleneck. Likely not even if you repeatedly appended one zero-payload message at a time at a rate of 10 MB/s.
There was a problem hiding this comment.
we rarely reach even the 10 MB limit, leave alone 128 MB
Then do we really need to increase the limit if we don't even expect to reach the current one?
It's all in-memory shuffling of pointers
To me it looks like we also verify a BLS signature, clone the data twice, and proto encode/decode a second time
given our maximum throughput of ~10 MB/s and a maximum of 2.5 new slices per second
I guess this assumes that consensus can keep up with the rate of ingestion and no XNet backlog is building up? To me the problem is that the complexity of append seems to be O(merged slice) and not O(suffix)
There was a problem hiding this comment.
Then do we really need to increase the limit if we don't even expect to reach the current one?
We do, because reaching the current limit means we cannot provide any kind of fairness. Whatever algorithm we use for fairly picking slices for the next block, will be forced to pick between 3 slices, with everyone else kept out of the pool. Even ignoring that, if we start with an empty pool; add a couple of huge slices and a whole lot of smaller ones; then we will essentially end up including those exact pool contents (in whatever order) before we can pool any new slices. So subnets producing a trickle of messages are necessarily limited by our throughput vs the volume of a couple of noisy streams.
To me it looks like we also verify a BLS signature, clone the data twice, and proto encode/decode a second time
You are, unfortunately, right. I had missed the decode_certified_stream_slice() call. I suppose we can split append() (and put()) into two steps: a first step that retrieves the existing slice under a readonly lock and does the work; and a second step that (under a read-write lock) conditionally replaces the pooled slice iff it has not changed in the meantime. It will break the abstraction, but I suppose it can't be helped. It's also something for a separate PR.
I guess this assumes that consensus can keep up with the rate of ingestion and no XNet backlog is building up?
It looks at the worst-case scenario (for the XNet pool) of Consensus running at a full 2.5 blocks/s, yes. And whether or not there's a backlog, its growth (or rather advance) would necessarily be limited to an average of 10 MB/s by payload builder throughput. There's not much point in the pool having a much higher worst-case throughput than that.
To me the problem is that the complexity of append seems to be O(merged slice) and not O(suffix)
Streams are limited to 10 MB (and 10k messages) on the sender side (also for reasons of fairness; if we were to dump 100 MB from a single canister into the stream, other canisters would have to wait minutes before their messages could be picked up; and given a maximum of 5 minutes TTL for bounded wait calls, minutes of waiting in either direction would cause most calls to time out).
So one can also look at this as constant time. For a reasonably large constant. (o;
But yeah, you are right. It's all down to verification (which we may choose to skip in the pool at the cost of making it impossible to pinpoint misbehaving peers) and particularly to doing said verification while holding the lock on the pool.
There was a problem hiding this comment.
Streams are limited to 10 MB
Ah this is something I missed, then maybe it isn't as bad as I thought. Still could be interesting to have some benchmarks to see how the (new) limits behave under higher load than we might see today. But I agree it could be done in a different PR.
| pub const POOL_SLICE_BYTE_SIZE_MAX: usize = 4 << 20; | ||
| /// Hard maximum slice size in bytes. We only pool up to 8 MB (two blocks' worth | ||
| /// of payload) from any one stream. | ||
| pub const POOLED_SLICE_BYTE_SIZE_MAX: usize = 8 << 20; |
There was a problem hiding this comment.
Claude tells me there are some histogram buckets that we should update for this new limit
There was a problem hiding this comment.
Also some grafana panels that make assumptions about xnet_pool_size_bytes
There was a problem hiding this comment.
I was only able to find two histograms that track stream slice sizes: xnet_pool_take_size_bytes and xnet_builder_slice_payload_size_bytes. And they both track slices in / going into XNet payloads. These are still limited by the same 4 MB payload size, so no change there.
As for xnet_pool_size_bytes, it is used to estimate "artifact pool latency" (mean time spent in the XNet pool and/or the ingress artifact pool) on the Subnet Load and IC Health dashboards; and displayed in the Memory Usage, by Component of the IC Health dashboard. But neither of them have fixed ranges. The Subnet Load dashboard has a threshold at 2 seconds of latency displayed as a filled region, but that gets displayed if the X axis exceeds 2 seconds and not displayed at all otherwise, it's not a hard limit.
There was a problem hiding this comment.
I suppose that the 2 second threshold does make an assumption about xnet_pool_size_bytes (i.e. it should be smaller than ~20 MB under ideal conditions), but bumping the pool size does not change the fact that 2 seconds end-to-end latency is an (arbitrary) threshold beyond which user experience is meaningfully affected.
There was a problem hiding this comment.
How about these (they also say 10 B - 5 MB)?
xnet_builder_response_body_size_bytesxnet_endpoint_slice_payload_size_bytesxnet_endpoint_response_size_bytes
| /// we're polling multiple subnets in parallel and we don't want to discard | ||
| /// slices that we've already pulled. | ||
| pub const POOL_BYTE_SIZE_SOFT_CAP: usize = 10 << 20; | ||
| pub const POOL_BYTE_SIZE_SOFT_CAP: usize = 128 << 20; |
There was a problem hiding this comment.
Can this limit actually be reached still (as the name and comment would suggest)?
There was a problem hiding this comment.
If you have enough streams with enough large messages (which we never do, AFAIK), yes.
In the current "poll everyone" implementation, if there are N bytes available in the pool, we poll every other subnet with a limit of N/16. If more than 16 subnets respond with enough bytes, we may well exceed the limit.
In the future, with advert triggered polling, while we won't be polling everyone at once with the same byte limit, there's still a race condition between when we compute the limit (before querying) and when we pool the response. And any (or well, a limited) number of other pulls could happen concurrently, with the same or a similar limit.
There was a problem hiding this comment.
Ah, I think I was confused by POOL_BYTE_SIZE_SOFT_CAP being both an asymptotic limit (which should never be reached), but also a soft cap in the sense that it may be exceeded transiently
2 participants
Increase the XNet pool soft byte limit from 10 MB to 128 MB. Apply a dynamic pooled slice size limit of 1/16 of the space left (8 MB or two XNet payloads' worth initially, less as it fills up), so per-slice limits taper.
This is necessary in order to provide real fairness across XNet peers: with the earlier limits (10 MB pool size, fixed 4 MB max per slice) a couple of slices could fill the pool, particularly once we switched to adverts (the existing sweep at least gives everyone equal chances).