@@ -214,8 +214,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
214214 */
215215 private connectionDelayTimeout : NodeJS . Timeout ;
216216
217- private triedAllSubchannels = false ;
218-
219217 /**
220218 * The LB policy enters sticky TRANSIENT_FAILURE mode when all
221219 * subchannels have failed to connect at least once, and it stays in that
@@ -226,12 +224,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
226224
227225 private reportHealthStatus : boolean ;
228226
229- /**
230- * Indicates whether we called channelControlHelper.requestReresolution since
231- * the last call to updateAddressList
232- */
233- private requestedResolutionSinceLastUpdate = false ;
234-
235227 /**
236228 * The most recent error reported by any subchannel as it transitioned to
237229 * TRANSIENT_FAILURE.
@@ -261,6 +253,10 @@ export class PickFirstLoadBalancer implements LoadBalancer {
261253 return this . children . every ( child => child . hasReportedTransientFailure ) ;
262254 }
263255
256+ private resetChildrenReportedTF ( ) {
257+ this . children . every ( child => child . hasReportedTransientFailure = false ) ;
258+ }
259+
264260 private calculateAndReportNewState ( ) {
265261 if ( this . currentPick ) {
266262 if ( this . reportHealthStatus && ! this . currentPick . isHealthy ( ) ) {
@@ -293,23 +289,15 @@ export class PickFirstLoadBalancer implements LoadBalancer {
293289 }
294290
295291 private requestReresolution ( ) {
296- this . requestedResolutionSinceLastUpdate = true ;
297292 this . channelControlHelper . requestReresolution ( ) ;
298293 }
299294
300295 private maybeEnterStickyTransientFailureMode ( ) {
301296 if ( ! this . allChildrenHaveReportedTF ( ) ) {
302297 return ;
303298 }
304- if ( ! this . requestedResolutionSinceLastUpdate ) {
305- /* Each time we get an update we reset each subchannel's
306- * hasReportedTransientFailure flag, so the next time we get to this
307- * point after that, each subchannel has reported TRANSIENT_FAILURE
308- * at least once since then. That is the trigger for requesting
309- * reresolution, whether or not the LB policy is already in sticky TF
310- * mode. */
311- this . requestReresolution ( ) ;
312- }
299+ this . requestReresolution ( ) ;
300+ this . resetChildrenReportedTF ( ) ;
313301 if ( this . stickyTransientFailureMode ) {
314302 this . calculateAndReportNewState ( ) ;
315303 return ;
@@ -323,21 +311,16 @@ export class PickFirstLoadBalancer implements LoadBalancer {
323311
324312 private removeCurrentPick ( ) {
325313 if ( this . currentPick !== null ) {
326- /* Unref can cause a state change, which can cause a change in the value
327- * of this.currentPick, so we hold a local reference to make sure that
328- * does not impact this function. */
329- const currentPick = this . currentPick ;
330- this . currentPick = null ;
331- currentPick . unref ( ) ;
332- currentPick . removeConnectivityStateListener ( this . subchannelStateListener ) ;
314+ this . currentPick . removeConnectivityStateListener ( this . subchannelStateListener ) ;
333315 this . channelControlHelper . removeChannelzChild (
334- currentPick . getChannelzRef ( )
316+ this . currentPick . getChannelzRef ( )
335317 ) ;
336- if ( this . reportHealthStatus ) {
337- currentPick . removeHealthStateWatcher (
338- this . pickedSubchannelHealthListener
339- ) ;
340- }
318+ this . currentPick . removeHealthStateWatcher (
319+ this . pickedSubchannelHealthListener
320+ ) ;
321+ // Unref last, to avoid triggering listeners
322+ this . currentPick . unref ( ) ;
323+ this . currentPick = null ;
341324 }
342325 }
343326
@@ -377,9 +360,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
377360
378361 private startNextSubchannelConnecting ( startIndex : number ) {
379362 clearTimeout ( this . connectionDelayTimeout ) ;
380- if ( this . triedAllSubchannels ) {
381- return ;
382- }
383363 for ( const [ index , child ] of this . children . entries ( ) ) {
384364 if ( index >= startIndex ) {
385365 const subchannelState = child . subchannel . getConnectivityState ( ) ;
@@ -392,7 +372,6 @@ export class PickFirstLoadBalancer implements LoadBalancer {
392372 }
393373 }
394374 }
395- this . triedAllSubchannels = true ;
396375 this . maybeEnterStickyTransientFailureMode ( ) ;
397376 }
398377
@@ -421,20 +400,25 @@ export class PickFirstLoadBalancer implements LoadBalancer {
421400 this . connectionDelayTimeout . unref ?.( ) ;
422401 }
423402
403+ /**
404+ * Declare that the specified subchannel should be used to make requests.
405+ * This functions the same independent of whether subchannel is a member of
406+ * this.children and whether it is equal to this.currentPick.
407+ * Prerequisite: subchannel.getConnectivityState() === READY.
408+ * @param subchannel
409+ */
424410 private pickSubchannel ( subchannel : SubchannelInterface ) {
425- if ( this . currentPick && subchannel . realSubchannelEquals ( this . currentPick ) ) {
426- return ;
427- }
428411 trace ( 'Pick subchannel with address ' + subchannel . getAddress ( ) ) ;
429412 this . stickyTransientFailureMode = false ;
430- this . removeCurrentPick ( ) ;
431- this . currentPick = subchannel ;
413+ /* Ref before removeCurrentPick and resetSubchannelList to avoid the
414+ * refcount dropping to 0 during this process. */
432415 subchannel . ref ( ) ;
433- if ( this . reportHealthStatus ) {
434- subchannel . addHealthStateWatcher ( this . pickedSubchannelHealthListener ) ;
435- }
436416 this . channelControlHelper . addChannelzChild ( subchannel . getChannelzRef ( ) ) ;
417+ this . removeCurrentPick ( ) ;
437418 this . resetSubchannelList ( ) ;
419+ subchannel . addConnectivityStateListener ( this . subchannelStateListener ) ;
420+ subchannel . addHealthStateWatcher ( this . pickedSubchannelHealthListener ) ;
421+ this . currentPick = subchannel ;
438422 clearTimeout ( this . connectionDelayTimeout ) ;
439423 this . calculateAndReportNewState ( ) ;
440424 }
@@ -451,20 +435,11 @@ export class PickFirstLoadBalancer implements LoadBalancer {
451435
452436 private resetSubchannelList ( ) {
453437 for ( const child of this . children ) {
454- if (
455- ! (
456- this . currentPick &&
457- child . subchannel . realSubchannelEquals ( this . currentPick )
458- )
459- ) {
460- /* The connectivity state listener is the same whether the subchannel
461- * is in the list of children or it is the currentPick, so if it is in
462- * both, removing it here would cause problems. In particular, that
463- * always happens immediately after the subchannel is picked. */
464- child . subchannel . removeConnectivityStateListener (
465- this . subchannelStateListener
466- ) ;
467- }
438+ /* Always remoev the connectivity state listener. If the subchannel is
439+ getting picked, it will be re-added then. */
440+ child . subchannel . removeConnectivityStateListener (
441+ this . subchannelStateListener
442+ ) ;
468443 /* Refs are counted independently for the children list and the
469444 * currentPick, so we call unref whether or not the child is the
470445 * currentPick. Channelz child references are also refcounted, so
@@ -476,20 +451,16 @@ export class PickFirstLoadBalancer implements LoadBalancer {
476451 }
477452 this . currentSubchannelIndex = 0 ;
478453 this . children = [ ] ;
479- this . triedAllSubchannels = false ;
480- this . requestedResolutionSinceLastUpdate = false ;
481454 }
482455
483456 private connectToAddressList ( addressList : SubchannelAddress [ ] ) {
457+ trace ( 'connectToAddressList([' + addressList . map ( address => subchannelAddressToString ( address ) ) + '])' ) ;
484458 const newChildrenList = addressList . map ( address => ( {
485459 subchannel : this . channelControlHelper . createSubchannel ( address , { } , null ) ,
486460 hasReportedTransientFailure : false ,
487461 } ) ) ;
488- trace ( 'connectToAddressList([' + addressList . map ( address => subchannelAddressToString ( address ) ) + '])' ) ;
489462 for ( const { subchannel } of newChildrenList ) {
490463 if ( subchannel . getConnectivityState ( ) === ConnectivityState . READY ) {
491- this . channelControlHelper . addChannelzChild ( subchannel . getChannelzRef ( ) ) ;
492- subchannel . addConnectivityStateListener ( this . subchannelStateListener ) ;
493464 this . pickSubchannel ( subchannel ) ;
494465 return ;
495466 }
0 commit comments