[Snapshot Cache] Simplify Snapshot cache responses. #1356
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mohit-pal-kohli
approved these changes
Dec 22, 2025
tnawathe21
approved these changes
Jan 8, 2026
pkg/cache/v3/simple.go
Outdated
| cache.log.Debugf("respond %s (requested %v) version %q with version %q and resources %v", request.GetTypeUrl(), request.GetResourceNames(), request.GetVersionInfo(), version, slices.Collect(maps.Keys(resp.GetReturnedResources()))) | ||
| // This implementation can seem more complex than needed, as it does not blindly rely on the request version. | ||
| // This allows for a more generic implemenentation when considering wildcard + subscribed, or partial replies. | ||
| // In other context a lot could be simplified as |
Contributor
Author
There was a problem hiding this comment.
Good catch, updated
pkg/cache/v3/snapshot.go
Outdated
| typeURL, referenceSet, len(items.Items)) | ||
| } | ||
|
|
||
| // Check superset. |
There was a problem hiding this comment.
Suggested change
| // Check superset. | |
| // Check difference. |
valerian-roche
force-pushed
the
vr/snapshot-reply
branch
from
db37516 to
eaddffa
Compare
The snapshot cache reply logic for sotw and delta watches has grown complex and very distinct from the linear cache. This has created issues recently, mostly related to the logic in `CreateWatch` having to perfectly match the one in `respond` to avoid major issues. This commit simplifies the code in the same model as the linear cache: a single method creates the response, and the watch is created if no response is returned. There is some behavior change: if the type is not Listeners or Clusters, the control-plane will now only return the modified resources, per the xDS protocol. Please create an issue if this creates problems. Further work will build on this to: - support a new snapshot model, allowing per type updates and versions, as well as better delta handling - use per-resource version even in sotw to avoid resending resources when not needed. - provide support for partial wildcard responses, to provide a new support model for wildcard + resources subscriptions (e.g. for OdCDS). Signed-off-by: Valerian Roche <valerian.roche@gmail.com>
valerian-roche
force-pushed
the
vr/snapshot-reply
branch
from
eaddffa to
aaeb8a9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The snapshot cache reply logic for sotw and delta watches has grown complex and very distinct from the linear cache. This has created issues recently, mostly related to the logic in
CreateWatchhaving to perfectly match the one inrespondto avoid major issues. This commit simplifies the code in the same model as the linear cache: a single method creates the response, and the watch is created if no response is returned.There is some behavior change: if the type is not Listeners or Clusters, the control-plane will now only return the modified resources, per the xDS protocol. Please create an issue if this creates problems.
Further work will build on this to: