randomWRR: remove lock for accessing WRR.items

[arvindbr8]

Add() is always called as part of constructing the WRR object and never concurrently. Also turns out Next() is never called concurrently either. I started the PR (#5970) started out to avoid the race when accessing WRR.items in String(), but the mutex can be avoided all together for random WRR.

Also updating docstring to say the same thing.

RELEASE NOTES: N/A

[easwars]

Why is this file empty?

[dfawley]

+1

[arvindbr8]

oops. I had initially thought about adding a unit test for this package. But i realized this was basically covered by other tests indirectly. Removed

[dfawley]

+1

[dfawley]

This is risky as we'll deadlock if String is ever called while the lock is held.

Note that String is called indirectly, too, e.g. fmt.Sprint(rw). I don't think I like this. Maybe we should just exclude the items from what is printed and print only static info instead, with a different method that can retrieve the detailed data as a string if needed.

Consider:

func (rw *randomWRR) Whatever() {
	rw.mu.Lock()
	defer rw.mu.Unlock()
	...
	if badThing {
		rw.logger.Warningf("saw bad thing in %s", rw)
	}
	// or:
	if rw.logger.V(2) {
		rw.logger.Infof("saw weird thing in %s", rw)
	}
}

We'd deadlock in rare cases that might not be covered in tests.

[arvindbr8]

I just noticed that rw.mu is of type sync.RWMutex. Which means i could prolly use mu.RLock() in String(), which would avoid deadlocks on the mutex? wdyt? @dfawley

[arvindbr8]

Spoke with him offline.. my b, rLock still blocks on wLock.

[codecov]

Codecov Report

Merging #6666 (84f59a7) into master (afaf31a) will increase coverage by 0.06%.
The diff coverage is n/a.

Additional details and impacted files

[arvindbr8]

Life happens, and pull request intents change.

Discussed with Doug offline. Add() is always called as part of constructing the WRR object and never concurrently. Also turns out Next() is never called concurrently either. I started the PR started out to avoid the race when accessing WRR.items in String() to fix #5970, the mutex can be avoided all together for random WRR.

Updating the PR description

[dfawley]

Nits:

s/All//

s/Next()/Next/

[arvindbr8]

Done.

[dfawley]

Possibly for clarity: "Add may not be called after any call to Next."

[arvindbr8]

Done.