envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.76k stars 4.76k forks source link

Repeated on-demand vhds updates unsubscribe from previously resolved vhosts #12158

Open dmitri-d opened 4 years ago

dmitri-d commented 4 years ago

Description: If a vhost named "foo.com" has been previously resolved, an attempt to resolve "bar.com" will result in "foo.com" vhost being unsubscribed from, and all updates for it lost.

This is due to how GrpcMuxWatch::update is implemented -- it expects a list of resource names that should be watched. The resources which names are not in the list will be unsubscribed from.

edit: not an issue for NewGrpcMuxImpl::addWatch

htuch commented 4 years ago

@dmitri-d shouldn't we have one watch per vhost?

dmitri-d commented 4 years ago

@htuch: it's per subscription right now (handled by GrpcSubscriptionImpl, so the same as other xDS protocols then). I need to think about the implications of what you are suggesting. Are you thinking just VHDS, or for the rest of xDS too?

htuch commented 4 years ago

For RDS/EDS, there is already one subscription/watch (singleton) per consumer, e.g. each EdsClusterImpl owns a subscription for the EDS service or cluster name. These are then multiplexed in ADS to a single EDS request/response. So, in those cases, each subscribed resource has a subscription and watch.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.