From 07c55977937a1e1d4878c711969ad43eeaddb2ae Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Thu, 20 Feb 2025 18:19:53 +0530 Subject: [PATCH] easwars review 4 --- xds/internal/clients/lrsclient/load_store.go | 67 ++----------------- xds/internal/clients/lrsclient/lrsconfig.go | 6 +- .../clients/xdsclient/resource_type.go | 2 +- .../clients/xdsclient/resource_watcher.go | 15 +++-- xds/internal/clients/xdsclient/xdsclient.go | 2 +- xds/internal/clients/xdsclient/xdsconfig.go | 15 ++--- 6 files changed, 24 insertions(+), 83 deletions(-) diff --git a/xds/internal/clients/lrsclient/load_store.go b/xds/internal/clients/lrsclient/load_store.go index 4fc6cec62735..e4e6c0ccda0f 100644 --- a/xds/internal/clients/lrsclient/load_store.go +++ b/xds/internal/clients/lrsclient/load_store.go @@ -20,8 +20,6 @@ package lrsclient -import "time" - // LoadStore keep track of the loads for multiple clusters and services that // are intended to be reported via LRS. // @@ -32,73 +30,16 @@ import "time" type LoadStore struct { } -// Stats returns the load data for the given cluster names. Data is returned in -// a slice with no specific order. -// -// If no clusterName is given (an empty slice), all data for all known clusters -// is returned. -// -// If a cluster's Data is empty (no load to report), it's not appended to the -// returned slice. -// -// Calling Stats clears the previous load data from the LoadStore. -func (s *LoadStore) Stats(clusterNames []string) []*Data { - panic("unimplemented") -} - // PerCluster returns the PerClusterReporter for the given cluster and service. -func (s *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter { +func (ls *LoadStore) PerCluster(clusterName, serviceName string) PerClusterReporter { panic("unimplemented") } -// Data contains all load data reported to the LoadStore since the most recent -// call to Stats(). -type Data struct { - // Cluster is the name of the cluster this data is for. - Cluster string - // Service is the name of the EDS service this data is for. - Service string - // TotalDrops is the total number of dropped requests. - TotalDrops uint64 - // Drops is the number of dropped requests per category. - Drops map[string]uint64 - // LocalityStats contains load reports per locality. - LocalityStats map[string]LocalityData - // ReportInterval is the duration over which load was reported. - ReportInterval time.Duration -} - -// LocalityData contains load data for a single locality. -type LocalityData struct { - // RequestStats contains counts of requests made to the locality. - RequestStats RequestData - // LoadStats contains server load data for requests made to the locality, - // indexed by the load type. - LoadStats map[string]ServerLoadData -} - -// RequestData contains request counts. -type RequestData struct { - // Succeeded is the number of succeeded requests. - Succeeded uint64 - // Errored is the number of requests which ran into errors. - Errored uint64 - // InProgress is the number of requests in flight. - InProgress uint64 - // Issued is the total number requests that were sent. - Issued uint64 -} - -// ServerLoadData contains server load data. -type ServerLoadData struct { - // Count is the number of load reports. - Count uint64 - // Sum is the total value of all load reports. - Sum float64 -} - // PerClusterReporter defines the methods that the LoadStore uses to track // per-cluster load reporting data. +// +// The lrsclient package provides an implementation of this which can be used +// to push loads to the received LoadStore from the LRS client. type PerClusterReporter interface { // CallStarted records a call started in the LoadStore. CallStarted(locality string) diff --git a/xds/internal/clients/lrsclient/lrsconfig.go b/xds/internal/clients/lrsclient/lrsconfig.go index f0a8c0869019..c05516dffef7 100644 --- a/xds/internal/clients/lrsclient/lrsconfig.go +++ b/xds/internal/clients/lrsclient/lrsconfig.go @@ -22,9 +22,9 @@ import ( "google.golang.org/grpc/xds/internal/clients" ) -// Config is used to configure an LRS client. After one has been passed to an -// LRS function, it must not be modified. A Config may be reused; the LRS -// package will also not modify it. +// Config is used to configure an LRS client. After one has been passed to the +// LRS client's New function, it must not be modified. A Config may be reused; +// the lrsclient package will also not modify it. type Config struct { // Node is the identity of the client application reporting load to the // LRS server. diff --git a/xds/internal/clients/xdsclient/resource_type.go b/xds/internal/clients/xdsclient/resource_type.go index 30e7fb49784f..a9ce42f6b937 100644 --- a/xds/internal/clients/xdsclient/resource_type.go +++ b/xds/internal/clients/xdsclient/resource_type.go @@ -59,7 +59,7 @@ type Decoder interface { Decode(resource any, options DecodeOptions) (*DecodeResult, error) } -// DecodeOptions wraps the options required by ResourceType implementation for +// DecodeOptions wraps the options required by ResourceType implementations for // decoding configuration received from the xDS management server. type DecodeOptions struct { // Config contains the complete configuration passed to the xDS client. diff --git a/xds/internal/clients/xdsclient/resource_watcher.go b/xds/internal/clients/xdsclient/resource_watcher.go index 9fd78a7e2afc..89441a156e4c 100644 --- a/xds/internal/clients/xdsclient/resource_watcher.go +++ b/xds/internal/clients/xdsclient/resource_watcher.go @@ -22,23 +22,24 @@ package xdsclient // corresponding to the resource being watched. gRFC A88 contains an exhaustive // list of what method is invoked under what conditions. // -// onCallbackProcessed in the callbacks is a function to be invoked by -// resource watcher implementations upon completing the processing of a -// callback from the xDS client. Failure to invoke this callback prevents the -// xDS client from reading further messages from the xDS server. +// changeProcessed and errorProcessed in the callbacks are functions to be +// invoked by resource watcher implementations upon completing the processing +// of a callback from the xDS client for change and error respectively. Failure +// to invoke this callback prevents the xDS client from reading further +// messages from the xDS server. type ResourceWatcher interface { // ResourceChanged indicates a new version of the resource is available. - ResourceChanged(resourceData ResourceData, onCallbackProcessed func()) + ResourceChanged(resourceData ResourceData, changeProcessed func()) // ResourceError indicates an error occurred while trying to fetch or // decode the associated resource. The previous version of the resource // should be considered invalid. - ResourceError(err error, onCallbackProcessed func()) + ResourceError(err error, errorProcessed func()) // AmbientError indicates an error occurred after a resource has been // received that should not modify the use of that resource but may be // provide useful information about the ambient state of the XDSClient for // debugging purposes. The previous version of the resource should still be // considered valid. - AmbientError(err error, onCallbackProcessed func()) + AmbientError(err error, errorProcessed func()) } diff --git a/xds/internal/clients/xdsclient/xdsclient.go b/xds/internal/clients/xdsclient/xdsclient.go index 601eddc45f01..267475158e73 100644 --- a/xds/internal/clients/xdsclient/xdsclient.go +++ b/xds/internal/clients/xdsclient/xdsclient.go @@ -53,7 +53,7 @@ func New(config Config) (*XDSClient, error) { // // The returned function cancels the watch and prevents future calls to the // watcher. -func (c *XDSClient) WatchResource(typeURL string, name string, watcher ResourceWatcher) (cancel func()) { +func (c *XDSClient) WatchResource(typeURL, name string, watcher ResourceWatcher) (cancel func()) { panic("unimplemented") } diff --git a/xds/internal/clients/xdsclient/xdsconfig.go b/xds/internal/clients/xdsclient/xdsconfig.go index 2fd6c64741d5..c13ddab2a57c 100644 --- a/xds/internal/clients/xdsclient/xdsconfig.go +++ b/xds/internal/clients/xdsclient/xdsconfig.go @@ -22,9 +22,9 @@ import ( "google.golang.org/grpc/xds/internal/clients" ) -// Config is used to configure an xDS client. After one has been passed to an -// xDS function, it must not be modified. A Config may be reused; the xDS -// package will also not modify it. +// Config is used to configure an xDS client. After one has been passed to the +// xDS client's New function, it must not be modified. A Config may be reused; +// the xdsclient package will also not modify it. type Config struct { // Servers specifies a list of xDS management servers to connect to. The // order of the servers in this list reflects the order of preference of @@ -73,13 +73,12 @@ type Authority struct { // Extensions can be populated with arbitrary authority-specific data to be // passed from the xDS client configuration down to the user defined - // resource decoder implementations. This allows the user to provide + // resource type implementations. This allows the user to provide // authority-specific context or configuration to their resource // processing logic. - // - // The xDS client do not interpret the contents of this field. It is the - // responsibility of the user's implementations to handle and interpret - // these extensions. + // The xDS client does not interpret the contents of this field. It is the + // responsibility of the user's resource type implementations to handle and + // interpret these extensions. Extensions any }