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 11 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
6 changes: 6 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,12 @@ 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.
// 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
6 changes: 6 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 @@ -32,6 +32,7 @@ envoy_cc_library(
"//include/envoy/network:listener_interface",
"//include/envoy/network:transport_socket_interface",
"//include/envoy/server:watchdog_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/server/watchdog.h"
#include "envoy/stats/scope.h"
#include "envoy/stats/stats_macros.h"
Expand Down Expand Up @@ -100,12 +101,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
39 changes: 30 additions & 9 deletions include/envoy/event/scaled_timer_minimum.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#pragma once

#include <chrono>
#include <ostream>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worried that adding this and the related inline methods here may have a detrimental effect on compile-time. Does this need to live in a header?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The method definitions could be moved to a .cc file. Should I put that under include or move the whole thing to source/common/common?

Copy link
Member

Choose a reason for hiding this comment

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

These ostream helpers don't have to be defined inside the struct and you don't even need friend since the members are public, so we can define in another .h files, where are they used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're used in ScaledTimerMinimum::operator<<, which does depend on private state.


#include "absl/types/variant.h"

Expand All @@ -11,32 +12,43 @@ namespace Event {
* Describes a minimum timer value that is equal to a scale factor applied to the maximum.
*/
struct ScaledMinimum {
explicit ScaledMinimum(double scale_factor) : scale_factor_(scale_factor) {}
explicit constexpr ScaledMinimum(double scale_factor) : scale_factor_(scale_factor) {}
inline bool operator==(const ScaledMinimum& other) const {
return other.scale_factor_ == scale_factor_;
}
inline friend std::ostream& operator<<(std::ostream& output, const ScaledMinimum& minimum) {
return output << "ScaledMinimum { scale_factor_ = " << minimum.scale_factor_ << " }";
}

const double scale_factor_;
};

/**
* Describes a minimum timer value that is an absolute duration.
*/
struct AbsoluteMinimum {
explicit AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {}
explicit constexpr AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {}
inline bool operator==(const AbsoluteMinimum& other) const { return other.value_ == value_; }
inline friend std::ostream& operator<<(std::ostream& output, const AbsoluteMinimum& minimum) {
return output << "AbsoluteMinimum { value = " << minimum.value_.count() << "ms }";
}
const std::chrono::milliseconds value_;
};

/**
* Class that describes how to compute a minimum timeout given a maximum timeout value. It wraps
* ScaledMinimum and AbsoluteMinimum and provides a single computeMinimum() method.
*/
class ScaledTimerMinimum : private absl::variant<ScaledMinimum, AbsoluteMinimum> {
class ScaledTimerMinimum {
public:
// Use base class constructor.
using absl::variant<ScaledMinimum, AbsoluteMinimum>::variant;
// Forward arguments to impl_'s constructor.
template <typename T> constexpr ScaledTimerMinimum(T arg) : impl_(arg) {}

// Computes the minimum value for a given maximum timeout. If this object was constructed with a
// - ScaledMinimum value:
// the return value is the scale factor applied to the provided maximum.
// the return value is the scale factor applied to the provided maximum.
// - AbsoluteMinimum:
// the return value is that minimum, and the provided maximum is ignored.
// the return value is that minimum, and the provided maximum is ignored.
std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const {
struct Visitor {
explicit Visitor(std::chrono::milliseconds value) : value_(value) {}
Expand All @@ -49,9 +61,18 @@ class ScaledTimerMinimum : private absl::variant<ScaledMinimum, AbsoluteMinimum>
}
const std::chrono::milliseconds value_;
};
return absl::visit<Visitor, const absl::variant<ScaledMinimum, AbsoluteMinimum>&>(
Visitor(maximum), *this);
return absl::visit(Visitor(maximum), impl_);
}

inline bool operator==(const ScaledTimerMinimum& other) const { return impl_ == other.impl_; }

inline friend std::ostream& operator<<(std::ostream& output, const ScaledTimerMinimum& minimum) {
return absl::visit([&](const auto& minimum) -> std::ostream& { return output << minimum; },
minimum.impl_);
}

private:
absl::variant<ScaledMinimum, AbsoluteMinimum> impl_;
};

} // namespace Event
Expand Down
4 changes: 4 additions & 0 deletions include/envoy/server/overload/thread_local_overload_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ enum class OverloadTimerType {
// 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,
};

/**
Expand Down
9 changes: 4 additions & 5 deletions source/common/event/dispatcher_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,13 +110,12 @@ void DispatcherImpl::clearDeferredDeleteList() {
deferred_deleting_ = false;
}

Network::ServerConnectionPtr
DispatcherImpl::createServerConnection(Network::ConnectionSocketPtr&& socket,
Network::TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info) {
Network::ServerConnectionPtr DispatcherImpl::createServerConnection(
Network::ConnectionSocketPtr&& socket, Network::TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info, Server::ThreadLocalOverloadState& overload_state) {
ASSERT(isThreadSafe());
return std::make_unique<Network::ServerConnectionImpl>(
*this, std::move(socket), std::move(transport_socket), stream_info, true);
*this, overload_state, std::move(socket), std::move(transport_socket), stream_info, true);
}

Network::ClientConnectionPtr
Expand Down
4 changes: 3 additions & 1 deletion source/common/event/dispatcher_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "envoy/event/deferred_deletable.h"
#include "envoy/event/dispatcher.h"
#include "envoy/network/connection_handler.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 "common/common/logger.h"
Expand Down Expand Up @@ -50,7 +51,8 @@ class DispatcherImpl : Logger::Loggable<Logger::Id::main>,
Network::ServerConnectionPtr
createServerConnection(Network::ConnectionSocketPtr&& socket,
Network::TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info) override;
StreamInfo::StreamInfo& stream_info,
Server::ThreadLocalOverloadState& overload_state) override;
Network::ClientConnectionPtr
createClientConnection(Network::Address::InstanceConstSharedPtr address,
Network::Address::InstanceConstSharedPtr source_address,
Expand Down
1 change: 1 addition & 0 deletions source/common/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ envoy_cc_library(
"//include/envoy/event:timer_interface",
"//include/envoy/network:connection_interface",
"//include/envoy/network:filter_interface",
"//include/envoy/server/overload:thread_local_overload_state",
"//source/common/buffer:buffer_lib",
"//source/common/buffer:watermark_buffer_lib",
"//source/common/common:assert_lib",
Expand Down
8 changes: 6 additions & 2 deletions source/common/network/connection_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "envoy/event/timer.h"
#include "envoy/network/filter.h"
#include "envoy/network/socket.h"
#include "envoy/server/overload/thread_local_overload_state.h"

#include "common/common/assert.h"
#include "common/common/empty_string.h"
Expand Down Expand Up @@ -731,19 +732,22 @@ void ConnectionImpl::flushWriteBuffer() {
}

ServerConnectionImpl::ServerConnectionImpl(Event::Dispatcher& dispatcher,
Server::ThreadLocalOverloadState& overload_state,
ConnectionSocketPtr&& socket,
TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info, bool connected)
: ConnectionImpl(dispatcher, std::move(socket), std::move(transport_socket), stream_info,
connected) {}
connected),
overload_state_(overload_state) {}

void ServerConnectionImpl::setTransportSocketConnectTimeout(std::chrono::milliseconds timeout) {
if (!transport_connect_pending_) {
return;
}
if (transport_socket_connect_timer_ == nullptr) {
transport_socket_connect_timer_ =
dispatcher_.createTimer([this] { onTransportSocketConnectTimeout(); });
overload_state_.createScaledTimer(Server::OverloadTimerType::TransportSocketConnectTimeout,
Copy link
Contributor

Choose a reason for hiding this comment

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

I did a local change that undid changes to this line and fixed all the unused variable compile errors, and noticed that the only test that failed //test/common/network:connection_impl_test due to changes to mock expectations. It would be good to have some e2e integration test coverage for this functionality.

This issue also came up in #14155 . We have multiple scaled timers, we should have some generic recipes for how to integration test scaled timer functionality. Possible strawman drive the connection to a certain state, force proxy into overload and verify that the timeout triggers shortly afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sent #14290 to do just that for the existing scaled timer.

[this] { onTransportSocketConnectTimeout(); });
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should revisit the possibility of having the dispatcher be the one that is aware of the overload state and provide a method to create scaled timers. Dispatcher is plumbed in widely.

I can see a counter argument involving our desire to bring overload manager closer to connections as a starting point for adjusting work done on wakeup based on overload state; something like smaller allocations/reads when under memory pressure. So this new plumbing may be more generally useful. But I think the direction we're taking there is to measure memory usage by connection, which actually doesn't require involvement from the overload mananger.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As mentioned elsewhere, I'd rather keep scaled timer creation going through the Overload Manager since it is ultimately responsible for determining the scale factor of those timers. We can revisit making the Dispatcher aware of the OM later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of plumbing the OM widely, could the dispatcher hold a reference to it? I dislike that there are now two very different places to create timers: OM and 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.

There's a chicken-and-egg problem there since the thread-local overload state requires a reference to the dispatcher. We could work around this, though, if that sounds preferable. It seems like the larger issue is that there is a divide between the dispatcher, which represents a thread worker, and thread-local state, which is managed elsewhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

The dispatcher is also owned by the thread. I wonder if the solution would be to have the dispatcher own the thread overload state instead of having it be part of the thread local storage.

Copy link
Member

Choose a reason for hiding this comment

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

The dispatcher is also owned by the thread. I wonder if the solution would be to have the dispatcher own the thread overload state instead of having it be part of the thread local storage.

Yeah I think that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

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 started working on this. It's doable but introduces some weirdness; see #14401 for more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a fresh attempt at this in #14679 which feels reasonably clean.

}
transport_socket_connect_timer_->enableTimer(timeout);
}
Expand Down
9 changes: 6 additions & 3 deletions source/common/network/connection_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <string>

#include "envoy/network/transport_socket.h"
#include "envoy/server/overload/thread_local_overload_state.h"

#include "common/buffer/watermark_buffer.h"
#include "common/event/libevent.h"
Expand Down Expand Up @@ -209,9 +210,10 @@ class ConnectionImpl : public ConnectionImplBase, public TransportSocketCallback

class ServerConnectionImpl : public ConnectionImpl, virtual public ServerConnection {
public:
ServerConnectionImpl(Event::Dispatcher& dispatcher, ConnectionSocketPtr&& socket,
TransportSocketPtr&& transport_socket, StreamInfo::StreamInfo& stream_info,
bool connected);
ServerConnectionImpl(Event::Dispatcher& dispatcher,
Server::ThreadLocalOverloadState& overload_state,
ConnectionSocketPtr&& socket, TransportSocketPtr&& transport_socket,
StreamInfo::StreamInfo& stream_info, bool connected);

// ServerConnection impl
void setTransportSocketConnectTimeout(std::chrono::milliseconds timeout) override;
Expand All @@ -220,6 +222,7 @@ class ServerConnectionImpl : public ConnectionImpl, virtual public ServerConnect
private:
void onTransportSocketConnectTimeout();

Server::ThreadLocalOverloadState& overload_state_;
bool transport_connect_pending_{true};
// Implements a timeout for the transport socket signaling connection. The timer is enabled by a
// call to setTransportSocketConnectTimeout and is reset when the connection is established.
Expand Down
1 change: 1 addition & 0 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ envoy_cc_library(
"//include/envoy/network:listener_interface",
"//include/envoy/server:active_udp_listener_config_interface",
"//include/envoy/server:listener_manager_interface",
"//include/envoy/server/overload:overload_manager_interface",
"//include/envoy/stats:timespan_interface",
"//source/common/common:linked_object",
"//source/common/common:non_copyable",
Expand Down
6 changes: 4 additions & 2 deletions source/server/connection_handler_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ void emitLogs(Network::ListenerConfig& config, StreamInfo::StreamInfo& stream_in
} // namespace

ConnectionHandlerImpl::ConnectionHandlerImpl(Event::Dispatcher& dispatcher,
Server::OverloadManager& overload_manager,
absl::optional<uint32_t> worker_index)
: worker_index_(worker_index), dispatcher_(dispatcher),
: worker_index_(worker_index), dispatcher_(dispatcher), overload_manager_(overload_manager),
per_handler_stat_prefix_(dispatcher.name() + "."), disable_listeners_(false) {}

void ConnectionHandlerImpl::incNumConnections() { ++num_handler_connections_; }
Expand Down Expand Up @@ -481,7 +482,8 @@ void ConnectionHandlerImpl::ActiveTcpListener::newConnection(
stream_info->setDownstreamSslConnection(transport_socket->ssl());
auto& active_connections = getOrCreateActiveConnections(*filter_chain);
auto server_conn_ptr = parent_.dispatcher_.createServerConnection(
std::move(socket), std::move(transport_socket), *stream_info);
std::move(socket), std::move(transport_socket), *stream_info,
parent_.overload_manager_.getThreadLocalOverloadState());
if (const auto timeout = filter_chain->transportSocketConnectTimeout();
timeout != std::chrono::milliseconds::zero()) {
server_conn_ptr->setTransportSocketConnectTimeout(timeout);
Expand Down
5 changes: 4 additions & 1 deletion source/server/connection_handler_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "envoy/network/listener.h"
#include "envoy/server/active_udp_listener_config.h"
#include "envoy/server/listener_manager.h"
#include "envoy/server/overload/overload_manager.h"
#include "envoy/stats/scope.h"
#include "envoy/stats/timespan.h"

Expand Down Expand Up @@ -66,7 +67,8 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler,
NonCopyable,
Logger::Loggable<Logger::Id::conn_handler> {
public:
ConnectionHandlerImpl(Event::Dispatcher& dispatcher, absl::optional<uint32_t> worker_index);
ConnectionHandlerImpl(Event::Dispatcher& dispatcher, OverloadManager& overload_manager,
absl::optional<uint32_t> worker_index);

// Network::ConnectionHandler
uint64_t numConnections() const override { return num_handler_connections_; }
Expand Down Expand Up @@ -359,6 +361,7 @@ class ConnectionHandlerImpl : public Network::ConnectionHandler,
// This has a value on worker threads, and no value on the main thread.
const absl::optional<uint32_t> worker_index_;
Event::Dispatcher& dispatcher_;
Server::OverloadManager& overload_manager_;
const std::string per_handler_stat_prefix_;
std::list<std::pair<Network::Address::InstanceConstSharedPtr, ActiveListenerDetails>> listeners_;
std::atomic<uint64_t> num_handler_connections_{};
Expand Down
2 changes: 2 additions & 0 deletions source/server/overload_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ OverloadTimerType parseTimerType(
switch (config_timer_type) {
case Config::HTTP_DOWNSTREAM_CONNECTION_IDLE:
return OverloadTimerType::HttpDownstreamIdleConnectionTimeout;
case Config::TRANSPORT_SOCKET_CONNECT:
return OverloadTimerType::TransportSocketConnectTimeout;
default:
throw EnvoyException(fmt::format("Unknown timer type {}", config_timer_type));
}
Expand Down
5 changes: 4 additions & 1 deletion source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "server/connection_handler_impl.h"
#include "server/guarddog_impl.h"
#include "server/listener_hooks.h"
#include "server/overload_manager_impl.h"
#include "server/ssl_context_manager.h"

namespace Envoy {
Expand All @@ -77,7 +78,6 @@ InstanceImpl::InstanceImpl(
: absl::nullopt)),
dispatcher_(api_->allocateDispatcher("main_thread")),
singleton_manager_(new Singleton::ManagerImpl(api_->threadFactory())),
handler_(new ConnectionHandlerImpl(*dispatcher_, absl::nullopt)),
listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks),
access_log_manager_(options.fileFlushIntervalMsec(), *api_, *dispatcher_, access_log_lock,
store),
Expand Down Expand Up @@ -462,6 +462,9 @@ void InstanceImpl::initialize(const Options& options,
// We can now initialize stats for threading.
stats_store_.initializeThreading(*dispatcher_, thread_local_);

handler_ =
ggreenway marked this conversation as resolved.
Show resolved Hide resolved
std::make_unique<ConnectionHandlerImpl>(*dispatcher_, *overload_manager_, absl::nullopt);

// It's now safe to start writing stats from the main thread's dispatcher.
if (bootstrap_.enable_dispatcher_stats()) {
dispatcher_->initializeStats(stats_store_, "server.");
Expand Down
7 changes: 4 additions & 3 deletions source/server/worker_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ namespace Server {
WorkerPtr ProdWorkerFactory::createWorker(uint32_t index, OverloadManager& overload_manager,
const std::string& worker_name) {
Event::DispatcherPtr dispatcher(api_.allocateDispatcher(worker_name));
return std::make_unique<WorkerImpl>(tls_, hooks_, std::move(dispatcher),
std::make_unique<ConnectionHandlerImpl>(*dispatcher, index),
overload_manager, api_);
return std::make_unique<WorkerImpl>(
tls_, hooks_, std::move(dispatcher),
std::make_unique<ConnectionHandlerImpl>(*dispatcher, overload_manager, index),
overload_manager, api_);
}

WorkerImpl::WorkerImpl(ThreadLocal::Instance& tls, ListenerHooks& hooks,
Expand Down
1 change: 1 addition & 0 deletions test/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ envoy_cc_test(
"//test/mocks/event:event_mocks",
"//test/mocks/http:http_mocks",
"//test/mocks/network:network_mocks",
"//test/mocks/server:overload_manager_mocks",
"//test/mocks/ssl:ssl_mocks",
"//test/mocks/upstream:cluster_info_mocks",
"//test/test_common:environment_lib",
Expand Down
5 changes: 4 additions & 1 deletion test/common/http/codec_client_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "test/mocks/event/mocks.h"
#include "test/mocks/http/mocks.h"
#include "test/mocks/network/mocks.h"
#include "test/mocks/server/overload_manager.h"
#include "test/mocks/ssl/mocks.h"
#include "test/mocks/upstream/cluster_info.h"
#include "test/test_common/environment.h"
Expand Down Expand Up @@ -301,7 +302,8 @@ class CodecNetworkTest : public testing::TestWithParam<Network::Address::IpVersi
EXPECT_CALL(listener_callbacks_, onAccept_(_))
.WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket) -> void {
upstream_connection_ = dispatcher_->createServerConnection(
std::move(socket), Network::Test::createRawBufferSocket(), stream_info_);
std::move(socket), Network::Test::createRawBufferSocket(), stream_info_,
overload_state_);
upstream_connection_->addConnectionCallbacks(upstream_callbacks_);

expected_callbacks--;
Expand Down Expand Up @@ -347,6 +349,7 @@ class CodecNetworkTest : public testing::TestWithParam<Network::Address::IpVersi
protected:
Api::ApiPtr api_;
Event::DispatcherPtr dispatcher_;
Server::MockThreadLocalOverloadState overload_state_;
Network::ListenerPtr upstream_listener_;
Network::MockTcpListenerCallbacks listener_callbacks_;
Network::MockConnectionHandler connection_handler_;
Expand Down
Loading