balacergroup: cleanup exitIdle()

[hugehoo]

context

exitIdle() is called in both ExitIdle() and ExitIdleOne()

func (bg *BalancerGroup) ExitIdle() {
	bg.outgoingMu.Lock()
	defer bg.outgoingMu.Unlock()
	if bg.outgoingClosed {
		return
	}
	for _, config := range bg.idToBalancerConfig {
		if !config.exitIdle() { // here
			bg.connect(config)
		}
	}
}

func (bg *BalancerGroup) ExitIdleOne(id string) {
	bg.outgoingMu.Lock()
	defer bg.outgoingMu.Unlock()
	if bg.outgoingClosed {
		return
	}
	if config := bg.idToBalancerConfig[id]; config != nil {
		if !config.exitIdle() { // here
			bg.connect(config)
		}
	}
}

it returns boolean value. if it returns false if-statement works.
But the current implementation of exitIdle() is returning only true.

As-Is

exitIdle method returns boolean type but implementation returns only true.

func (sbc *subBalancerWrapper) exitIdle() (complete bool) {
	b := sbc.balancer
	if b == nil {
		return true
	}
	b.ExitIdle()
	return true
}

To-be

so i simplified implementation as method only returns true.
And fixed comment too as Returns a true

// exitIdle invokes the sub-balancer's ExitIdle method. Returns a true
// indicating whether or not the operation was completed.
func (sbc *subBalancerWrapper) exitIdle() (complete bool) {
	if b := sbc.balancer; b != nil {
		b.ExitIdle()
	}
	return true
}

I'm not sure this refactoring is necessary but at least the comment on exitIdle seems to be updated.

RELEASE NOTES: N/A

[codecov]

Codecov Report

Attention: Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 82.28%. Comparing base (32e57de) to head (827d468).
Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/balancergroup/balancergroup.go	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8347      +/-   ##
==========================================
- Coverage   82.35%   82.28%   -0.07%     
==========================================
  Files         419      419              
  Lines       42034    42039       +5     
==========================================
- Hits        34615    34591      -24     
- Misses       5968     5985      +17     
- Partials     1451     1463      +12     

Files with missing lines	Coverage Δ
internal/balancergroup/balancergroup.go	80.40% <75.00%> (+3.70%)	⬆️

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hugehoo]

nit) i suggest that whether or not to be whether.

[injae-kim]

oh.. return true * 2 😅 hard to understand code.. refactor +1!

[eshitachandwani]

Hey @hugehoo , thank you for the PR , I have a few suggestions since the code has changed.
Since the function only returns true , you can change it to not return anything and just call ExitIdle and also remove the condition in balancerGroup because it is never executed and the functionality of connecting to all subconns has been passed down to gracefulSwitch Balancer and since the balancer in subBalancerWrapper has been changed to gracefulSwitch balancer, it will always have ExitIdle function:

func (sbc *subBalancerWrapper) exitIdle(){
	b := sbc.balancer
	if b == nil {
		return
	}
	b.ExitIdle()
}

And change the these parts as follows:

if config := bg.idToBalancerConfig[id]; config != nil {
	config.exitIdle()
}

[eshitachandwani]

Also do add this to the description:
RELEASE NOTES: N/A

[hugehoo]

i removed here, it fails CI-test because it's unused.

[eshitachandwani]

nit) i suggest that whether or not to be whether.

Can you add a clarification in comments that the sub-balancer is a graceful switch balancer

[eshitachandwani]

Can you add a clarification in comments that the sub-balancer is a graceful switch balancer

[hugehoo]

@eshitachandwani I added it 👍🏻

[eshitachandwani]

LGTM modulo one minor comment , adding @arjan-bal for another review.

[eshitachandwani]

Suggested change

	// exitIdle invokes the sub-balancer's ExitIdle method
	// which is a graceful switch balancer.
	// exitIdle invokes the ExitIdle method on the sub-balancer, a gracefulswitch
	// balancer.

[arjan-bal]

LGTM, thanks for the cleanup!