[pick_first] changes to support dualstack design #34218
Conversation
markdroth
added
the
release notes: no
| return; | ||
| } | ||
| // Ignore any other updates for subchannels we're not currently trying to | ||
| // connect to. | ||
| if (Index() != subchannel_list()->attempting_index()) return; | ||
| if (Index() != subchannel_list_->attempting_index_) return; |
There was a problem hiding this comment.
Lack of encapsulation for attempting_index_ seems unusual.
There was a problem hiding this comment.
I'm not sure what you mean by "encapsulation". If you mean the fact that we're directly accessing a data member, I think it's fine, because this access is coming from SubchannelData, which is nested inside of SubchannelList and can therefore access its data members by normal C++ scoping rules. In effect, SubchannelData is part of the implementation of SubchannelList, so it's fine to access its private data members. We use this pattern quite a bit.
| GPR_ASSERT(sc->connectivity_state().has_value()); | ||
| if (sc->connectivity_state() != GRPC_CHANNEL_TRANSIENT_FAILURE) { | ||
| subchannel_list()->set_attempting_index(next_index); | ||
| next_index < subchannel_list_->size(); ++next_index) { |
There was a problem hiding this comment.
645-667 seem to use more of subchannel_list_ - so maybe there should be a "find_next_subchannel_not_transient_failure` method in subchannel list class?
There was a problem hiding this comment.
This isn't a method that would ever need to be called from anywhere else, so I don't see any real benefit in moving it to a separate function.
grpc-checks
bot
added
per-call-memory/neutral
per-channel-memory/neutral
bloat/none
labels
copybara-service
bot
added
the
imported
This rolls forward only the pick_first changes from #32692, which were rolled back in #33718. Specifically:
GRPC_ARG_INHIBIT_HEALTH_CHECKINGchannel arg.