xds/internal:remove Generic resource Decoder and added concrete functions #8381 #8582
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8582 +/- ##
==========================================
+ Coverage 81.98% 82.00% +0.02%
==========================================
Files 413 415 +2
Lines 40528 40738 +210
==========================================
+ Hits 33225 33407 +182
+ Misses 5944 5934 -10
- Partials 1359 1397 +38
🚀 New features to boost your workflow:
|
vinothkumarr227
changed the title
eshitachandwani
added
Type: Internal Cleanup
|
@vinothkumarr227 , can you please break this PR down into smaller PRs with one resource in one PR so that it is easier to understand and review. |
|
|
||
| // LoadStore is the minimal interface returned by ReportLoad. It provides a | ||
| // way to get per-cluster reporters and to stop the store. | ||
| type LoadStore interface { |
There was a problem hiding this comment.
Why did we add this interface?
There was a problem hiding this comment.
As part of task #8381, I added the LoadStore interface to replace the previous wrapper and embedded logic used for load reporting. Earlier, the balancers and xDS components used a concrete wrapper (load_store_wrapper.go) to talk to the LRS client.
| // balancer when LRS is enabled. Before that, all functions to record loads | ||
| // are no-op. | ||
| store *lrsclient.LoadStore | ||
| perCluster *lrsclient.PerClusterReporter |
There was a problem hiding this comment.
Why did we change it to use the grpc xdsclient interfaces that we created?
There was a problem hiding this comment.
Previously, our balancer directly referenced lrsclient types like LoadStore and PerClusterReporter.
But in the new design, all xDS functionality goes through our gRPC xdsclient interfaces.
So I changed it to use those interfaces to decouple balancer code from the internal LRS implementation — it’s part of the #8381 cleanup to remove wrappers and standardize on the generic client.
| // are are deserialized and validated, as received from the xDS management | ||
| // server. Upon receipt of a response from the management server, an | ||
| // appropriate callback on the watcher is invoked. | ||
| func (c *clientImpl) WatchResource(rType xdsresource.Type, resourceName string, watcher xdsresource.ResourceWatcher) (cancel func()) { |
There was a problem hiding this comment.
If we are changing the type parameter, can we change it to a string so that the signatures match and we can directly call the xdsclient.WatchResource?
There was a problem hiding this comment.
We considered changing the parameter to string, but we’re keeping xdsresource.Type intentionally. In the generic design, a resource “type” isn’t just a typeURL string — it carries behavior (decode)
| // interface with ref counting so that it can be shared by the xds resolver and | ||
| // balancer implementations, across multiple ClientConns and Servers. | ||
| type clientImpl struct { | ||
| *xdsclient.XDSClient // TODO: #8313 - get rid of embedding, if possible. |
There was a problem hiding this comment.
Why did we make this change?
There was a problem hiding this comment.
We embedded xdsclient.XDSClient here as a temporary delegation layer to the generic client while we migrate call sites for #8381.
Fixes: #8381
RELEASE NOTES: