-
-
Notifications
You must be signed in to change notification settings - Fork 320
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow subscribing to reflector store updates #1426
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Natalie Klestrup Röijezon <[email protected]>
Signed-off-by: Natalie Klestrup Röijezon <[email protected]>
Signed-off-by: Natalie Klestrup Röijezon <[email protected]>
Signed-off-by: Natalie Klestrup Röijezon <[email protected]>
Signed-off-by: Natalie Klestrup Röijezon <[email protected]>
hey, just linking this here since I've been working on the same thing main...mateiidavid:kube:matei/arc-watcher I too switched to async-broadcast after our discussion since it's better suited for what we need to do. |
Ah, hadn't seen that one. However, that also looks like it prefers to drop messages rather than apply backpressure (https://docs.rs/async-broadcast/latest/async_broadcast/enum.RecvError.html#variant.Overflowed). |
I think overflow is disabled by default. It can also be disabled manually (see https://docs.rs/async-broadcast/latest/async_broadcast/struct.Receiver.html#method.overflow). The I actually wanted to send that over to you for a quick sanity check but I guess now's as good a time as any 😂 |
Ahh: https://docs.rs/async-broadcast/latest/src/async_broadcast/lib.rs.html#156, gotcha, looks reasonable.
I assume you mean |
Motivation
This is largely a "shower thoughts" take on #1189 and the discussions from the last office hours.
It still fails
controller::tests::applier_must_not_deadlock_if_reschedule_buffer_fills
and introduces some type safety regressions that I'm not quite sure how to fix.Solution
It introduces a new (internal)
Broadcaster
channel, which has the backpressure and broadcasting properties that we need (async-channel
is inappropriate after all since it only sends each message to one receiver).It then adds an interface to
reflector::store::Writer
for subscribing to changes.This all leads to a fair amount of changes to
reflector
and its downstreams to accomodate forWriter::apply_watcher_event
now being async (so that it can send out the appropriate events).