Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions api/envoy/admin/v2alpha/server_info.proto
Original file line number Diff line number Diff line change
Expand Up @@ -128,4 +128,7 @@ message CommandLineOptions {

// See :option:`--restart-epoch` for details.
uint32 restart_epoch = 24;

// See :option:`--cpuset-threads` for details.
bool cpuset_threads = 25;
}
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Version history
* config: removed deprecated --v2-config-only from command line config.
* config: removed deprecated_v1 sds_config from :ref:`Bootstrap config <config_overview_v2_bootstrap>`.
* config: removed REST_LEGACY as a valid :ref:`ApiType <envoy_api_field_core.ApiConfigSource.api_type>`.
* config: use Envoy cpuset size to set the default number or worker threads if :option:`--cpuset-threads` is enabled.
* cors: added :ref:`filter_enabled & shadow_enabled RuntimeFractionalPercent flags <cors-runtime>` to filter.
* ext_authz: added an configurable option to make the gRPC service cross-compatible with V2Alpha. Note that this feature is already deprecated. It should be used for a short time, and only when transitioning from alpha to V2 release version.
* ext_authz: migrated from V2alpha to V2 and improved the documentation.
Expand Down
3 changes: 2 additions & 1 deletion docs/root/operations/admin.rst
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ modify different aspects of the server:
"restart_epoch": 0,
"file_flush_interval": "10s",
"drain_time": "600s",
"parent_shutdown_time": "900s"
"parent_shutdown_time": "900s",
"cpuset_threads": false
},
"uptime_current_epoch": "6s",
"uptime_all_epochs": "6s"
Expand Down
8 changes: 8 additions & 0 deletions docs/root/operations/cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ following are the command line options that Envoy supports.
`connection` component to run at `trace` level, you should pass ``upstream:debug,connection:trace`` to
this flag. See ``ALL_LOGGER_IDS`` in :repo:`/source/common/common/logger.h` for a list of components.

.. option:: --cpuset-threads

*(optional)* This flag is used to control the number of worker threads if :option:`--concurrency` is
not set. If enabled, the assigned cpuset size is used to determine the number of worker threads on
Linux-based systems. Otherwise the number of worker threads is set to the number of hardware threads
on the machine. You can read more about cpusets in the
`kernel documentation <https://www.kernel.org/doc/Documentation/cgroup-v1/cpusets.txt>`_.

.. option:: --log-path <path string>

*(optional)* The output file path where logs should be written. This file will be re-opened
Expand Down
6 changes: 5 additions & 1 deletion include/envoy/api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,9 @@ envoy_cc_library(

envoy_cc_library(
name = "os_sys_calls_interface",
hdrs = ["os_sys_calls.h"],
hdrs = [
"os_sys_calls.h",
"os_sys_calls_common.h",
"os_sys_calls_linux.h",
],
)
23 changes: 1 addition & 22 deletions include/envoy/api/os_sys_calls.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,12 @@
#include <memory>
#include <string>

#include "envoy/api/os_sys_calls_common.h"
#include "envoy/common/pure.h"

namespace Envoy {
namespace Api {

/**
* SysCallResult holds the rc and errno values resulting from a system call.
*/
template <typename T> struct SysCallResult {

/**
* The return code from the system call.
*/
T rc_;

/**
* The errno value as captured after the system call.
*/
int errno_;
};

typedef SysCallResult<int> SysCallIntResult;
typedef SysCallResult<ssize_t> SysCallSizeResult;
typedef SysCallResult<void*> SysCallPtrResult;
typedef SysCallResult<std::string> SysCallStringResult;
typedef SysCallResult<bool> SysCallBoolResult;

class OsSysCalls {
public:
virtual ~OsSysCalls() {}
Expand Down
31 changes: 31 additions & 0 deletions include/envoy/api/os_sys_calls_common.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#pragma once

#include <memory>
#include <string>

namespace Envoy {
namespace Api {
/**
* SysCallResult holds the rc and errno values resulting from a system call.
*/
template <typename T> struct SysCallResult {

/**
* The return code from the system call.
*/
T rc_;

/**
* The errno value as captured after the system call.
*/
int errno_;
};

typedef SysCallResult<int> SysCallIntResult;
typedef SysCallResult<ssize_t> SysCallSizeResult;
typedef SysCallResult<void*> SysCallPtrResult;
typedef SysCallResult<std::string> SysCallStringResult;
typedef SysCallResult<bool> SysCallBoolResult;

} // namespace Api
} // namespace Envoy
28 changes: 28 additions & 0 deletions include/envoy/api/os_sys_calls_linux.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#pragma once
Copy link
Member

Choose a reason for hiding this comment

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

Can you add something like:

#ifndef __LINUX__
#error "Linux platform included on non-Linux"
#endif 

(or something like that). Same for the impl.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, done.


#if !defined(__linux__)
#error "Linux platform file is part of non-Linux build."
#endif

#include <sched.h>

#include "envoy/api/os_sys_calls_common.h"
#include "envoy/common/pure.h"

namespace Envoy {
namespace Api {

class LinuxOsSysCalls {
public:
virtual ~LinuxOsSysCalls() {}

/**
* @see sched_getaffinity (man 2 sched_getaffinity)
*/
virtual SysCallIntResult sched_getaffinity(pid_t pid, size_t cpusetsize, cpu_set_t* mask) PURE;
};

typedef std::unique_ptr<LinuxOsSysCalls> LinuxOsSysCallsPtr;

} // namespace Api
} // namespace Envoy
5 changes: 5 additions & 0 deletions include/envoy/server/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ class Options {
*/
virtual bool mutexTracingEnabled() const PURE;

/**
* @return bool indicating whether cpuset size should determine the number of worker threads.
*/
virtual bool cpusetThreadsEnabled() const PURE;

/**
* Converts the Options in to CommandLineOptions proto message defined in server_info.proto.
* @return CommandLineOptionsPtr the protobuf representation of the options.
Expand Down
12 changes: 10 additions & 2 deletions source/common/api/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,16 @@ envoy_cc_library(

envoy_cc_library(
name = "os_sys_calls_lib",
srcs = ["os_sys_calls_impl.cc"],
hdrs = ["os_sys_calls_impl.h"],
srcs = ["os_sys_calls_impl.cc"] + select({
"@bazel_tools//src/conditions:linux_x86_64": ["os_sys_calls_impl_linux.cc"],
Copy link
Member

Choose a reason for hiding this comment

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

FTR, this breaks the Google import since we don't have @bazel_tools conditional in this form internally. I'd prefer to stick with //bazel:BUILD conditionals, I will do an update PR for this.

"@bazel_tools//src/conditions:linux_aarch64": ["os_sys_calls_impl_linux.cc"],
"//conditions:default": [],
}),
hdrs = ["os_sys_calls_impl.h"] + select({
"@bazel_tools//src/conditions:linux_x86_64": ["os_sys_calls_impl_linux.h"],
"@bazel_tools//src/conditions:linux_aarch64": ["os_sys_calls_impl_linux.h"],
"//conditions:default": [],
}),
deps = [
"//include/envoy/api:os_sys_calls_interface",
"//source/common/singleton:threadsafe_singleton",
Expand Down
20 changes: 20 additions & 0 deletions source/common/api/os_sys_calls_impl_linux.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#if !defined(__linux__)
#error "Linux platform file is part of non-Linux build."
#endif

#include "common/api/os_sys_calls_impl_linux.h"

#include <errno.h>
#include <sched.h>

namespace Envoy {
namespace Api {

SysCallIntResult LinuxOsSysCallsImpl::sched_getaffinity(pid_t pid, size_t cpusetsize,
cpu_set_t* mask) {
const int rc = ::sched_getaffinity(pid, cpusetsize, mask);
return {rc, errno};
}

} // namespace Api
} // namespace Envoy
23 changes: 23 additions & 0 deletions source/common/api/os_sys_calls_impl_linux.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#pragma once

#if !defined(__linux__)
#error "Linux platform file is part of non-Linux build."
#endif

#include "envoy/api/os_sys_calls_linux.h"

#include "common/singleton/threadsafe_singleton.h"

namespace Envoy {
namespace Api {

class LinuxOsSysCallsImpl : public LinuxOsSysCalls {
public:
// Api::LinuxOsSysCalls
SysCallIntResult sched_getaffinity(pid_t pid, size_t cpusetsize, cpu_set_t* mask) override;
};

typedef ThreadSafeSingleton<LinuxOsSysCallsImpl> LinuxOsSysCallsSingleton;

} // namespace Api
} // namespace Envoy
17 changes: 15 additions & 2 deletions source/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,26 @@ envoy_cc_library(

envoy_cc_library(
name = "options_lib",
srcs = ["options_impl.cc"],
hdrs = ["options_impl.h"],
srcs = ["options_impl.cc"] + select({
"@bazel_tools//src/conditions:linux_x86_64": ["options_impl_platform_linux.cc"],
"@bazel_tools//src/conditions:linux_aarch64": ["options_impl_platform_linux.cc"],
"//conditions:default": ["options_impl_platform_default.cc"],
}),
hdrs = [
"options_impl.h",
"options_impl_platform.h",
] + select({
"@bazel_tools//src/conditions:linux_x86_64": ["options_impl_platform_linux.h"],
"@bazel_tools//src/conditions:linux_aarch64": ["options_impl_platform_linux.h"],
"//conditions:default": [],
}),
external_deps = ["tclap"],
deps = [
"//include/envoy/network:address_interface",
"//include/envoy/server:options_interface",
"//include/envoy/stats:stats_interface",
"//source/common/api:os_sys_calls_lib",
"//source/common/common:logger_lib",
"//source/common/common:macros",
"//source/common/common:version_lib",
"//source/common/protobuf:utility_lib",
Expand Down
24 changes: 22 additions & 2 deletions source/server/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#include "common/common/version.h"
#include "common/protobuf/utility.h"

#include "server/options_impl_platform.h"

#include "absl/strings/str_split.h"
#include "spdlog/spdlog.h"
#include "tclap/CmdLine.h"
Expand Down Expand Up @@ -117,6 +119,8 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,
"Disable hot restart functionality", cmd, false);
TCLAP::SwitchArg enable_mutex_tracing(
"", "enable-mutex-tracing", "Enable mutex contention tracing functionality", cmd, false);
TCLAP::SwitchArg cpuset_threads(
"", "cpuset-threads", "Get the default # of worker threads from cpuset size", cmd, false);

cmd.setExceptionHandling(false);
try {
Expand Down Expand Up @@ -154,6 +158,8 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,

mutex_tracing_enabled_ = enable_mutex_tracing.getValue();

cpuset_threads_ = cpuset_threads.getValue();

log_level_ = default_log_level;
for (size_t i = 0; i < ARRAY_SIZE(spdlog::level::level_string_views); i++) {
if (log_level.getValue() == spdlog::level::level_string_views[i]) {
Expand Down Expand Up @@ -188,7 +194,20 @@ OptionsImpl::OptionsImpl(int argc, const char* const* argv,

// For base ID, scale what the user inputs by 10 so that we have spread for domain sockets.
base_id_ = base_id.getValue() * 10;
concurrency_ = std::max(1U, concurrency.getValue());

if (!concurrency.isSet() && cpuset_threads_) {
// The 'concurrency' command line option wasn't set but the 'cpuset-threads'
Copy link
Member

Choose a reason for hiding this comment

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

Should we warn if someone sets cpuset-threads but also sets concurrency, or just not allow it?

Copy link
Member Author

Choose a reason for hiding this comment

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

A warning might be enough? I'll have a candidate patch soon.

// option was set. Use the number of CPUs assigned to the process cpuset, if
// that can be known.
concurrency_ = OptionsImplPlatform::getCpuCount();
} else {
if (concurrency.isSet() && cpuset_threads_ && cpuset_threads.isSet()) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a test for this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a test.

ENVOY_LOG(warn, "Both --concurrency and --cpuset-threads options are set; not applying "
"--cpuset-threads.");
}
concurrency_ = std::max(1U, concurrency.getValue());
}

config_path_ = config_path.getValue();
config_yaml_ = config_yaml.getValue();
allow_unknown_fields_ = allow_unknown_fields.getValue();
Expand Down Expand Up @@ -291,6 +310,7 @@ Server::CommandLineOptionsPtr OptionsImpl::toCommandLineOptions() const {
command_line_options->set_max_obj_name_len(statsOptions().maxObjNameLength());
command_line_options->set_disable_hot_restart(hotRestartDisabled());
command_line_options->set_enable_mutex_tracing(mutexTracingEnabled());
command_line_options->set_cpuset_threads(cpusetThreadsEnabled());
command_line_options->set_restart_epoch(restartEpoch());
return command_line_options;
}
Expand All @@ -303,6 +323,6 @@ OptionsImpl::OptionsImpl(const std::string& service_cluster, const std::string&
service_cluster_(service_cluster), service_node_(service_node), service_zone_(service_zone),
file_flush_interval_msec_(10000), drain_time_(600), parent_shutdown_time_(900),
mode_(Server::Mode::Serve), max_stats_(ENVOY_DEFAULT_MAX_STATS), hot_restart_disabled_(false),
signal_handling_enabled_(true), mutex_tracing_enabled_(false) {}
signal_handling_enabled_(true), mutex_tracing_enabled_(false), cpuset_threads_(false) {}

} // namespace Envoy
6 changes: 5 additions & 1 deletion source/server/options_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "envoy/server/options.h"
#include "envoy/stats/stats_options.h"

#include "common/common/logger.h"
#include "common/stats/stats_options_impl.h"

#include "spdlog/spdlog.h"
Expand All @@ -16,7 +17,7 @@ namespace Envoy {
/**
* Implementation of Server::Options.
*/
class OptionsImpl : public Server::Options {
class OptionsImpl : public Server::Options, protected Logger::Loggable<Logger::Id::config> {
public:
/**
* Parameters are max_num_stats, max_stat_name_len, hot_restart_enabled
Expand Down Expand Up @@ -73,6 +74,7 @@ class OptionsImpl : public Server::Options {
void setSignalHandling(bool signal_handling_enabled) {
signal_handling_enabled_ = signal_handling_enabled;
}
void setCpusetThreads(bool cpuset_threads_enabled) { cpuset_threads_ = cpuset_threads_enabled; }

// Server::Options
uint64_t baseId() const override { return base_id_; }
Expand Down Expand Up @@ -107,6 +109,7 @@ class OptionsImpl : public Server::Options {
bool mutexTracingEnabled() const override { return mutex_tracing_enabled_; }
virtual Server::CommandLineOptionsPtr toCommandLineOptions() const override;
void parseComponentLogLevels(const std::string& component_log_levels);
bool cpusetThreadsEnabled() const override { return cpuset_threads_; }
uint32_t count() const;

private:
Expand Down Expand Up @@ -137,6 +140,7 @@ class OptionsImpl : public Server::Options {
bool hot_restart_disabled_;
bool signal_handling_enabled_;
bool mutex_tracing_enabled_;
bool cpuset_threads_;
uint32_t count_;
};

Expand Down
12 changes: 12 additions & 0 deletions source/server/options_impl_platform.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#pragma once

#include <cstdint>

#include "common/common/logger.h"

namespace Envoy {
class OptionsImplPlatform : protected Logger::Loggable<Logger::Id::config> {
public:
static uint32_t getCpuCount();
};
} // namespace Envoy
14 changes: 14 additions & 0 deletions source/server/options_impl_platform_default.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#include <thread>

#include "common/common/logger.h"

#include "server/options_impl_platform.h"

namespace Envoy {

uint32_t OptionsImplPlatform::getCpuCount() {
ENVOY_LOG(warn, "CPU number provided by HW thread count (instead of cpuset).");
return std::thread::hardware_concurrency();
}

} // namespace Envoy
Loading