Repository: grpc/grpc-go
File: balancer/pickfirst/pickfirstleaf/pickfirstleaf.go (Go)

Code before:
		}
	}, connectionDelayInterval)
}

func (b *pickfirstBalancer) updateSubConnState(sd *scData, newState balancer.SubConnState) {
	b.mu.Lock()
	defer b.mu.Unlock()
	oldState := sd.state
	// Record a connection attempt when existing CONNECTING.
	if newState.ConnectivityState == connectivity.TransientFailure {
		sd.connectionAttempted = true
	}
	sd.state = newState.ConnectivityState
	// Previously relevant SubConns can still callback with state updates.
	// To prevent pickers from returning these obsolete SubConns, this logic
	// is included to check if the current list of active SubConns includes this
	// SubConn.
	if activeSD, found := b.subConns.Get(sd.addr); !found || activeSD != sd {
		return
	}
	if newState.ConnectivityState == connectivity.Shutdown {
		return
	}

	if newState.ConnectivityState == connectivity.Ready {
		b.shutdownRemainingLocked(sd)
		if !b.addressList.seekTo(sd.addr) {
			// This should not fail as we should have only one SubConn after
			// entering READY. The SubConn should be present in the addressList.
			b.logger.Errorf("Address %q not found address list in  %v", sd.addr, b.addressList.addresses)
			return
		}
		b.state = connectivity.Ready
		b.cc.UpdateState(balancer.State{
			ConnectivityState: connectivity.Ready,
			Picker:            &picker{result: balancer.PickResult{SubConn: sd.subConn}},
		})
		return
	}

	// If the LB policy is READY, and it receives a subchannel state change,
	// it means that the READY subchannel has failed.
	// A SubConn can also transition from CONNECTING directly to IDLE when
	// a transport is successfully created, but the connection fails
	// before the SubConn can send the notification for READY. We treat
	// this as a successful connection and transition to IDLE.
	if (b.state == connectivity.Ready && newState.ConnectivityState != connectivity.Ready) || (oldState == connectivity.Connecting && newState.ConnectivityState == connectivity.Idle) {
		// Once a transport fails, the balancer enters IDLE and starts from
		// the first address when the picker is used.
		b.shutdownRemainingLocked(sd)
		b.state = connectivity.Idle

Review comment (bug):
Optional:

I understand that this `for` and `switch` is existing code. But it took me a little while to figure out that the only time we run through the loop is when we are seeing subchannels in TF. Whenever we see a subchannel in any other state, we hit the `return` statement on the last line of the `for` loop`. Do you think it might be better to write this as follows instead

```golang
func (b *pickfirstBalancer) requestConnectionLocked() {
	...
	for (...) {
		...
		switch scd.state {
		case connectivity.Idle:
			...
			return
		case connectivity.TransientFailure:
			...
			continue
		case connectivity.Ready:
			...
			return
		case connectivity.Shutdown:
			...
			return
		case connectivity.Connecting:
			...
			return
		}
	}
}
```

I acknowledge that this will be a few more lines of code, but it does make it very explicit that we return in all cases other than TF.

Code after:
	if !b.addressList.isValid() {
		return
	}
	var lastErr error
	for valid := true; valid; valid = b.addressList.increment() {
		curAddr := b.addressList.currentAddress()
		sd, ok := b.subConns.Get(curAddr)
		if !ok {
			var err error
			// We want to assign the new scData to sd from the outer scope,
			// hence we can't use := below.
			sd, err = b.newSCData(curAddr)
			if err != nil {
				// This should never happen, unless the clientConn is being shut
				// down.
				if b.logger.V(2) {
					b.logger.Infof("Failed to create a subConn for address %v: %v", curAddr.String(), err)
				}
				// Do nothing, the LB policy will be closed soon.
				return
			}
			b.subConns.Set(curAddr, sd)
		}

		scd := sd.(*scData)
		switch scd.state {
		case connectivity.Idle:
			scd.subConn.Connect()
			b.scheduleNextConnectionLocked()
			return
		case connectivity.TransientFailure:
			// The SubConn is being re-used and failed during a previous pass
			// over the addressList. It has not completed backoff yet.
			// Mark it as having failed and try the next address.
			scd.connectionFailedInFirstPass = true
			lastErr = scd.lastErr
			continue
		case connectivity.Connecting:
			// Wait for the connection attempt to complete or the timer to fire
			// before attempting the next address.
			b.scheduleNextConnectionLocked()
			return
		default:
			b.logger.Errorf("SubConn with unexpected state %v present in SubConns map.", scd.state)
			return

		}
	}

	// All the remaining addresses in the list are in TRANSIENT_FAILURE, end the
	// first pass if possible.