Fix reliable-channel message loss with multiple reliable subscribers - #124
Fix reliable-channel message loss with multiple reliable subscribers#124rudicus wants to merge 1 commit into
Conversation
A reliable subscriber only holds a slot reference while it is reading a message, and kMessageSeenByReliable is set by the FIRST reliable subscriber to read a slot. With two or more reliable subscribers, a faster one marks a slot seen and the slower one's unread backlog carries no references while it sits between messages - so FindFreeSlotReliable reclaims those slots and the slower reliable subscriber silently loses messages. Measured in production: 0.05-0.14% loss on a ~4.2k msg/s reliable channel whenever it had a second subscriber; exactly lossless (1,032,252/1,032,252) with a single subscriber. Fix: ChannelControlBlock gains a reliable_subscribers bitset (maintained next to `subscribers`: registration records reliability before the subscriber becomes visible to publishers, server cleanup clears it so a dead subscriber cannot gate publishers forever, placement-initialized at channel creation since AtomicBitSet carries a runtime num_bits_). FindFreeSlotReliable stops at the first slot still pending in any reliable subscriber's available-slots bitset - the authoritative per-subscriber delivery state, set at publication and cleared on claim. Activation messages are exempt (subscribers skip them without claiming, so their delivery bits can remain set; reclaiming them loses no data), and kReadNewest skips remain deliberate skips. The check only runs on the ring-full slow path. kChannelControlBlockVersion bumps to 6 for the CCB layout change. Tests: ReliableChannelIsLosslessForEveryReliableSubscriber fills the ring past a tight-reading reliable subscriber while a second reliable subscriber reads nothing, asserts the publisher backpressures instead of handing out its backlog, then asserts every message arrives in order; ReliablePublisherUnblocksWhenLaggingSubscriberUnsubscribes asserts a departed subscriber releases the gate. Both fail on unpatched main. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hi Matt, good to hear from you. Unfortunately I am in the UK for a while due to a family emergency. I can look at this in a couple of days if that's OK. |
|
Matt, I think there is an already supported way to solve this. The subscriber has an option called You can call BTW, I got to this earlier than expected because I am jet lagged and can't sleep at the moment |
|
@dallison - Sorry to hear of your family troubles. Thanks for the tip on what already exists, I will give it a shot! Question though on what you mentioned just so I understand - (and no rush to respond) : If two reliable subscribers (A and B) are both reading the same channel, and A reads the message first before B, it will set kMessageSeenByReliable on that message. If for whatever reason B is blocked (in our case a long running algorithm), A could keep reading new messages (and releasing the old ones with that bit set) before B reads it? We are trying to do faster than realtime replay (64x) so certain things are super fast and could churn through the queue while we do have some algos that can't actually keep up that rate and would just like the system to "slow down" to match what we can actually handle |
|
Quick update: Your suggestion does seemingly resolve my issues! (Though not sure entirely how given the above scenario) Thanks. I can close this out if you like. |
|
Glad it works. If you have a really slow subscriber that is not keeping up with the messages and you are using reliable messages, then by definition the publisher will be unable to publish messages that will be missed by the slow subscriber. For reliable channels, the publisher will always be blocked if a subscriber will miss the message (like a nonblocking socket behavior in TCP). So with reliable messages, you might get into a situation where the publisher is backpressured by the slowest subscriber. That is probably what you want. |
|
I don't think we need this PR since it's already supported, but it might be worth adding some documentation since it's not obvious and the behavior of multiple publishers is subtle Maybe it would be worth automatically setting the |
|
Yeah, automatically setting it seems wise especially since the main purpose of having it off, afaik, is to economize slots, and nobody should really be trying to use reliable channels on a tight slot-budget anyway. |
|
I thought about it overnight and it actually makes sense to always enable the |
|
Oh, I also thought that the best way to answer your question about why it works would be to ask Claude or GPT. They are very good at that. |
|
Here's what Grok says: |
Hey Dave - Hope all is well. Been using subspace regularly and love it. I ran into an issue with backpressure + multiple reliable subscribers. I was getting drops when one of the reliable subscribers read it, and the other one wasnt able to. Pardon the Claude PR, but this fixes it for me and lets me get 100% reliable playback with back pressure +multiple subscribers.
The bug
A reliable channel loses messages for its slower reliable subscribers the moment it has more than one subscriber:
kMessageSeenByReliableis set by the first reliable subscriber to read a slot.FindFreeSlotReliablewalks past the seen flag, finds zero refs, and reclaims the slot — the slower reliable subscriber silently loses that message (no drop counter fires; the loss is invisible on both ends).We measured this in production (an autonomy stack replaying sensor data through reliable channels at 64×): a ~4.2k msg/s reliable channel lost 0.05–0.14% of messages whenever it had a second subscriber — whether that second subscriber was reliable or not — and was exactly lossless (1,032,252 / 1,032,252) with a single subscriber. #104's
kMessageSeenByReliablefixes the unreliable-co-subscriber case but not the reliable–reliable one.The fix
The per-subscriber available-slot bitsets already record exactly the needed state — "this slot has not yet been delivered to subscriber X" — they were just never consulted at reclaim time (and shared memory didn't record which subscriber ids are reliable):
ChannelControlBlockgains areliable_subscribersbitset, maintained besidesubscribers: registration records reliability before the subscriber becomes visible to publishers; server-side cleanup clears it so a dead subscriber can't gate publishers forever; placement-initialized at channel creation (likesubscribers, sinceAtomicBitSetcarries a runtimenum_bits_).FindFreeSlotReliablestops at the first slot still pending in any reliable subscriber's available-slots bitset. Activation messages are exempt — subscribers skip them without claiming, so their delivery bits can remain set forever, and reclaiming them loses no data.kReadNewestskips remain deliberate skips (they clear the pending bits). The check only runs on the ring-full slow path, so nothing is added to the publish fast path.kChannelControlBlockVersion→ 6 for the layout change.Tests
ReliableChannelIsLosslessForEveryReliableSubscriber— deterministic, single-threaded: fills the ring past a tight-reading reliable subscriber while a second reliable subscriber reads nothing; asserts the publisher backpressures (GetMessageBufferreturns null) instead of handing out the slow subscriber's backlog, that every message then arrives in order, and that the publisher resumes once both have caught up.ReliablePublisherUnblocksWhenLaggingSubscriberUnsubscribes— a lagging reliable subscriber gates the publisher; its departure releases the gate.Both fail on unpatched main and pass with the fix; the full
//client:client_test,//common:common_test, and//server:server_testsuites pass with the fix (includingReliablePublisherDoesNotBlockOnUnreliableSubscriberand the other reliable-path tests).We're carrying this as a patch against 3.0.3 in our tree in the meantime — happy to adjust whatever you'd like on naming, the version bump, or test placement.