From a041e0f3ae90e9a6248a331a9ddaab55324e16da Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 22 Jan 2018 18:18:00 -0800 Subject: Change if to switch, add more descriptions --- src/core/ext/filters/max_age/max_age_filter.cc | 83 ++++++++++++++++++-------- 1 file changed, 57 insertions(+), 26 deletions(-) (limited to 'src/core/ext/filters') diff --git a/src/core/ext/filters/max_age/max_age_filter.cc b/src/core/ext/filters/max_age/max_age_filter.cc index 942e890407..a3f9780f3f 100644 --- a/src/core/ext/filters/max_age/max_age_filter.cc +++ b/src/core/ext/filters/max_age/max_age_filter.cc @@ -91,6 +91,7 @@ struct channel_data { grpc_connectivity_state connectivity_state; /* Number of active calls */ gpr_atm call_count; + /* TODO(zyc): C++lize this state machine */ /* 'idle_state' holds the states of max_idle_timer and channel idleness. It can contain one of the following values: +--------------------------------+----------------+---------+ @@ -101,10 +102,32 @@ struct channel_data { | MAX_IDLE_STATE_SEEN_EXIT_IDLE | set, invalid | busy | | MAX_IDLE_STATE_SEEN_ENTER_IDLE | set, invalid | idle | +--------------------------------+----------------+---------+ + + MAX_IDLE_STATE_INIT: The initial and final state of 'idle_state'. The + channel has 1 or 1+ active calls, and the the timer is not set. Note that + we may put a virtual call to hold this state at channel initialization or + shutdown, so that the channel won't enter other states. + + MAX_IDLE_STATE_TIMER_SET: The state after the timer is set and no calls + have arrived after the timer is set. The channel must have 0 active call in + this state. If the timer is fired in this state, we will close the channel + due to idleness. + + MAX_IDLE_STATE_SEEN_EXIT_IDLE: The state after the timer is set and at + least one call has arrived after the timer is set. The channel must have 1 + or 1+ active calls in this state. If the timer is fired in this state, we + won't reschudle it. + + MAX_IDLE_STATE_SEEN_ENTER_IDLE: The state after the timer is set and the at + least one call has arrived after the timer is set, BUT the channel + currently has 1 or 1+ active calls. If the timer is fired in this state, we + will reschudle it. + max_idle_timer will not be cancelled (unless the channel is shutting down). If the timer callback is called when the max_idle_timer is valid (i.e. idle_state is MAX_IDLE_STATE_TIMER_SET), the channel will be closed due to idleness, otherwise the channel won't be changed. + State transitions: MAX_IDLE_STATE_INIT <-------3------ MAX_IDLE_STATE_SEEN_EXIT_IDLE ^ | ^ ^ | @@ -113,6 +136,7 @@ struct channel_data { | | | | | | v | | v MAX_IDLE_STATE_TIMER_SET <----5------ MAX_IDLE_STATE_SEEN_ENTER_IDLE + For 1, 3, 5 : See max_idle_timer_cb() function For 2, 7 : See decrease_call_count() function For 4, 6 : See increase_call_count() function */ @@ -240,34 +264,41 @@ static void close_max_idle_channel(channel_data* chand) { static void max_idle_timer_cb(void* arg, grpc_error* error) { channel_data* chand = (channel_data*)arg; if (error == GRPC_ERROR_NONE) { - while (true) { + bool try_again = true; + while (try_again) { gpr_atm idle_state = gpr_atm_acq_load(&chand->idle_state); - if (idle_state == MAX_IDLE_STATE_TIMER_SET) { - close_max_idle_channel(chand); - /* This MAX_IDLE_STATE_INIT is a final state, we don't have to check if - * idle_state has been changed */ - gpr_atm_rel_store(&chand->idle_state, MAX_IDLE_STATE_INIT); - break; - } else if (idle_state == MAX_IDLE_STATE_SEEN_EXIT_IDLE) { - if (gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_EXIT_IDLE, - MAX_IDLE_STATE_INIT)) { + switch (idle_state) { + case MAX_IDLE_STATE_TIMER_SET: + close_max_idle_channel(chand); + /* This MAX_IDLE_STATE_INIT is a final state, we don't have to check + * if idle_state has been changed */ + gpr_atm_rel_store(&chand->idle_state, MAX_IDLE_STATE_INIT); + try_again = false; + break; + case MAX_IDLE_STATE_SEEN_EXIT_IDLE: + if (gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_EXIT_IDLE, + MAX_IDLE_STATE_INIT)) { + try_again = false; + } + break; + case MAX_IDLE_STATE_SEEN_ENTER_IDLE: + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age max_idle_timer"); + grpc_timer_init(&chand->max_idle_timer, + (grpc_millis)gpr_atm_no_barrier_load( + &chand->last_enter_idle_time_millis) + + chand->max_connection_idle, + &chand->max_idle_timer_cb); + /* idle_state may have already been set to + MAX_IDLE_STATE_SEEN_EXIT_IDLE by increase_call_count(), in this + case, we don't need to set it to MAX_IDLE_STATE_TIMER_SET */ + gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_ENTER_IDLE, + MAX_IDLE_STATE_TIMER_SET); + try_again = false; + break; + default: + /* try again */ break; - } - } else if (idle_state == MAX_IDLE_STATE_SEEN_ENTER_IDLE) { - GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_idle_timer"); - grpc_timer_init(&chand->max_idle_timer, - (grpc_millis)gpr_atm_no_barrier_load( - &chand->last_enter_idle_time_millis) + - chand->max_connection_idle, - &chand->max_idle_timer_cb); - /* idle_state may have already been set to MAX_IDLE_STATE_SEEN_EXIT_IDLE - by increase_call_count(), in this case, we don't need to set it to - MAX_IDLE_STATE_TIMER_SET */ - gpr_atm_rel_cas(&chand->idle_state, MAX_IDLE_STATE_SEEN_ENTER_IDLE, - MAX_IDLE_STATE_TIMER_SET); - break; - } else { - /* try again */ } } } -- cgit v1.2.3