In LinearCache, respond can't tell the caller is from update or delete.

envoyproxy / go-control-plane

Go implementation of data-plane-api

Apache License 2.0

1.52k stars 514 forks source link

In LinearCache, respond can't tell the caller is from update or delete. #540

Open fscnick opened 2 years ago

fscnick commented 2 years ago

UpdateResource and DeleteResource will call notifyAll once it has changes. And It respond the resource that has been changed. https://github.com/envoyproxy/go-control-plane/blob/4455b6323de455adf8d0c8afeb3fa5a90652555c/pkg/cache/v3/linear.go#L140-L151

However, the xds-client get the response. It would remove the resources that aren't in the response from local cache. https://github.com/grpc/grpc-go/blob/011544f72939c85397b0e24378280e6075061cb1/xds/internal/xdsclient/pubsub/update.go#L227-L240

Thus, a DeleteResource has been called, the xds-server would respond the deleted resource. But the resource has been deleted, actually it's empty. The xds-client remove the other resource not in the response from its local cache. it causes the grpc-client is unable to connect to other endpoints that haven't been deleted. https://github.com/envoyproxy/go-control-plane/blob/4455b6323de455adf8d0c8afeb3fa5a90652555c/pkg/cache/v3/linear.go#L126-L131

github-actions[bot] commented 2 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 "no stalebot" or other activity occurs. Thank you for your contributions.

fscnick commented 2 years ago

no stalebot

valerian-roche commented 10 months ago

The current behavior of Linear Cache is:

wildcard requests always get all existing resources returned, even if using RDS or EDS when only the updated resources could be returned
non-wildcard requests only get updated resources, even for CDS and LDS when all matching resources should be returned Simple cache also does not match the expected behavior, as it currently always returns all resources.

I plan on addressing it, but I first need to do some lower level changes (example passing the state of subscription to the watch) as other issues are currently present in the system even before creating the response (e.g. when adding a new resource in a watch without a version change in sotw in linear cache)

atollena commented 9 months ago

Current code in grpc-go is here: https://github.com/grpc/grpc-go/blob/28ac6efee63b6e2433f94fa263e6ed0610afb69b/xds/internal/xdsclient/authority.go#L245-L302

valerian-roche commented 7 months ago

This issue has been fixed in this PR in our fork. We are currently validating the behavior with our internal usage of gRPC and envoy as xDS clients, and will upstream it when possible