Skip to content

Commit

Permalink
eds/lrs: handle nil when LRS is disabled (#4086)
Browse files Browse the repository at this point in the history
  • Loading branch information
menghanl committed Dec 7, 2020
1 parent 0d6a24f commit bce1fdf
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 8 deletions.
27 changes: 27 additions & 0 deletions xds/internal/balancer/edsbalancer/eds_impl_test.go
Expand Up @@ -743,3 +743,30 @@ func (s) TestEDS_LoadReport(t *testing.T) {
t.Errorf("store.stats() returned unexpected diff (-want +got):\n%s", diff)
}
}

// TestEDS_LoadReportDisabled covers the case that LRS is disabled. It makes
// sure the EDS implementation isn't broken (doesn't panic).
func (s) TestEDS_LoadReportDisabled(t *testing.T) {
lsWrapper := &loadStoreWrapper{}
lsWrapper.updateServiceName(testClusterNames[0])
// Not calling lsWrapper.updateLoadStore(loadStore) because LRS is disabled.

cc := testutils.NewTestClientConn(t)
edsb := newEDSBalancerImpl(cc, nil, lsWrapper, nil)
edsb.enqueueChildBalancerStateUpdate = edsb.updateState

// One localities, with one backend.
clab1 := testutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil)
clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil)
edsb.handleEDSResponse(parseEDSRespProtoForTesting(clab1.Build()))
sc1 := <-cc.NewSubConnCh
edsb.handleSubConnStateChange(sc1, connectivity.Connecting)
edsb.handleSubConnStateChange(sc1, connectivity.Ready)

// Test roundrobin with two subconns.
p1 := <-cc.NewPickerCh
// We call picks to make sure they don't panic.
for i := 0; i < 10; i++ {
p1.Pick(balancer.PickInfo{})
}
}
16 changes: 12 additions & 4 deletions xds/internal/balancer/edsbalancer/load_store_wrapper.go
Expand Up @@ -58,23 +58,31 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) {
func (lsw *loadStoreWrapper) CallStarted(locality string) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallStarted(locality)
if lsw.perCluster != nil {
lsw.perCluster.CallStarted(locality)
}
}

func (lsw *loadStoreWrapper) CallFinished(locality string, err error) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallFinished(locality, err)
if lsw.perCluster != nil {
lsw.perCluster.CallFinished(locality, err)
}
}

func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallServerLoad(locality, name, val)
if lsw.perCluster != nil {
lsw.perCluster.CallServerLoad(locality, name, val)
}
}

func (lsw *loadStoreWrapper) CallDropped(category string) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallDropped(category)
if lsw.perCluster != nil {
lsw.perCluster.CallDropped(category)
}
}
16 changes: 12 additions & 4 deletions xds/internal/balancer/lrs/balancer.go
Expand Up @@ -195,25 +195,33 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) {
func (lsw *loadStoreWrapper) CallStarted(locality string) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallStarted(locality)
if lsw.perCluster != nil {
lsw.perCluster.CallStarted(locality)
}
}

func (lsw *loadStoreWrapper) CallFinished(locality string, err error) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallFinished(locality, err)
if lsw.perCluster != nil {
lsw.perCluster.CallFinished(locality, err)
}
}

func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallServerLoad(locality, name, val)
if lsw.perCluster != nil {
lsw.perCluster.CallServerLoad(locality, name, val)
}
}

func (lsw *loadStoreWrapper) CallDropped(category string) {
lsw.mu.RLock()
defer lsw.mu.RUnlock()
lsw.perCluster.CallDropped(category)
if lsw.perCluster != nil {
lsw.perCluster.CallDropped(category)
}
}

type xdsClientWrapper struct {
Expand Down

0 comments on commit bce1fdf

Please sign in to comment.