From e199a5a903e9606977a4e08025e138bcd690ae46 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Thu, 22 Oct 2020 10:21:15 -0700 Subject: [PATCH] [lrs_stream_report] c3 --- xds/internal/balancer/edsbalancer/eds.go | 2 +- .../edsbalancer/xds_client_wrapper.go | 29 ++++--------------- xds/internal/balancer/lrs/balancer.go | 24 +++------------ 3 files changed, 11 insertions(+), 44 deletions(-) diff --git a/xds/internal/balancer/edsbalancer/eds.go b/xds/internal/balancer/edsbalancer/eds.go index 7136baa0200..a148db0e42e 100644 --- a/xds/internal/balancer/edsbalancer/eds.go +++ b/xds/internal/balancer/edsbalancer/eds.go @@ -178,7 +178,7 @@ func (x *edsBalancer) handleGRPCUpdate(update interface{}) { } if err := x.client.handleUpdate(cfg, u.ResolverState.Attributes); err != nil { - x.logger.Infof("failed to update xds clients: %v", err) + x.logger.Warningf("failed to update xds clients: %v", err) } if x.config == nil { diff --git a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go index 8d3830f6bc0..fe4e996a7b5 100644 --- a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go +++ b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go @@ -51,7 +51,9 @@ type loadStoreWrapper struct { mu sync.RWMutex service string // Both store and perCluster will be nil if load reporting is disabled (EDS - // response doesn't have LRS server name). + // response doesn't have LRS server name). Note that methods on Store and + // perCluster all handle nil, so there's no need to check nil before calling + // them. store *load.Store perCluster load.PerClusterReporter } @@ -63,10 +65,6 @@ func (lsw *loadStoreWrapper) updateServiceName(service string) { return } lsw.service = service - - if lsw.store == nil { - return - } lsw.perCluster = lsw.store.PerCluster(lsw.service, "") } @@ -78,45 +76,30 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) { } lsw.store = store lsw.perCluster = nil - if lsw.store != nil { - lsw.perCluster = lsw.store.PerCluster(lsw.service, "") - } - + lsw.perCluster = lsw.store.PerCluster(lsw.service, "") } func (lsw *loadStoreWrapper) CallStarted(locality string) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallStarted(locality) } func (lsw *loadStoreWrapper) CallFinished(locality string, err error) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallFinished(locality, err) } func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallServerLoad(locality, name, val) } func (lsw *loadStoreWrapper) CallDropped(category string) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallDropped(category) } @@ -225,8 +208,8 @@ func (c *xdsClientWrapper) updateXDSClient(config *EDSConfig, attr *attributes.A dopts = []grpc.DialOption{grpc.WithContextDialer(dialer)} } - // TODO: there's no long a need to read bootstrap file and create a new xds - // client. The EDS balancer should always get the xds client from + // TODO: there's no longer a need to read bootstrap file and create a new + // xds client. The EDS balancer should always get the xds client from // attributes. Otherwise, this function should just fail. Also, xdsclient // will be shared by multiple clients, so trying to make an xds client is // just the wrong move. diff --git a/xds/internal/balancer/lrs/balancer.go b/xds/internal/balancer/lrs/balancer.go index 916a48fc3ff..f8e7673f7d8 100644 --- a/xds/internal/balancer/lrs/balancer.go +++ b/xds/internal/balancer/lrs/balancer.go @@ -155,7 +155,9 @@ type loadStoreWrapper struct { cluster string edsService string // Both store and perCluster will be nil if load reporting is disabled (EDS - // response doesn't have LRS server name). + // response doesn't have LRS server name). Note that methods on Store and + // perCluster all handle nil, so there's no need to check nil before calling + // them. store *load.Store perCluster load.PerClusterReporter } @@ -168,10 +170,6 @@ func (lsw *loadStoreWrapper) updateClusterAndService(cluster, edsService string) } lsw.cluster = cluster lsw.edsService = edsService - - if lsw.store == nil { - return - } lsw.perCluster = lsw.store.PerCluster(lsw.cluster, lsw.edsService) } @@ -183,44 +181,30 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) { } lsw.store = store lsw.perCluster = nil - if lsw.store != nil { - lsw.perCluster = lsw.store.PerCluster(lsw.cluster, lsw.edsService) - } + lsw.perCluster = lsw.store.PerCluster(lsw.cluster, lsw.edsService) } func (lsw *loadStoreWrapper) CallStarted(locality string) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallStarted(locality) } func (lsw *loadStoreWrapper) CallFinished(locality string, err error) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallFinished(locality, err) } func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallServerLoad(locality, name, val) } func (lsw *loadStoreWrapper) CallDropped(category string) { lsw.mu.RLock() defer lsw.mu.RUnlock() - if lsw.perCluster == nil { - return - } lsw.perCluster.CallDropped(category) }