easwars review 4

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)

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.

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.

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())
 }

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")
 }
 

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
 }