Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fscnick opened this issue Feb 22, 2022 · 5 comments · May be fixed by #543
Open

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

fscnick opened this issue Feb 22, 2022 · 5 comments · May be fixed by #543

Comments

@fscnick
Copy link

fscnick commented Feb 22, 2022

UpdateResource and DeleteResource will call notifyAll once it has changes. And It respond the resource that has been changed.

func (cache *LinearCache) notifyAll(modified map[string]struct{}) {
// de-duplicate watches that need to be responded
notifyList := make(map[chan Response][]string)
for name := range modified {
for watch := range cache.watches[name] {
notifyList[watch] = append(notifyList[watch], name)
}
delete(cache.watches, name)
}
for value, stale := range notifyList {
cache.respond(value, stale)
}

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.

for _, name := range staleResources {
resource := cache.resources[name]
if resource != nil {
resources = append(resources, types.ResourceWithTTL{Resource: resource})
}
}

@github-actions
Copy link

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.

@github-actions github-actions bot added the stale label Mar 24, 2022
@fscnick
Copy link
Author

fscnick commented Mar 24, 2022

no stalebot

@valerian-roche
Copy link
Contributor

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)

@valerian-roche
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants