Listener: respect the connection balancer of the redirected listener #15842
Listener: respect the connection balancer of the redirected listener #15842htuch merged 23 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
To see if I got the issue right: listener1 connection count is low, it gets a connection. Then, in If the above is accurate, then your changes lgtm, but I'll need to take a better look at tests. |
Thanks @dmitri-d! The linked #15146 has more details regarding listener1. In some cases including istio, the listener1(virtual outbound in istio) doesn't own any connection after the redirect. Now that the accounting is bad, it's likely all the workers of listener1 will always rebalance the connection to worker0's listener1... |
|
the test lgtm too. |
|
gentle ping @ggreenway |
ggreenway
left a comment
ggreenway
left a comment
There was a problem hiding this comment.
I think this will be fine. I've never looked at the rebalancer code before, but I think I've familiarized myself with it. @mattklein123 do you want to take a quick look for sanity at the main code change and make sure this makes sense to you?
I think this would benefit from an integration test, verifying that multiple levels of balancing end up with the correct result.
Also, is there a good place to document that the exact-rebalancer should not be used in these use cases? I think it's just extra overhead without any benefit.
|
Yeah this seems fine to me at a high level. 👍 |
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
/wait |
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
|
@htuch can you sign off on the api change? |
…nvoyproxy#15842) If listener1 redirects the connection to listener2, the balancer field in listener2 decides whether to rebalance. Previously we rely on the rebalancing at listener1, however, the rebalance is weak because listener1 is likely to not own any connection and the rebalance is no-op. Risk Level: MID. Rebalance may introduce latency. User needs to clear rebalancer field of listener2 to recover the original behavior. Fix envoyproxy#15146 envoyproxy#16113 Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Gokul Nair <gnair@twitter.com>
…nvoyproxy#15842) If listener1 redirects the connection to listener2, the balancer field in listener2 decides whether to rebalance. Previously we rely on the rebalancing at listener1, however, the rebalance is weak because listener1 is likely to not own any connection and the rebalance is no-op. Risk Level: MID. Rebalance may introduce latency. User needs to clear rebalancer field of listener2 to recover the original behavior. Fix envoyproxy#15146 envoyproxy#16113 Signed-off-by: Yuchen Dai <silentdai@gmail.com> Signed-off-by: Gokul Nair <gnair@twitter.com>
6 participants
Commit Message:
If listener1 redirects the connection to listener2, the balancer field in listener2 decides whether to rebalance.
Previously we rely on the rebalancing at listener1, however, the rebalance is weak because listener1 is likely to
not own any connection and the rebalance is no-op.
Additional Description:
on top of #15815merged main branch
Risk Level: MID. Rebalance may introduce latency. User needs to clear rebalancer field of listener2 to recover the original behavior.
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
Fix #15146 #16113
[Optional Deprecated:]
[Optional API Considerations:]