natas_hw
/
grpc
0
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

subchannel: accept RequestConnection() only in IDLE state (#29978) 

* subchannel: accept RequestConnection() only in IDLE state

* fix crash
This commit is contained in:
Mark D. Roth 2022-06-10 17:39:12 -07:00 committed by GitHub
parent 8c780929c0
commit 5cd446ddde
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 36 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/core/ext/filters/client_channel/lb_policy/pick_first/pick_first.cc
View File

@ -346,6 +346,9 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
// TODO(qianchengz): We may want to request re-resolution in
// ExitIdleLocked().
p->channel_control_helper()->RequestReresolution();
// TODO(roth): We chould check the connectivity states of all the
// subchannels here, just in case one of them happens to be READY,
// and we could switch to that rather than going IDLE.
// Enter idle.
p->idle_ = true;
p->selected_ = nullptr;
@ -389,13 +392,10 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
case GRPC_CHANNEL_READY:
// Already handled this case above, so this should not happen.
GPR_UNREACHABLE_CODE(break);
case GRPC_CHANNEL_TRANSIENT_FAILURE:
case GRPC_CHANNEL_IDLE: {
PickFirstSubchannelData* sd = this;
size_t next_index =
(sd->Index() + 1) % subchannel_list()->num_subchannels();
case GRPC_CHANNEL_TRANSIENT_FAILURE: {
size_t next_index = (Index() + 1) % subchannel_list()->num_subchannels();
subchannel_list()->set_attempting_index(next_index);
sd = subchannel_list()->subchannel(next_index);
PickFirstSubchannelData* sd = subchannel_list()->subchannel(next_index);
// If we're tried all subchannels, set state to TRANSIENT_FAILURE.
if (sd->Index() == 0) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_pick_first_trace)) {
@ -432,7 +432,20 @@ void PickFirst::PickFirstSubchannelData::ProcessConnectivityChangeLocked(
absl::make_unique<TransientFailurePicker>(status));
}
}
sd->subchannel()->RequestConnection();
// If the next subchannel is in IDLE, trigger a connection attempt.
// If it's in READY, we can't get here, because we would already
// have selected the subchannel above.
// If it's already in CONNECTING, we don't need to do this.
// If it's in TRANSIENT_FAILURE, then we will trigger the
// connection attempt later when it reports IDLE.
auto sd_state = sd->connectivity_state();
if (sd_state.has_value() && *sd_state == GRPC_CHANNEL_IDLE) {
sd->subchannel()->RequestConnection();
}
break;
}
case GRPC_CHANNEL_IDLE: {
subchannel()->RequestConnection();
break;
}
case GRPC_CHANNEL_CONNECTING: {

7
src/core/ext/filters/client_channel/lb_policy/ring_hash/ring_hash.cc
View File

@ -711,10 +711,9 @@ void RingHash::RingHashSubchannelData::ProcessConnectivityChangeLocked(
}
GPR_ASSERT(subchannel() != nullptr);
// If this is not the initial state notification and the new state is
// TRANSIENT_FAILURE or IDLE, re-resolve and attempt to reconnect.
// Note that we don't want to do this on the initial state
// notification, because that would result in an endless loop of
// re-resolution.
// TRANSIENT_FAILURE or IDLE, re-resolve.
// Note that we don't want to do this on the initial state notification,
// because that would result in an endless loop of re-resolution.
if (old_state.has_value() && (new_state == GRPC_CHANNEL_TRANSIENT_FAILURE ||
new_state == GRPC_CHANNEL_IDLE)) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_ring_hash_trace)) {

18
src/core/ext/filters/client_channel/lb_policy/round_robin/round_robin.cc
View File

@ -131,10 +131,6 @@ class RoundRobin : public LoadBalancingPolicy {
// any references to subchannels, since the subchannels'
// pollset_sets will include the LB policy's pollset_set.
policy->Ref(DEBUG_LOCATION, "subchannel_list").release();
// Start connecting to all subchannels.
for (size_t i = 0; i < num_subchannels(); i++) {
subchannel(i)->subchannel()->RequestConnection();
}
}
~RoundRobinSubchannelList() override {
@ -417,10 +413,9 @@ void RoundRobin::RoundRobinSubchannelData::ProcessConnectivityChangeLocked(
RoundRobin* p = static_cast<RoundRobin*>(subchannel_list()->policy());
GPR_ASSERT(subchannel() != nullptr);
// If this is not the initial state notification and the new state is
// TRANSIENT_FAILURE or IDLE, re-resolve and attempt to reconnect.
// Note that we don't want to do this on the initial state
// notification, because that would result in an endless loop of
// re-resolution.
// TRANSIENT_FAILURE or IDLE, re-resolve.
// Note that we don't want to do this on the initial state notification,
// because that would result in an endless loop of re-resolution.
if (old_state.has_value() && (new_state == GRPC_CHANNEL_TRANSIENT_FAILURE ||
new_state == GRPC_CHANNEL_IDLE)) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) {
@ -429,6 +424,13 @@ void RoundRobin::RoundRobinSubchannelData::ProcessConnectivityChangeLocked(
subchannel(), ConnectivityStateName(new_state));
}
p->channel_control_helper()->RequestReresolution();
}
if (new_state == GRPC_CHANNEL_IDLE) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_round_robin_trace)) {
gpr_log(GPR_INFO,
"[RR %p] Subchannel %p reported IDLE; requesting connection", p,
subchannel());
}
subchannel()->RequestConnection();
}
// Update logical connectivity state.

17
src/core/ext/filters/client_channel/subchannel.cc
View File

@ -793,8 +793,6 @@ void Subchannel::RequestConnection() {
MutexLock lock(&mu_);
if (state_ == GRPC_CHANNEL_IDLE) {
StartConnectingLocked();
} else if (state_ == GRPC_CHANNEL_TRANSIENT_FAILURE) {
connection_requested_ = true;
}
}
@ -899,18 +897,9 @@ void Subchannel::OnRetryTimer() {
void Subchannel::OnRetryTimerLocked() {
if (shutdown_) return;
if (connection_requested_) {
gpr_log(GPR_INFO,
"subchannel %p %s: connection attempt requested while backoff "
"timer was pending, retrying now",
this, key_.ToString().c_str());
connection_requested_ = false;
StartConnectingLocked();
} else {
gpr_log(GPR_INFO, "subchannel %p %s: backoff delay elapsed, reporting IDLE",
this, key_.ToString().c_str());
SetConnectivityStateLocked(GRPC_CHANNEL_IDLE, absl::OkStatus());
}
gpr_log(GPR_INFO, "subchannel %p %s: backoff delay elapsed, reporting IDLE",
this, key_.ToString().c_str());
SetConnectivityStateLocked(GRPC_CHANNEL_IDLE, absl::OkStatus());
}
void Subchannel::StartConnectingLocked() {

3
src/core/ext/filters/client_channel/subchannel.h
View File

@ -390,9 +390,6 @@ class Subchannel : public DualRefCounted<Subchannel> {
bool shutdown_ ABSL_GUARDED_BY(mu_) = false;
// Records if RequestConnection() was called while in backoff.
bool connection_requested_ ABSL_GUARDED_BY(mu_) = false;
// Connectivity state tracking.
// Note that the connectivity state implies the state of the
// Subchannel object: