bar: add ability to update resolver state atomically and pass directly to the balancer #2693
Conversation
resolver_conn_wrapper.go
Outdated
| return | ||
| } | ||
| grpclog.Infof("ccResolverWrapper: sending update to cc: %v", s) | ||
| // TODO: add Channelz Trace Event |
There was a problem hiding this comment.
TODO is irrelevant now, channelz event is recored inside updateResolverState.
There was a problem hiding this comment.
The event in there is for the service config only.
I was thinking there probably should just be one event for the whole new resolver.State, regardless of the previous state - what do you think?
There was a problem hiding this comment.
I think it will be confusing for the people who read the trace events. They need to manually digest the trace to understand when the change in addresses or service config happens.I would prefer only logging the event when there's a change.
There was a problem hiding this comment.
If it's helpful for debugging, the event could also include the differences from the previous state - addresses added & removed, and old/new service config (if different). That could go into one event or several - what do you think?
There was a problem hiding this comment.
Yes, I think combine them into one is ok as long as it's clear which change(s) at this update.
No description provided.