EventEngine::RunAfter: OutlierDetection LB Policy (#30040)

* EventEngine::RunAfter: OutlierDetection LB Policy

* iwyu, clang format, fix_auto_deps

* fix TSAN: EjectionTimer needs no cleanup on cancellation

* redo

* exec_ctx and fix use after move

* handle orphaning with an unset timer handle

* Automated change: Fix sanity tests

* reviewer feedback

Co-authored-by: drfloob <drfloob@users.noreply.github.com>
pull/31826/head
AJ Heller 2 years ago committed by GitHub
parent 322e85253e
commit 7eb99baad8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      src/core/BUILD
  2. 74
      src/core/ext/filters/client_channel/lb_policy/outlier_detection/outlier_detection.cc

@ -4245,8 +4245,6 @@ grpc_cc_library(
language = "c++",
deps = [
"channel_args",
"closure",
"error",
"grpc_outlier_detection_header",
"iomgr_fwd",
"json",
@ -4259,11 +4257,11 @@ grpc_cc_library(
"validation_errors",
"//:config",
"//:debug_location",
"//:exec_ctx",
"//:gpr",
"//:grpc_base",
"//:grpc_client_channel",
"//:grpc_trace",
"//:iomgr_timer",
"//:orphanable",
"//:ref_counted_ptr",
"//:server_address",

@ -52,11 +52,9 @@
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/gprpp/validation_errors.h"
#include "src/core/lib/gprpp/work_serializer.h"
#include "src/core/lib/iomgr/closure.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/iomgr/exec_ctx.h"
#include "src/core/lib/iomgr/iomgr_fwd.h"
#include "src/core/lib/iomgr/pollset_set.h"
#include "src/core/lib/iomgr/timer.h"
#include "src/core/lib/json/json.h"
#include "src/core/lib/load_balancing/lb_policy.h"
#include "src/core/lib/load_balancing/lb_policy_factory.h"
@ -71,6 +69,8 @@ TraceFlag grpc_outlier_detection_lb_trace(false, "outlier_detection_lb");
namespace {
using ::grpc_event_engine::experimental::EventEngine;
constexpr absl::string_view kOutlierDetection =
"outlier_detection_experimental";
@ -352,13 +352,10 @@ class OutlierDetectionLb : public LoadBalancingPolicy {
Timestamp StartTime() const { return start_time_; }
private:
static void OnTimer(void* arg, grpc_error_handle error);
void OnTimerLocked(grpc_error_handle);
void OnTimerLocked();
RefCountedPtr<OutlierDetectionLb> parent_;
grpc_timer timer_;
grpc_closure on_timer_;
bool timer_pending_ = true;
absl::optional<EventEngine::TaskHandle> timer_handle_;
Timestamp start_time_;
absl::BitGen bit_gen_;
};
@ -782,28 +779,28 @@ OutlierDetectionLb::EjectionTimer::EjectionTimer(
gpr_log(GPR_INFO, "[outlier_detection_lb %p] ejection timer will run in %s",
parent_.get(), interval.ToString().c_str());
}
GRPC_CLOSURE_INIT(&on_timer_, OnTimer, this, nullptr);
Ref().release();
grpc_timer_init(&timer_, start_time_ + interval, &on_timer_);
timer_handle_ = parent_->channel_control_helper()->GetEventEngine()->RunAfter(
interval, [self = Ref(DEBUG_LOCATION, "EjectionTimer")]() mutable {
ApplicationCallbackExecCtx callback_exec_ctx;
ExecCtx exec_ctx;
auto self_ptr = self.get();
self_ptr->parent_->work_serializer()->Run(
[self = std::move(self)]() { self->OnTimerLocked(); },
DEBUG_LOCATION);
});
}
void OutlierDetectionLb::EjectionTimer::Orphan() {
if (timer_pending_) {
timer_pending_ = false;
grpc_timer_cancel(&timer_);
if (timer_handle_.has_value()) {
parent_->channel_control_helper()->GetEventEngine()->Cancel(*timer_handle_);
timer_handle_.reset();
}
Unref();
}
void OutlierDetectionLb::EjectionTimer::OnTimer(void* arg,
grpc_error_handle error) {
auto* self = static_cast<EjectionTimer*>(arg);
self->parent_->work_serializer()->Run(
[self, error]() { self->OnTimerLocked(error); }, DEBUG_LOCATION);
}
void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
if (error.ok() && timer_pending_) {
void OutlierDetectionLb::EjectionTimer::OnTimerLocked() {
if (!timer_handle_.has_value()) return;
timer_handle_.reset();
if (GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(GPR_INFO, "[outlier_detection_lb %p] ejection timer running",
parent_.get());
@ -840,8 +837,7 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
if (config.failure_percentage_ejection.has_value()) {
if (request_volume >=
config.failure_percentage_ejection->request_volume) {
failure_percentage_ejection_candidates[subchannel_state] =
success_rate;
failure_percentage_ejection_candidates[subchannel_state] = success_rate;
}
}
}
@ -874,8 +870,7 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
variance /= success_rate_ejection_candidates.size();
double stdev = std::sqrt(variance);
const double success_rate_stdev_factor =
static_cast<double>(config.success_rate_ejection->stdev_factor) /
1000;
static_cast<double>(config.success_rate_ejection->stdev_factor) / 1000;
double ejection_threshold = mean - stdev * success_rate_stdev_factor;
if (GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(GPR_INFO,
@ -891,8 +886,8 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
}
if (candidate.second < ejection_threshold) {
uint32_t random_key = absl::Uniform(bit_gen_, 1, 100);
double current_percent = 100.0 * ejected_host_count /
parent_->subchannel_state_map_.size();
double current_percent =
100.0 * ejected_host_count / parent_->subchannel_state_map_.size();
if (GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(GPR_INFO,
"[outlier_detection_lb %p] random_key=%d "
@ -900,8 +895,7 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
parent_.get(), random_key, ejected_host_count,
current_percent);
}
if (random_key <
config.success_rate_ejection->enforcement_percentage &&
if (random_key < config.success_rate_ejection->enforcement_percentage &&
(ejected_host_count == 0 ||
(current_percent < config.max_ejection_percent))) {
// Eject and record the timestamp for use when ejecting addresses in
@ -921,8 +915,7 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
failure_percentage_ejection_candidates.size() >=
config.failure_percentage_ejection->minimum_hosts) {
if (GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(
GPR_INFO,
gpr_log(GPR_INFO,
"[outlier_detection_lb %p] running failure percentage algorithm",
parent_.get());
}
@ -939,8 +932,8 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
if ((100.0 - candidate.second) >
config.failure_percentage_ejection->threshold) {
uint32_t random_key = absl::Uniform(bit_gen_, 1, 100);
double current_percent = 100.0 * ejected_host_count /
parent_->subchannel_state_map_.size();
double current_percent =
100.0 * ejected_host_count / parent_->subchannel_state_map_.size();
if (GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(GPR_INFO,
"[outlier_detection_lb %p] random_key=%d "
@ -972,21 +965,16 @@ void OutlierDetectionLb::EjectionTimer::OnTimerLocked(grpc_error_handle error) {
// address.
for (auto& state : parent_->subchannel_state_map_) {
auto* subchannel_state = state.second.get();
const bool unejected =
subchannel_state->MaybeUneject(config.base_ejection_time.millis(),
config.max_ejection_time.millis());
if (unejected &&
GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
const bool unejected = subchannel_state->MaybeUneject(
config.base_ejection_time.millis(), config.max_ejection_time.millis());
if (unejected && GRPC_TRACE_FLAG_ENABLED(grpc_outlier_detection_lb_trace)) {
gpr_log(GPR_INFO, "[outlier_detection_lb %p] unejected address %s (%p)",
parent_.get(), state.first.c_str(), subchannel_state);
}
}
timer_pending_ = false;
parent_->ejection_timer_ =
MakeOrphanable<EjectionTimer>(parent_, Timestamp::Now());
}
Unref(DEBUG_LOCATION, "Timer");
}
//
// factory

Loading…
Cancel
Save