contour
contour copied to clipboard
Published
20 hours ago •
projectcontour
projectcontour
Rethink the way notifications are sent to gRPC stream watchers
Currently notifying cache watchers is a manual process, we instrument the code path to defer a notification unconditionally. This is conservative and makes reasoning about notifications straight forward (we don't have e2e test coverage about version counters on caches, which reflects how many times notify has been called).
However there are downsides
- Each call to update will trigger a stream to envoy, parse and reload on its end, this is not cheap for large cache sizes
- This scales poorly in load testing and high update rate scenarios where lots of data is being changed in bulk.
This is the sequences of changes I propose
- We instrument the caches so they know if there has been a change to their contents; initially this is just a sequence counter that is ticked when there is an Add or Remove.
- We replace calls to notify in the per object code paths with a more general call; maybeNotify?, that is called unconditionally on the way out of the ResourceEventHandler code path. maybeNotify, as its name suggests will not notify if the internal sequence counter has not updated since the last maybeNotify.
- When caches can choose to not notify, we can wrap the 4 caches in a single meta maybeNotify call and move that higher up the stack to the buffer layer so the sequence is; receive notification off channel, process then call maybe notify which will notify watchers for all affected caches.
- We can then implement delayed notifications by running timer that starts when an update arrives at the buffer and is reset when another notification arrives. In that way we can have a small update window to batch updates if there is data in the buffer. This could probably be simulated with a call to len(buffer) after the update has been processed to see if there is another update immediately following the one we just updated.
