Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/root/configuration/cluster_manager/cluster_stats.rst
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ Circuit breakers statistics will be rooted at *cluster.<name>.circuit_breakers.<
rq_pending_open, BoolIndicator, Whether the pending requests circuit breaker is closed (false) or open (true)
rq_open, BoolIndicator, Whether the requests circuit breaker is closed (false) or open (true)
rq_retry_open, BoolIndicator, Whether the retry circuit breaker is closed (false) or open (true)
remaining_cx, Gauge, Number of remaining connections until the circuit breaker opens
remaining_pending, Gauge, Number of remaining pending requests until the circuit breaker opens
remaining_rq, Gauge, Number of remaining requests until the circuit breaker opens
remaining_retries, Gauge, Number of remaining retries until the circuit breaker opens

.. _config_cluster_manager_cluster_stats_dynamic_http:

Expand Down
3 changes: 2 additions & 1 deletion docs/root/intro/arch_overview/circuit_breaking.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ configure and code each application independently. Envoy supports various types
Each circuit breaking limit is :ref:`configurable <config_cluster_manager_cluster_circuit_breakers>`
and tracked on a per upstream cluster and per priority basis. This allows different components of
the distributed system to be tuned independently and have different limits. The live state of these
circuit breakers can be observed via :ref:`statistics <config_cluster_manager_cluster_stats_circuit_breakers>`.
circuit breakers, including the number of resources remaining until a circuit breaker opens, can
be observed via :ref:`statistics <config_cluster_manager_cluster_stats_circuit_breakers>`.

Note that circuit breaking will cause the :ref:`x-envoy-overloaded
<config_http_filters_router_x-envoy-overloaded_set>` header to be set by the router filter in the
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ Version history
* stats: added usedonly flag to prometheus stats to only output metrics which have been
updated at least once.
* stats: added BoolIndicator stat type, converted the following 1-or-0 Gauges: control_plane.connected_state, cx_open, rq_pending_open, rq_open, rq_retry_open, runtime.admin_overrides_active, open_gauge, config.active, server.live.
* stats: added gauges tracking remaining resources before circuit breakers open.
* tap: added new alpha :ref:`HTTP tap filter <config_http_filters_tap>`.
* tls: enabled TLS 1.3 on the server-side (non-FIPS builds).
* upstream: add hash_function to specify the hash function for :ref:`ring hash<envoy_api_msg_Cluster.RingHashLbConfig>` as either xxHash or `murmurHash2 <https://sites.google.com/site/murmurhash>`_. MurmurHash2 is compatible with std::hash in GNU libstdc++ 3.4.20 or above. This is typically the case when compiled on Linux and not macOS.
Expand Down
10 changes: 7 additions & 3 deletions include/envoy/upstream/upstream.h
Original file line number Diff line number Diff line change
Expand Up @@ -524,11 +524,15 @@ class PrioritySet {
* Cluster circuit breakers stats.
*/
// clang-format off
#define ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(BOOL_INDICATOR) \
#define ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(BOOL_INDICATOR, GAUGE) \
BOOL_INDICATOR (cx_open) \
BOOL_INDICATOR (rq_pending_open) \
BOOL_INDICATOR (rq_open) \
BOOL_INDICATOR (rq_retry_open)
BOOL_INDICATOR (rq_retry_open) \
GAUGE (remaining_cx) \
GAUGE (remaining_pending) \
GAUGE (remaining_rq) \
GAUGE (remaining_retries)
// clang-format on

/**
Expand All @@ -549,7 +553,7 @@ struct ClusterLoadReportStats {
* Struct definition for cluster circuit breakers stats. @see stats_macros.h
*/
struct ClusterCircuitBreakersStats {
ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_BOOL_INDICATOR_STRUCT)
ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(GENERATE_BOOL_INDICATOR_STRUCT, GENERATE_GAUGE_STRUCT)
};

/**
Expand Down
37 changes: 31 additions & 6 deletions source/common/upstream/resource_manager_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ class ResourceManagerImpl : public ResourceManager {
uint64_t max_connections, uint64_t max_pending_requests,
uint64_t max_requests, uint64_t max_retries,
ClusterCircuitBreakersStats cb_stats)
: connections_(max_connections, runtime, runtime_key + "max_connections", cb_stats.cx_open_),
: connections_(max_connections, runtime, runtime_key + "max_connections", cb_stats.cx_open_,
cb_stats.remaining_cx_),
pending_requests_(max_pending_requests, runtime, runtime_key + "max_pending_requests",
cb_stats.rq_pending_open_),
requests_(max_requests, runtime, runtime_key + "max_requests", cb_stats.rq_open_),
retries_(max_retries, runtime, runtime_key + "max_retries", cb_stats.rq_retry_open_) {}
cb_stats.rq_pending_open_, cb_stats.remaining_pending_),
requests_(max_requests, runtime, runtime_key + "max_requests", cb_stats.rq_open_,
cb_stats.remaining_rq_),
retries_(max_retries, runtime, runtime_key + "max_retries", cb_stats.rq_retry_open_,
cb_stats.remaining_retries_) {}

// Upstream::ResourceManager
Resource& connections() override { return connections_; }
Expand All @@ -44,24 +47,41 @@ class ResourceManagerImpl : public ResourceManager {
private:
struct ResourceImpl : public Resource {
ResourceImpl(uint64_t max, Runtime::Loader& runtime, const std::string& runtime_key,
Stats::BoolIndicator& circuit_breaker_open)
Stats::BoolIndicator& circuit_breaker_open, Stats::Gauge& remaining)
: max_(max), runtime_(runtime), runtime_key_(runtime_key),
circuit_breaker_open_(circuit_breaker_open) {}
circuit_breaker_open_(circuit_breaker_open), remaining_(remaining) {
remaining_.set(max);
}
~ResourceImpl() { ASSERT(current_ == 0); }

// Upstream::Resource
bool canCreate() override { return current_ < max(); }
void inc() override {
current_++;
circuit_breaker_open_.set(!canCreate());
updateRemaining();
}
void dec() override {
ASSERT(current_ > 0);
current_--;
circuit_breaker_open_.set(!canCreate());
updateRemaining();
}
uint64_t max() override { return runtime_.snapshot().getInteger(runtime_key_, max_); }

/**
* We set the gauge instead of incrementing and decrementing because,
* though atomics are used, it is possible for the current resource count
* to be greater than the supplied max.
*/
void updateRemaining() {
/**
* We cannot use std::max here because max() and current_ are
* unsigned and subtracting them may overflow.
*/
remaining_.set(max() > current_ ? max() - current_ : 0);
Comment thread
spenceral marked this conversation as resolved.
Outdated
}

const uint64_t max_;
std::atomic<uint64_t> current_{};
Runtime::Loader& runtime_;
Expand All @@ -72,6 +92,11 @@ class ResourceManagerImpl : public ResourceManager {
* true when open.
*/
Stats::BoolIndicator& circuit_breaker_open_;

/**
* The number of resources remaining before the circuit breaker opens.
*/
Stats::Gauge& remaining_;
};

ResourceImpl connections_;
Expand Down
3 changes: 2 additions & 1 deletion source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,8 @@ ClusterInfoImpl::ResourceManagers::ResourceManagers(const envoy::api::v2::Cluste
ClusterCircuitBreakersStats
ClusterInfoImpl::generateCircuitBreakersStats(Stats::Scope& scope, const std::string& stat_prefix) {
std::string prefix(fmt::format("circuit_breakers.{}.", stat_prefix));
return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR_PREFIX(scope, prefix))};
return {ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR_PREFIX(scope, prefix),
POOL_GAUGE_PREFIX(scope, prefix))};
}

ResourceManagerImplPtr
Expand Down
8 changes: 6 additions & 2 deletions test/common/http/http1/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "test/test_common/printers.h"
#include "test/test_common/utility.h"

#include "absl/strings/match.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -128,9 +129,12 @@ class Http1ConnPoolImplTest : public testing::Test {
conn_pool_(dispatcher_, cluster_, upstream_ready_timer_) {}

~Http1ConnPoolImplTest() {
// Make sure all gauges are 0.
// Make sure all gauges are 0, except those in circuit_breakers which default
// to the resource max.
for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) {
EXPECT_EQ(0U, gauge->value());
if (!absl::StrContains(gauge->name(), "circuit_breakers")) {
EXPECT_EQ(0U, gauge->value());
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions test/common/http/http2/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "test/mocks/upstream/mocks.h"
#include "test/test_common/printers.h"

#include "absl/strings/match.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -68,9 +69,12 @@ class Http2ConnPoolImplTest : public testing::Test {
pool_(dispatcher_, host_, Upstream::ResourcePriority::Default, nullptr) {}

~Http2ConnPoolImplTest() {
// Make sure all gauges are 0.
// Make sure all gauges are 0, except those in circuit_breakers which default
// to the resource max.
for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) {
EXPECT_EQ(0U, gauge->value());
if (!absl::StrContains(gauge->name(), "circuit_breakers")) {
EXPECT_EQ(0U, gauge->value());
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions test/common/tcp/conn_pool_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "test/test_common/printers.h"
#include "test/test_common/utility.h"

#include "absl/strings/match.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -159,9 +160,12 @@ class TcpConnPoolImplTest : public testing::Test {
conn_pool_(dispatcher_, cluster_, upstream_ready_timer_) {}

~TcpConnPoolImplTest() {
// Make sure all gauges are 0.
// Make sure all gauges are 0, except those in circuit_breakers which default
// to the resource max.
for (const Stats::GaugeSharedPtr& gauge : cluster_->stats_store_.gauges()) {
EXPECT_EQ(0U, gauge->value());
if (!absl::StrContains(gauge->name(), "circuit_breakers")) {
EXPECT_EQ(0U, gauge->value());
}
}
}

Expand Down
51 changes: 50 additions & 1 deletion test/common/upstream/resource_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ namespace {
TEST(ResourceManagerImplTest, RuntimeResourceManager) {
NiceMock<Runtime::MockLoader> runtime;
NiceMock<Stats::MockBoolIndicator> bool_indicator;
NiceMock<Stats::MockGauge> gauge;
NiceMock<Stats::MockStore> store;

ON_CALL(store, boolIndicator(_)).WillByDefault(ReturnRef(bool_indicator));
ON_CALL(store, gauge(_)).WillByDefault(ReturnRef(gauge));

ResourceManagerImpl resource_manager(
runtime, "circuit_breakers.runtime_resource_manager_test.default.", 0, 0, 0, 1,
ClusterCircuitBreakersStats{ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR(store))});
ClusterCircuitBreakersStats{
ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR(store), POOL_GAUGE(store))});

EXPECT_CALL(
runtime.snapshot_,
Expand Down Expand Up @@ -59,6 +62,52 @@ TEST(ResourceManagerImplTest, RuntimeResourceManager) {
EXPECT_FALSE(resource_manager.retries().canCreate());
}

TEST(ResourceManagerImplTest, RemainingResourceGauges) {
NiceMock<Runtime::MockLoader> runtime;
Stats::IsolatedStoreImpl store;

auto stats = ClusterCircuitBreakersStats{
ALL_CLUSTER_CIRCUIT_BREAKERS_STATS(POOL_BOOL_INDICATOR(store), POOL_GAUGE(store))};
ResourceManagerImpl resource_manager(
runtime, "circuit_breakers.runtime_resource_manager_test.default.", 1, 2, 1, 0, stats);

// Test remaining_cx_ gauge
EXPECT_EQ(1U, resource_manager.connections().max());
EXPECT_EQ(1U, stats.remaining_cx_.value());
resource_manager.connections().inc();
EXPECT_EQ(0U, stats.remaining_cx_.value());
resource_manager.connections().dec();
EXPECT_EQ(1U, stats.remaining_cx_.value());

// Test remaining_pending_ gauge
EXPECT_EQ(2U, resource_manager.pendingRequests().max());
EXPECT_EQ(2U, stats.remaining_pending_.value());
resource_manager.pendingRequests().inc();
EXPECT_EQ(1U, stats.remaining_pending_.value());
resource_manager.pendingRequests().inc();
EXPECT_EQ(0U, stats.remaining_pending_.value());
resource_manager.pendingRequests().dec();
EXPECT_EQ(1U, stats.remaining_pending_.value());
resource_manager.pendingRequests().dec();
EXPECT_EQ(2U, stats.remaining_pending_.value());

// Test remaining_rq_ gauge
EXPECT_EQ(1U, resource_manager.requests().max());
EXPECT_EQ(1U, stats.remaining_rq_.value());
resource_manager.requests().inc();
EXPECT_EQ(0U, stats.remaining_rq_.value());
resource_manager.requests().dec();
EXPECT_EQ(1U, stats.remaining_rq_.value());

// Test remaining_retries_ gauge. Confirm that the value will not be negative
// despite having more retries than the configured max
EXPECT_EQ(0U, resource_manager.retries().max());
EXPECT_EQ(0U, stats.remaining_retries_.value());
resource_manager.retries().inc();
EXPECT_EQ(0U, stats.remaining_retries_.value());
resource_manager.retries().dec();
EXPECT_EQ(0U, stats.remaining_retries_.value());
}
} // namespace
} // namespace Upstream
} // namespace Envoy
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "test/mocks/upstream/mocks.h"
#include "test/test_common/printers.h"

#include "absl/strings/match.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"

Expand Down Expand Up @@ -52,9 +53,12 @@ class RedisClientImplTest : public testing::Test, public Common::Redis::DecoderF
~RedisClientImplTest() {
client_.reset();

// Make sure all gauges are 0.
// Make sure all gauges are 0, except those in circuit_breakers which default
// to the resource max.
for (const Stats::GaugeSharedPtr& gauge : host_->cluster_.stats_store_.gauges()) {
EXPECT_EQ(0U, gauge->value());
if (!absl::StrContains(gauge->name(), "circuit_breakers")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this whitelisting is fine, thought it might make sense at one point to add a initialValue() function to Gauge so that these checks can just be EXPECT_EQ(gauge->initialValue(), gauge->value()), but that would arguably fall out of the scope of this PR

@spenceral spenceral Mar 12, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will defer to your (or anyone else's) judgment on whether or not to make that change here. It makes sense to me.

EXPECT_EQ(0U, gauge->value());
}
}
for (const Stats::GaugeSharedPtr& gauge : host_->stats_store_.gauges()) {
EXPECT_EQ(0U, gauge->value());
Expand Down