Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overload: scale transport socket connect timeout #13800

Merged
merged 22 commits into from
Jan 25, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
abcdb84
Split thread_local_object.h and overload_manager.h
akonradi Oct 23, 2020
f5b8d56
Scale transport socket handshake timeout
akonradi Oct 27, 2020
7d28234
Add tests for different scaled timer types
akonradi Oct 29, 2020
5d0d600
Change PrintTo to operator<<
akonradi Nov 3, 2020
76cd8c4
Merge remote-tracking branch 'upstream/master' into tls-scaled-timeout
akonradi Nov 3, 2020
2f169bf
Merge branch 'master' of https://github.com/envoyproxy/envoy into tls…
akonradi Nov 12, 2020
0c1cf92
Address review feedback
akonradi Nov 13, 2020
3376062
Address more review feedback
akonradi Nov 16, 2020
927ea02
Make ScaledTimerMinimum wrap, not inherit
akonradi Nov 17, 2020
68e62d4
Remove unused 'using'
akonradi Nov 17, 2020
9d85d8a
Merge branch 'master' of https://github.com/envoyproxy/envoy into tls…
akonradi Nov 24, 2020
29ea543
Fix mock method expectation
akonradi Dec 1, 2020
f52c76e
Merge branch 'master' of https://github.com/envoyproxy/envoy into tls…
akonradi Dec 1, 2020
a5ccd82
Address review feedback
akonradi Dec 7, 2020
18f6022
Merge branch 'master' of https://github.com/envoyproxy/envoy into tls…
akonradi Dec 8, 2020
4ca4b11
Add integration test
akonradi Dec 10, 2020
ee93040
Merge remote-tracking branch 'upstream/main' into tls-scaled-timeout
akonradi Jan 20, 2021
ddad3e5
Fix formatting
akonradi Jan 20, 2021
9be6e8f
Merge branch 'main' of https://github.com/envoyproxy/envoy into tls-s…
akonradi Jan 21, 2021
475253b
Document new scaled timer in release notes
akonradi Jan 21, 2021
2f2ed9f
Merge branch 'main' of https://github.com/envoyproxy/envoy into tls-s…
akonradi Jan 22, 2021
6ad33cf
Remove unused operator<< methods
akonradi Jan 22, 2021
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
5 changes: 5 additions & 0 deletions api/envoy/config/overload/v3/overload.proto
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ message ScaleTimersOverloadActionConfig {
// Adjusts the idle timer for downstream HTTP connections that takes effect when there are no active streams.
// This affects the value of :ref:`RouteAction.idle_timeout <envoy_v3_api_field_config.route.v3.RouteAction.idle_timeout>`.
HTTP_DOWNSTREAM_CONNECTION_IDLE = 1;

// Adjusts the timer for how long downstream clients have to finish transport-level negotiations before the connection is closed.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: This line seem very long. Can you rerun format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed manually since the formatter doesn't seem to touch proto comments.

// This affects the value of
// :ref:`FilterChain.transport_socket_connect_timeout <envoy_v3_api_field_config.listener.v3.FilterChain.transport_socket_connect_timeout>`.
TRANSPORT_SOCKET_CONNECT = 2;
}

message ScaleTimer {
Expand Down
5 changes: 5 additions & 0 deletions generated_api_shadow/envoy/config/overload/v3/overload.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions include/envoy/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ envoy_cc_library(
"//include/envoy/network:listen_socket_interface",
"//include/envoy/network:listener_interface",
"//include/envoy/network:transport_socket_interface",
"//include/envoy/server/overload:thread_local_overload_state",
"//include/envoy/thread:thread_interface",
],
)
Expand Down
9 changes: 5 additions & 4 deletions include/envoy/event/dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "envoy/network/listen_socket.h"
#include "envoy/network/listener.h"
#include "envoy/network/transport_socket.h"
#include "envoy/server/overload/thread_local_overload_state.h"
antoniovicente marked this conversation as resolved.
Show resolved Hide resolved
#include "envoy/stats/scope.h"
#include "envoy/stats/stats_macros.h"
#include "envoy/stream_info/stream_info.h"
Expand Down Expand Up @@ -90,12 +91,12 @@ class Dispatcher {
* connection. Takes ownership of the socket.
* @param transport_socket supplies a transport socket to be used by the connection.
* @param stream_info info object for the server connection
* @param overload_state the overload state for this dispatcher.
* @return Network::ConnectionPtr a server connection that is owned by the caller.
*/
virtual Network::ServerConnectionPtr
createServerConnection(Network::ConnectionSocketPtr&& socket,
Network::TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info) PURE;
virtual Network::ServerConnectionPtr createServerConnection(
Network::ConnectionSocketPtr&& socket, Network::TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info, Server::ThreadLocalOverloadState& overload_state) PURE;

/**
* Creates an instance of Envoy's Network::ClientConnection. Does NOT initiate the connection;
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/runtime/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ envoy_cc_library(
],
deps = [
"//include/envoy/stats:stats_interface",
"//include/envoy/thread_local:thread_local_interface",
"//include/envoy/thread_local:thread_local_object",
"//source/common/common:assert_lib",
"//source/common/singleton:threadsafe_singleton",
"@envoy_api//envoy/type/v3:pkg_cc_proto",
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/runtime/runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include "envoy/common/pure.h"
#include "envoy/stats/store.h"
#include "envoy/thread_local/thread_local.h"
#include "envoy/thread_local/thread_local_object.h"
#include "envoy/type/v3/percent.pb.h"

#include "common/common/assert.h"
Expand Down
18 changes: 4 additions & 14 deletions include/envoy/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ envoy_cc_library(
"//include/envoy/local_info:local_info_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/secret:secret_manager_interface",
"//include/envoy/server:overload_manager_interface",
"//include/envoy/server/overload:overload_manager_interface",
"//include/envoy/ssl:context_manager_interface",
"//include/envoy/thread_local:thread_local_interface",
"//include/envoy/tracing:http_tracer_interface",
Expand All @@ -150,8 +150,8 @@ envoy_cc_library(
name = "worker_interface",
hdrs = ["worker.h"],
deps = [
":overload_manager_interface",
"//include/envoy/server:guarddog_interface",
"//include/envoy/server/overload:overload_manager_interface",
],
)

Expand Down Expand Up @@ -184,7 +184,7 @@ envoy_cc_library(
"//include/envoy/local_info:local_info_interface",
"//include/envoy/network:drain_decision_interface",
"//include/envoy/runtime:runtime_interface",
"//include/envoy/server:overload_manager_interface",
"//include/envoy/server/overload:overload_manager_interface",
"//include/envoy/singleton:manager_interface",
"//include/envoy/thread_local:thread_local_interface",
"//include/envoy/tracing:http_tracer_interface",
Expand All @@ -208,8 +208,8 @@ envoy_cc_library(
"//include/envoy/config:typed_config_interface",
"//include/envoy/http:codes_interface",
"//include/envoy/http:filter_interface",
"//include/envoy/server:overload_manager_interface",
"//include/envoy/server:transport_socket_config_interface",
"//include/envoy/server/overload:overload_manager_interface",
"//include/envoy/singleton:manager_interface",
"//include/envoy/thread_local:thread_local_interface",
"//include/envoy/tracing:http_tracer_interface",
Expand Down Expand Up @@ -303,16 +303,6 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "overload_manager_interface",
hdrs = ["overload_manager.h"],
deps = [
"//include/envoy/event:timer_interface",
"//include/envoy/thread_local:thread_local_interface",
"//source/common/singleton:const_singleton",
],
)

envoy_cc_library(
name = "tracer_config_interface",
hdrs = ["tracer_config.h"],
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/server/factory_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include "envoy/server/admin.h"
#include "envoy/server/drain_manager.h"
#include "envoy/server/lifecycle_notifier.h"
#include "envoy/server/overload_manager.h"
#include "envoy/server/overload/overload_manager.h"
#include "envoy/server/process_context.h"
#include "envoy/singleton/manager.h"
#include "envoy/stats/scope.h"
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/server/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#include "envoy/server/lifecycle_notifier.h"
#include "envoy/server/listener_manager.h"
#include "envoy/server/options.h"
#include "envoy/server/overload_manager.h"
#include "envoy/server/overload/overload_manager.h"
#include "envoy/ssl/context_manager.h"
#include "envoy/thread_local/thread_local.h"
#include "envoy/tracing/http_tracer.h"
Expand Down
29 changes: 29 additions & 0 deletions include/envoy/server/overload/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
)

licenses(["notice"]) # Apache 2

envoy_package()

envoy_cc_library(
name = "overload_manager_interface",
hdrs = ["overload_manager.h"],
deps = [
":thread_local_overload_state",
"//include/envoy/event:dispatcher_interface",
"//include/envoy/thread_local:thread_local_interface",
"//source/common/singleton:const_singleton",
],
)

envoy_cc_library(
name = "thread_local_overload_state",
hdrs = ["thread_local_overload_state.h"],
deps = [
"//include/envoy/event:timer_interface",
"//include/envoy/thread_local:thread_local_object",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,13 @@
#include <string>

#include "envoy/common/pure.h"
#include "envoy/event/timer.h"
#include "envoy/thread_local/thread_local.h"
#include "envoy/event/dispatcher.h"
#include "envoy/server/overload/thread_local_overload_state.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

The refactor to add a directory for overload manager components in the server subdirectory and thread_local_object.h adds a lot of noise to this review. Worth doing it as a separate change instead? Also see comments about forward declaring instead.

Example:

class Dispatcher;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to do the refactor in a separate PR. I don't have strong opinions about forward declarations vs #includes other than an inclination to follow the style guide.


#include "common/common/macros.h"
#include "common/singleton/const_singleton.h"

namespace Envoy {
namespace Server {

/**
* Tracks the state of an overload action. The state is a number between 0 and 1 that represents the
* level of saturation. The values are categorized in two groups:
* - Saturated (value = 1): indicates that an overload action is active because at least one of its
* triggers has reached saturation.
* - Scaling (0 <= value < 1): indicates that an overload action is not saturated.
*/
class OverloadActionState {
public:
static constexpr OverloadActionState inactive() { return OverloadActionState(0); }

static constexpr OverloadActionState saturated() { return OverloadActionState(1.0); }

explicit constexpr OverloadActionState(float value)
: action_value_(std::min(1.0f, std::max(0.0f, value))) {}

float value() const { return action_value_; }
bool isSaturated() const { return action_value_ == 1; }

private:
float action_value_;
};

/**
* Callback invoked when an overload action changes state.
*/
using OverloadActionCb = std::function<void(OverloadActionState)>;

enum class OverloadTimerType {
// Timers created with this type will never be scaled. This should only be used for testing.
UnscaledRealTimerForTest,
// The amount of time an HTTP connection to a downstream client can remain idle (no streams). This
// corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto.
HttpDownstreamIdleConnectionTimeout,
};

/**
* Thread-local copy of the state of each configured overload action.
*/
class ThreadLocalOverloadState : public ThreadLocal::ThreadLocalObject {
public:
// Get a thread-local reference to the value for the given action key.
virtual const OverloadActionState& getState(const std::string& action) PURE;

// Get a scaled timer whose minimum corresponds to the configured value for the given timer type.
virtual Event::TimerPtr createScaledTimer(OverloadTimerType timer_type,
Event::TimerCb callback) PURE;
};

/**
* Well-known overload action names.
*/
Expand Down
67 changes: 67 additions & 0 deletions include/envoy/server/overload/thread_local_overload_state.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@

#pragma once

#include <string>

#include "envoy/common/pure.h"
#include "envoy/event/timer.h"
#include "envoy/thread_local/thread_local_object.h"

namespace Envoy {
namespace Server {

/**
* Tracks the state of an overload action. The state is a number between 0 and 1 that represents the
* level of saturation. The values are categorized in two groups:
* - Saturated (value = 1): indicates that an overload action is active because at least one of its
* triggers has reached saturation.
* - Scaling (0 <= value < 1): indicates that an overload action is not saturated.
*/
class OverloadActionState {
public:
static constexpr OverloadActionState inactive() { return OverloadActionState(0); }

static constexpr OverloadActionState saturated() { return OverloadActionState(1.0); }

explicit constexpr OverloadActionState(float value)
: action_value_(std::min(1.0f, std::max(0.0f, value))) {}

float value() const { return action_value_; }
bool isSaturated() const { return action_value_ == 1; }

private:
float action_value_;
};

/**
* Callback invoked when an overload action changes state.
*/
using OverloadActionCb = std::function<void(OverloadActionState)>;

enum class OverloadTimerType {
// Timers created with this type will never be scaled. This should only be used for testing.
UnscaledRealTimerForTest,
// The amount of time an HTTP connection to a downstream client can remain idle (no streams). This
// corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto.
HttpDownstreamIdleConnectionTimeout,
// The amount of time a connection to a downstream client can spend waiting for the transport to
// report connection establishment before the connection is closed.
// This corresponds to the TRANSPORT_SOCKET_CONNECT_TIMEOUT TimerType in overload.proto.
TransportSocketConnectTimeout,
};

/**
* Thread-local copy of the state of each configured overload action.
*/
class ThreadLocalOverloadState : public ThreadLocal::ThreadLocalObject {
public:
// Get a thread-local reference to the value for the given action key.
virtual const OverloadActionState& getState(const std::string& action) PURE;

// Get a scaled timer whose minimum corresponds to the configured value for the given timer type.
virtual Event::TimerPtr createScaledTimer(OverloadTimerType timer_type,
Event::TimerCb callback) PURE;
};

} // namespace Server
} // namespace Envoy
2 changes: 1 addition & 1 deletion include/envoy/server/worker.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include <functional>

#include "envoy/server/guarddog.h"
#include "envoy/server/overload_manager.h"
#include "envoy/server/overload/overload_manager.h"

namespace Envoy {
namespace Server {
Expand Down
12 changes: 11 additions & 1 deletion include/envoy/thread_local/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,15 @@ envoy_package()
envoy_cc_library(
name = "thread_local_interface",
hdrs = ["thread_local.h"],
deps = ["//include/envoy/event:dispatcher_interface"],
deps = [
":thread_local_object",
"//include/envoy/event:dispatcher_interface",
"//source/common/common:assert_lib",
],
)

envoy_cc_library(
name = "thread_local_object",
hdrs = ["thread_local_object.h"],
deps = ["//source/common/common:assert_lib"],
)
21 changes: 3 additions & 18 deletions include/envoy/thread_local/thread_local.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,13 @@

#include "envoy/common/pure.h"
#include "envoy/event/dispatcher.h"
#include "envoy/thread_local/thread_local_object.h"

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the dispatcher could be forward declared and dispatcher interface library depedency removed as a dependency if that would make some of these changes easier.

#include "common/common/assert.h"

namespace Envoy {
namespace ThreadLocal {

/**
* All objects that are stored via the ThreadLocal interface must derive from this type.
*/
class ThreadLocalObject {
public:
virtual ~ThreadLocalObject() = default;

/**
* Return the object casted to a concrete type. See getTyped() below for comments on the casts.
*/
template <class T> T& asType() {
ASSERT(dynamic_cast<T*>(this) != nullptr);
return *static_cast<T*>(this);
}
};

using ThreadLocalObjectSharedPtr = std::shared_ptr<ThreadLocalObject>;

/**
* An individual allocated TLS slot. When the slot is destroyed the stored thread local will
* be freed on each thread.
Expand Down
Loading