xds/priority: bug fix and minor behavior change #5417
Conversation
- Bug: push config updates to all child policies, not just the active one. - Process all child picker updates before determining active priority.
| // an update of the aggregated picker to the parent. Cleared after all | ||
| // sub-balancers have finished UpdateClientConnState, after which | ||
| // syncPriority is called manually. | ||
| inhibitChildUpdates bool |
There was a problem hiding this comment.
Nit: I don't like this name. We are inhibiting the child picker updates, but we are inhibiting the calculation/synchronization of the priority, which determines what child picker update to send back. Maybe inhibitSyncPriority?
There was a problem hiding this comment.
Yeah sure, even though syncPriority calls aren't exactly 1:1 with a picker update, you're still inhibiting all of the possible picker updates, even though that's only a fraction of the time syncPriority is called.
There was a problem hiding this comment.
syncPriority is the only thing that pushes a picker update to the parent policy, which is how I was thinking of this (and how the functionality was debated/designed cross-language).
| } | ||
|
|
||
| // Two new subconns are created by the previous update; one by p0 and one | ||
| // by p1. They don't happen concurrently, but they could happen in any |
There was a problem hiding this comment.
Nondeterministic map iterations :0.
| // an update of the aggregated picker to the parent. Cleared after all | ||
| // sub-balancers have finished UpdateClientConnState, after which | ||
| // syncPriority is called manually. | ||
| inhibitChildUpdates bool |
There was a problem hiding this comment.
Yeah sure, even though syncPriority calls aren't exactly 1:1 with a picker update, you're still inhibiting all of the possible picker updates, even though that's only a fraction of the time syncPriority is called.
dfawley
changed the title
#5211
Logic changes are all in
.../balancer/priority/*.go; the other changes are tests that are too dependent on the priority balancer's behavior (not correctness related; just not allowing for the possibility of duplicate picker updates).RELEASE NOTES: