tonic-xds: deadlock when more than 65 services on the same port.

[aryehlev]

Hey we have in our environment alot of versions of services all on the same port, this causes a deadlock in our xDS client at startup.
The shared route config for the port references more clusters than the client's internal queue can hold (64), and the two tasks that need each other to drain it end up waiting on each other.

@YutaoMa thanks for pushing my last commit we still have another blocker and would appreciate if you can take a look, hope i am not asking too much.
if you have another implementation also will be good.

https://github.com/grpc/grpc-rust/pull/2777/changes

[YutaoMa]

The current fix looks good after the optimization refactoring, I've merged it.

However one more similar deadlock pattern exist: handle_resource_timeout has a .await on the watcher's event channel (size 16), while the pending dispatch events are polled in the same select! as the handle_resource_timeout itself. This can create a deadlock if a full watch channel and a timer coexist (e.g. a full event channel for a wildcard resource + a named resource that never arrived and is awaiting timer firing). A similar fix to stage but not block on send in handle_resource_timeout would address the problem. Let me know if you want to help tackle that PR too, thanks!

[aryehlev]

@YutaoMa thank you!
I will be away for two weeks.
This does solve the issue in some of our clusters.
But the issue when doing a rollout on the upstream and getting no traffic till client restart persists.
The two fixes didn't help.

I have a few theory's I will try to pinpoint the exact issue.

[YutaoMa]

Got it, feel free to share logs (you'll need to add some custom ones for sure as the current log could be lacking) and observations so I can help reproduce and debug from my side as well. As for the remaining deadlock issue mentioned above then I'll create the PR myself and get review from other maintainers, no worry on that.

[aryehlev]

Ok thank you @YutaoMa appreciate it🙏

[YutaoMa]

hi @ejona86 could you check if the deadlock mentioned in either the merged PR or my follow up comment above is the one you run into, if so we can discuss the solution here or otherwise need to add additional issues.

While working on a fix for the follow up comment I realized fundamentally these deadlocks are introduced because we went with an async watch API with bounded channels. Other grpc implementations all uses nonblocking sync watch APIs with callbacks and unbounded channels. While callback is not a good idea in Rust, I'm re-evaluating the tradeoff of using sync watch API and unbounded channel. I realized with the memory is bounded by events on a resource anyway, which isn't any worse than the current Vec<Delivery> fix.