diff options
Diffstat (limited to 'src/core/ext/filters/client_channel')
4 files changed, 169 insertions, 170 deletions
diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 67dd3a1fa7..51f9ae000a 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -63,6 +63,7 @@ #include "src/core/lib/transport/status_metadata.h" using grpc_core::internal::ClientChannelMethodParams; +using grpc_core::internal::ServerRetryThrottleData; /* Client channel implementation */ @@ -99,7 +100,7 @@ typedef struct client_channel_channel_data { /** currently active load balancer */ grpc_core::OrphanablePtr<grpc_core::LoadBalancingPolicy> lb_policy; /** retry throttle data */ - grpc_server_retry_throttle_data* retry_throttle_data; + grpc_core::RefCountedPtr<ServerRetryThrottleData> retry_throttle_data; /** maps method names to method_parameters structs */ grpc_core::RefCountedPtr<MethodParamsTable> method_params_table; /** incoming resolver result - set by resolver.next() */ @@ -225,7 +226,7 @@ static void start_resolving_locked(channel_data* chand) { typedef struct { char* server_name; - grpc_server_retry_throttle_data* retry_throttle_data; + grpc_core::RefCountedPtr<ServerRetryThrottleData> retry_throttle_data; } service_config_parsing_state; static void parse_retry_throttle_params( @@ -278,7 +279,7 @@ static void parse_retry_throttle_params( } } parsing_state->retry_throttle_data = - grpc_retry_throttle_map_get_data_for_server( + grpc_core::internal::ServerRetryThrottleMap::GetDataForServer( parsing_state->server_name, max_milli_tokens, milli_token_ratio); } } @@ -321,7 +322,7 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) { bool lb_policy_name_changed = false; grpc_core::OrphanablePtr<grpc_core::LoadBalancingPolicy> new_lb_policy; char* service_config_json = nullptr; - grpc_server_retry_throttle_data* retry_throttle_data = nullptr; + grpc_core::RefCountedPtr<ServerRetryThrottleData> retry_throttle_data; grpc_core::RefCountedPtr<MethodParamsTable> method_params_table; if (chand->resolver_result != nullptr) { if (chand->resolver != nullptr) { @@ -421,7 +422,7 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) { service_config->ParseGlobalParams(parse_retry_throttle_params, &parsing_state); grpc_uri_destroy(uri); - retry_throttle_data = parsing_state.retry_throttle_data; + retry_throttle_data = std::move(parsing_state.retry_throttle_data); } method_params_table = service_config->CreateMethodConfigTable( ClientChannelMethodParams::CreateFromJson); @@ -452,10 +453,7 @@ static void on_resolver_result_changed_locked(void* arg, grpc_error* error) { } gpr_mu_unlock(&chand->info_mu); // Swap out the retry throttle data. - if (chand->retry_throttle_data != nullptr) { - grpc_server_retry_throttle_data_unref(chand->retry_throttle_data); - } - chand->retry_throttle_data = retry_throttle_data; + chand->retry_throttle_data = std::move(retry_throttle_data); // Swap out the method params table. chand->method_params_table = std::move(method_params_table); // If we have a new LB policy or are shutting down (in which case @@ -725,12 +723,8 @@ static void cc_destroy_channel_elem(grpc_channel_element* elem) { } gpr_free(chand->info_lb_policy_name); gpr_free(chand->info_service_config_json); - if (chand->retry_throttle_data != nullptr) { - grpc_server_retry_throttle_data_unref(chand->retry_throttle_data); - } - if (chand->method_params_table != nullptr) { - chand->method_params_table.reset(); - } + chand->retry_throttle_data.reset(); + chand->method_params_table.reset(); grpc_client_channel_stop_backup_polling(chand->interested_parties); grpc_connectivity_state_destroy(&chand->state_tracker); grpc_pollset_set_destroy(chand->interested_parties); @@ -883,7 +877,7 @@ typedef struct client_channel_call_data { grpc_call_stack* owning_call; grpc_call_combiner* call_combiner; - grpc_server_retry_throttle_data* retry_throttle_data; + grpc_core::RefCountedPtr<ServerRetryThrottleData> retry_throttle_data; grpc_core::RefCountedPtr<ClientChannelMethodParams> method_params; grpc_subchannel_call* subchannel_call; @@ -1443,7 +1437,9 @@ static bool maybe_retry(grpc_call_element* elem, } // Check status. if (status == GRPC_STATUS_OK) { - grpc_server_retry_throttle_data_record_success(calld->retry_throttle_data); + if (calld->retry_throttle_data != nullptr) { + calld->retry_throttle_data->RecordSuccess(); + } if (grpc_client_channel_trace.enabled()) { gpr_log(GPR_DEBUG, "chand=%p calld=%p: call succeeded", chand, calld); } @@ -1465,8 +1461,8 @@ static bool maybe_retry(grpc_call_element* elem, // things like failures due to malformed requests (INVALID_ARGUMENT). // Conversely, it's important for this to come before the remaining // checks, so that we don't fail to record failures due to other factors. - if (!grpc_server_retry_throttle_data_record_failure( - calld->retry_throttle_data)) { + if (calld->retry_throttle_data != nullptr && + !calld->retry_throttle_data->RecordFailure()) { if (grpc_client_channel_trace.enabled()) { gpr_log(GPR_DEBUG, "chand=%p calld=%p: retries throttled", chand, calld); } @@ -2601,8 +2597,7 @@ static void apply_service_config_to_call_locked(grpc_call_element* elem) { chand, calld); } if (chand->retry_throttle_data != nullptr) { - calld->retry_throttle_data = - grpc_server_retry_throttle_data_ref(chand->retry_throttle_data); + calld->retry_throttle_data = chand->retry_throttle_data->Ref(); } if (chand->method_params_table != nullptr) { calld->method_params = grpc_core::ServiceConfig::MethodConfigTableLookup( @@ -2994,6 +2989,7 @@ static void cc_destroy_call_elem(grpc_call_element* elem, grpc_deadline_state_destroy(elem); } grpc_slice_unref_internal(calld->path); + calld->retry_throttle_data.reset(); calld->method_params.reset(); GRPC_ERROR_UNREF(calld->cancel_error); if (calld->subchannel_call != nullptr) { diff --git a/src/core/ext/filters/client_channel/client_channel_plugin.cc b/src/core/ext/filters/client_channel/client_channel_plugin.cc index 3c3a97532f..b4ef534d2e 100644 --- a/src/core/ext/filters/client_channel/client_channel_plugin.cc +++ b/src/core/ext/filters/client_channel/client_channel_plugin.cc @@ -65,7 +65,7 @@ static bool set_default_host_if_unset(grpc_channel_stack_builder* builder, void grpc_client_channel_init(void) { grpc_core::LoadBalancingPolicyRegistry::Builder::InitRegistry(); grpc_core::ResolverRegistry::Builder::InitRegistry(); - grpc_retry_throttle_map_init(); + grpc_core::internal::ServerRetryThrottleMap::Init(); grpc_proxy_mapper_registry_init(); grpc_register_http_proxy_mapper(); grpc_subchannel_index_init(); @@ -81,7 +81,7 @@ void grpc_client_channel_shutdown(void) { grpc_subchannel_index_shutdown(); grpc_channel_init_shutdown(); grpc_proxy_mapper_registry_shutdown(); - grpc_retry_throttle_map_shutdown(); + grpc_core::internal::ServerRetryThrottleMap::Shutdown(); grpc_core::ResolverRegistry::Builder::ShutdownRegistry(); grpc_core::LoadBalancingPolicyRegistry::Builder::ShutdownRegistry(); } diff --git a/src/core/ext/filters/client_channel/retry_throttle.cc b/src/core/ext/filters/client_channel/retry_throttle.cc index 45de6667c8..bdeb7e4cac 100644 --- a/src/core/ext/filters/client_channel/retry_throttle.cc +++ b/src/core/ext/filters/client_channel/retry_throttle.cc @@ -30,184 +30,162 @@ #include "src/core/lib/avl/avl.h" +namespace grpc_core { +namespace internal { + // -// server_retry_throttle_data +// ServerRetryThrottleData // -struct grpc_server_retry_throttle_data { - gpr_refcount refs; - int max_milli_tokens; - int milli_token_ratio; - gpr_atm milli_tokens; - // A pointer to the replacement for this grpc_server_retry_throttle_data - // entry. If non-nullptr, then this entry is stale and must not be used. - // We hold a reference to the replacement. - gpr_atm replacement; -}; - -static void get_replacement_throttle_data_if_needed( - grpc_server_retry_throttle_data** throttle_data) { +ServerRetryThrottleData::ServerRetryThrottleData( + intptr_t max_milli_tokens, intptr_t milli_token_ratio, + ServerRetryThrottleData* old_throttle_data) + : max_milli_tokens_(max_milli_tokens), + milli_token_ratio_(milli_token_ratio) { + intptr_t initial_milli_tokens = max_milli_tokens; + // If there was a pre-existing entry for this server name, initialize + // the token count by scaling proportionately to the old data. This + // ensures that if we're already throttling retries on the old scale, + // we will start out doing the same thing on the new one. + if (old_throttle_data != nullptr) { + double token_fraction = + static_cast<intptr_t>( + gpr_atm_acq_load(&old_throttle_data->milli_tokens_)) / + static_cast<double>(old_throttle_data->max_milli_tokens_); + initial_milli_tokens = + static_cast<intptr_t>(token_fraction * max_milli_tokens); + } + gpr_atm_rel_store(&milli_tokens_, static_cast<gpr_atm>(initial_milli_tokens)); + // If there was a pre-existing entry, mark it as stale and give it a + // pointer to the new entry, which is its replacement. + if (old_throttle_data != nullptr) { + Ref().release(); // Ref held by pre-existing entry. + gpr_atm_rel_store(&old_throttle_data->replacement_, + reinterpret_cast<gpr_atm>(this)); + } +} + +ServerRetryThrottleData::~ServerRetryThrottleData() { + ServerRetryThrottleData* replacement = + reinterpret_cast<ServerRetryThrottleData*>( + gpr_atm_acq_load(&replacement_)); + if (replacement != nullptr) { + replacement->Unref(); + } +} + +void ServerRetryThrottleData::GetReplacementThrottleDataIfNeeded( + ServerRetryThrottleData** throttle_data) { while (true) { - grpc_server_retry_throttle_data* new_throttle_data = - (grpc_server_retry_throttle_data*)gpr_atm_acq_load( - &(*throttle_data)->replacement); + ServerRetryThrottleData* new_throttle_data = + reinterpret_cast<ServerRetryThrottleData*>( + gpr_atm_acq_load(&(*throttle_data)->replacement_)); if (new_throttle_data == nullptr) return; *throttle_data = new_throttle_data; } } -bool grpc_server_retry_throttle_data_record_failure( - grpc_server_retry_throttle_data* throttle_data) { - if (throttle_data == nullptr) return true; +bool ServerRetryThrottleData::RecordFailure() { // First, check if we are stale and need to be replaced. - get_replacement_throttle_data_if_needed(&throttle_data); + ServerRetryThrottleData* throttle_data = this; + GetReplacementThrottleDataIfNeeded(&throttle_data); // We decrement milli_tokens by 1000 (1 token) for each failure. - const int new_value = static_cast<int>(gpr_atm_no_barrier_clamped_add( - &throttle_data->milli_tokens, static_cast<gpr_atm>(-1000), - static_cast<gpr_atm>(0), - static_cast<gpr_atm>(throttle_data->max_milli_tokens))); + const intptr_t new_value = + static_cast<intptr_t>(gpr_atm_no_barrier_clamped_add( + &throttle_data->milli_tokens_, static_cast<gpr_atm>(-1000), + static_cast<gpr_atm>(0), + static_cast<gpr_atm>(throttle_data->max_milli_tokens_))); // Retries are allowed as long as the new value is above the threshold // (max_milli_tokens / 2). - return new_value > throttle_data->max_milli_tokens / 2; + return new_value > throttle_data->max_milli_tokens_ / 2; } -void grpc_server_retry_throttle_data_record_success( - grpc_server_retry_throttle_data* throttle_data) { - if (throttle_data == nullptr) return; +void ServerRetryThrottleData::RecordSuccess() { // First, check if we are stale and need to be replaced. - get_replacement_throttle_data_if_needed(&throttle_data); + ServerRetryThrottleData* throttle_data = this; + GetReplacementThrottleDataIfNeeded(&throttle_data); // We increment milli_tokens by milli_token_ratio for each success. gpr_atm_no_barrier_clamped_add( - &throttle_data->milli_tokens, - static_cast<gpr_atm>(throttle_data->milli_token_ratio), + &throttle_data->milli_tokens_, + static_cast<gpr_atm>(throttle_data->milli_token_ratio_), static_cast<gpr_atm>(0), - static_cast<gpr_atm>(throttle_data->max_milli_tokens)); -} - -grpc_server_retry_throttle_data* grpc_server_retry_throttle_data_ref( - grpc_server_retry_throttle_data* throttle_data) { - gpr_ref(&throttle_data->refs); - return throttle_data; -} - -void grpc_server_retry_throttle_data_unref( - grpc_server_retry_throttle_data* throttle_data) { - if (gpr_unref(&throttle_data->refs)) { - grpc_server_retry_throttle_data* replacement = - (grpc_server_retry_throttle_data*)gpr_atm_acq_load( - &throttle_data->replacement); - if (replacement != nullptr) { - grpc_server_retry_throttle_data_unref(replacement); - } - gpr_free(throttle_data); - } -} - -static grpc_server_retry_throttle_data* grpc_server_retry_throttle_data_create( - int max_milli_tokens, int milli_token_ratio, - grpc_server_retry_throttle_data* old_throttle_data) { - grpc_server_retry_throttle_data* throttle_data = - static_cast<grpc_server_retry_throttle_data*>( - gpr_malloc(sizeof(*throttle_data))); - memset(throttle_data, 0, sizeof(*throttle_data)); - gpr_ref_init(&throttle_data->refs, 1); - throttle_data->max_milli_tokens = max_milli_tokens; - throttle_data->milli_token_ratio = milli_token_ratio; - int initial_milli_tokens = max_milli_tokens; - // If there was a pre-existing entry for this server name, initialize - // the token count by scaling proportionately to the old data. This - // ensures that if we're already throttling retries on the old scale, - // we will start out doing the same thing on the new one. - if (old_throttle_data != nullptr) { - double token_fraction = - static_cast<int>(gpr_atm_acq_load(&old_throttle_data->milli_tokens)) / - static_cast<double>(old_throttle_data->max_milli_tokens); - initial_milli_tokens = static_cast<int>(token_fraction * max_milli_tokens); - } - gpr_atm_rel_store(&throttle_data->milli_tokens, - (gpr_atm)initial_milli_tokens); - // If there was a pre-existing entry, mark it as stale and give it a - // pointer to the new entry, which is its replacement. - if (old_throttle_data != nullptr) { - grpc_server_retry_throttle_data_ref(throttle_data); - gpr_atm_rel_store(&old_throttle_data->replacement, (gpr_atm)throttle_data); - } - return throttle_data; + static_cast<gpr_atm>(throttle_data->max_milli_tokens_)); } // // avl vtable for string -> server_retry_throttle_data map // -static void* copy_server_name(void* key, void* unused) { +namespace { + +void* copy_server_name(void* key, void* unused) { return gpr_strdup(static_cast<const char*>(key)); } -static long compare_server_name(void* key1, void* key2, void* unused) { +long compare_server_name(void* key1, void* key2, void* unused) { return strcmp(static_cast<const char*>(key1), static_cast<const char*>(key2)); } -static void destroy_server_retry_throttle_data(void* value, void* unused) { - grpc_server_retry_throttle_data* throttle_data = - static_cast<grpc_server_retry_throttle_data*>(value); - grpc_server_retry_throttle_data_unref(throttle_data); +void destroy_server_retry_throttle_data(void* value, void* unused) { + ServerRetryThrottleData* throttle_data = + static_cast<ServerRetryThrottleData*>(value); + throttle_data->Unref(); } -static void* copy_server_retry_throttle_data(void* value, void* unused) { - grpc_server_retry_throttle_data* throttle_data = - static_cast<grpc_server_retry_throttle_data*>(value); - return grpc_server_retry_throttle_data_ref(throttle_data); +void* copy_server_retry_throttle_data(void* value, void* unused) { + ServerRetryThrottleData* throttle_data = + static_cast<ServerRetryThrottleData*>(value); + return throttle_data->Ref().release(); } -static void destroy_server_name(void* key, void* unused) { gpr_free(key); } +void destroy_server_name(void* key, void* unused) { gpr_free(key); } -static const grpc_avl_vtable avl_vtable = { +const grpc_avl_vtable avl_vtable = { destroy_server_name, copy_server_name, compare_server_name, destroy_server_retry_throttle_data, copy_server_retry_throttle_data}; +} // namespace + // -// server_retry_throttle_map +// ServerRetryThrottleMap // static gpr_mu g_mu; static grpc_avl g_avl; -void grpc_retry_throttle_map_init() { +void ServerRetryThrottleMap::Init() { gpr_mu_init(&g_mu); g_avl = grpc_avl_create(&avl_vtable); } -void grpc_retry_throttle_map_shutdown() { +void ServerRetryThrottleMap::Shutdown() { gpr_mu_destroy(&g_mu); grpc_avl_unref(g_avl, nullptr); } -grpc_server_retry_throttle_data* grpc_retry_throttle_map_get_data_for_server( - const char* server_name, int max_milli_tokens, int milli_token_ratio) { +RefCountedPtr<ServerRetryThrottleData> ServerRetryThrottleMap::GetDataForServer( + const char* server_name, intptr_t max_milli_tokens, + intptr_t milli_token_ratio) { + RefCountedPtr<ServerRetryThrottleData> result; gpr_mu_lock(&g_mu); - grpc_server_retry_throttle_data* throttle_data = - static_cast<grpc_server_retry_throttle_data*>( + ServerRetryThrottleData* throttle_data = + static_cast<ServerRetryThrottleData*>( grpc_avl_get(g_avl, const_cast<char*>(server_name), nullptr)); - if (throttle_data == nullptr) { - // Entry not found. Create a new one. - throttle_data = grpc_server_retry_throttle_data_create( - max_milli_tokens, milli_token_ratio, nullptr); - g_avl = grpc_avl_add(g_avl, const_cast<char*>(server_name), throttle_data, - nullptr); + if (throttle_data == nullptr || + throttle_data->max_milli_tokens() != max_milli_tokens || + throttle_data->milli_token_ratio() != milli_token_ratio) { + // Entry not found, or found with old parameters. Create a new one. + result = MakeRefCounted<ServerRetryThrottleData>( + max_milli_tokens, milli_token_ratio, throttle_data); + g_avl = grpc_avl_add(g_avl, gpr_strdup(server_name), + result->Ref().release(), nullptr); } else { - if (throttle_data->max_milli_tokens != max_milli_tokens || - throttle_data->milli_token_ratio != milli_token_ratio) { - // Entry found but with old parameters. Create a new one based on - // the original one. - throttle_data = grpc_server_retry_throttle_data_create( - max_milli_tokens, milli_token_ratio, throttle_data); - g_avl = grpc_avl_add(g_avl, const_cast<char*>(server_name), throttle_data, - nullptr); - } else { - // Entry found. Increase refcount. - grpc_server_retry_throttle_data_ref(throttle_data); - } + // Entry found. Return a new ref to it. + result = throttle_data->Ref(); } gpr_mu_unlock(&g_mu); - return throttle_data; + return result; } + +} // namespace internal +} // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/retry_throttle.h b/src/core/ext/filters/client_channel/retry_throttle.h index 0505fc27f2..2b6fa0a70b 100644 --- a/src/core/ext/filters/client_channel/retry_throttle.h +++ b/src/core/ext/filters/client_channel/retry_throttle.h @@ -21,32 +21,57 @@ #include <grpc/support/port_platform.h> -#include <stdbool.h> +#include "src/core/lib/gprpp/ref_counted.h" + +namespace grpc_core { +namespace internal { /// Tracks retry throttling data for an individual server name. -typedef struct grpc_server_retry_throttle_data grpc_server_retry_throttle_data; - -/// Records a failure. Returns true if it's okay to send a retry. -bool grpc_server_retry_throttle_data_record_failure( - grpc_server_retry_throttle_data* throttle_data); -/// Records a success. -void grpc_server_retry_throttle_data_record_success( - grpc_server_retry_throttle_data* throttle_data); - -grpc_server_retry_throttle_data* grpc_server_retry_throttle_data_ref( - grpc_server_retry_throttle_data* throttle_data); -void grpc_server_retry_throttle_data_unref( - grpc_server_retry_throttle_data* throttle_data); - -/// Initializes global map of failure data for each server name. -void grpc_retry_throttle_map_init(); -/// Shuts down global map of failure data for each server name. -void grpc_retry_throttle_map_shutdown(); - -/// Returns a reference to the failure data for \a server_name, creating -/// a new entry if needed. -/// Caller must eventually unref via \a grpc_server_retry_throttle_data_unref(). -grpc_server_retry_throttle_data* grpc_retry_throttle_map_get_data_for_server( - const char* server_name, int max_milli_tokens, int milli_token_ratio); +class ServerRetryThrottleData : public RefCounted<ServerRetryThrottleData> { + public: + ServerRetryThrottleData(intptr_t max_milli_tokens, intptr_t milli_token_ratio, + ServerRetryThrottleData* old_throttle_data); + + /// Records a failure. Returns true if it's okay to send a retry. + bool RecordFailure(); + + /// Records a success. + void RecordSuccess(); + + intptr_t max_milli_tokens() const { return max_milli_tokens_; } + intptr_t milli_token_ratio() const { return milli_token_ratio_; } + + private: + ~ServerRetryThrottleData(); + + void GetReplacementThrottleDataIfNeeded( + ServerRetryThrottleData** throttle_data); + + const intptr_t max_milli_tokens_; + const intptr_t milli_token_ratio_; + gpr_atm milli_tokens_; + // A pointer to the replacement for this ServerRetryThrottleData entry. + // If non-nullptr, then this entry is stale and must not be used. + // We hold a reference to the replacement. + gpr_atm replacement_ = 0; +}; + +/// Global map of server name to retry throttle data. +class ServerRetryThrottleMap { + public: + /// Initializes global map of failure data for each server name. + static void Init(); + /// Shuts down global map of failure data for each server name. + static void Shutdown(); + + /// Returns the failure data for \a server_name, creating a new entry if + /// needed. + static RefCountedPtr<ServerRetryThrottleData> GetDataForServer( + const char* server_name, intptr_t max_milli_tokens, + intptr_t milli_token_ratio); +}; + +} // namespace internal +} // namespace grpc_core #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_RETRY_THROTTLE_H */ |