Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
aaaecea
Checkout PerfAnnotation class (not ready for review).
jmarantz Feb 13, 2018
cacf76b
checkpoint with envoy_init_speed_test building.
jmarantz Feb 14, 2018
b21eabd
Checkpoint bazel option enabled to enable perf annotation.
jmarantz Feb 14, 2018
2263657
Checkpoint with hacky --disable_hot_restart flag.
jmarantz Feb 14, 2018
845c81c
All basic functionality implemented, including one more minor refacto…
jmarantz Feb 14, 2018
65f58c8
cleanup a little
jmarantz Feb 14, 2018
5a047b2
format fix-up.
jmarantz Feb 14, 2018
3750841
test-coverage for dump()
jmarantz Feb 14, 2018
b9366b9
Rather than making a new binary, add a new server mode.
jmarantz Feb 15, 2018
7a8b42a
Merge branch 'master' into perf-annotation
jmarantz Feb 15, 2018
479fa81
Remove last remnants of the envoy_init_speed_test. formatting.
jmarantz Feb 15, 2018
72d18aa
thread: annotation support when !OS X.
htuch Feb 14, 2018
32fa5f4
Fix hot restart test under --runs_per_test
dnoe Feb 14, 2018
46f43a4
Fix a few more cases where --base-id was required.
dnoe Feb 14, 2018
2db4b24
Add perf_annotation_lib.
jmarantz Feb 15, 2018
d440e99
formatting
jmarantz Feb 15, 2018
d69399b
Merge branch 'master' into perf-annotation-lib
jmarantz Feb 15, 2018
89fa9dc
Merge branch 'perf-annotation-lib' into perf-annotation
jmarantz Feb 15, 2018
c85f07e
Use the cleaned up version of perf_annotation from #2626
jmarantz Feb 15, 2018
759418c
fix crash and tweak output format
jmarantz Feb 16, 2018
869b490
fix test for left-justfied text column.
jmarantz Feb 16, 2018
f219f2b
Merge branch 'master' into perf-annotation
jmarantz Feb 28, 2018
99001c1
Resolve some remaining conflicts from the merge.
jmarantz Feb 28, 2018
2c7ab78
Tweak perf annotation format slightly as left-justifying text-fields …
jmarantz Feb 28, 2018
15c0386
Merge branch 'master' into perf-annotation
jmarantz Feb 28, 2018
8284adf
Remove more merge-conflict detritus, which was in a comment.
jmarantz Feb 28, 2018
9d6f4c4
Merge branch 'master' into perf-annotation
jmarantz Mar 1, 2018
0acf719
Use absl::string_view for a temp string constant rather than const ch…
jmarantz Mar 2, 2018
c2fc345
Merge branch 'master' into perf-annotation
jmarantz Mar 3, 2018
a242ace
Add test for '--mode init_only'.
jmarantz Mar 3, 2018
7b06b49
Clean up comments about why the random-number generator is still needed.
jmarantz Mar 3, 2018
090ae2e
Add explicit test for --mode init_only (in addition to using it to ma…
jmarantz Mar 3, 2018
479aebf
Correct a stale comment.
jmarantz Mar 3, 2018
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
7 changes: 6 additions & 1 deletion include/envoy/server/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,12 @@ enum class Mode {
*/
Validate,

// TODO(rlazarus): Add a third option for "light validation": Mock out access to the filesystem.
/**
* Completely load and initialize the config, and then exit without running the listener loop.
*/
InitOnly,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a test for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah no, but if #2679 can get merged it'll be way easier to add one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added one....i resurrected the random component to the main_common_test base ID calculation as it seems you can't immediately open one of the required resources immediate after closing it in the previous test method.


// TODO(rlazarus): Add a fourth option for "light validation": Mock out access to the filesystem.
// Perform no validation of files referenced in the config, such as runtime configs, SSL certs,
// etc. Validation will pass even if those files are malformed or don't exist, allowing the config
// to be validated in a non-prod environment.
Expand Down
4 changes: 3 additions & 1 deletion source/common/common/perf_annotation.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ std::string PerfAnnotationContext::toString() {
// Create format-strings to right justify each column, e.g. {:>14} for a column of width 14.
std::vector<std::string> formats;
for (size_t i = 0; i < num_columns; ++i) {
formats.push_back(absl::StrCat("{:>", widths[i], "}"));
// left-justify category & description, but right-justify the numeric columns.
const absl::string_view justify = (i < num_columns - 2) ? ">" : "<";
formats.push_back(absl::StrCat("{:", justify, widths[i], "}"));
}

// Write out the table.
Expand Down
1 change: 1 addition & 0 deletions source/common/stats/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ envoy_cc_library(
"//include/envoy/stats:stats_interface",
"//source/common/common:assert_lib",
"//source/common/common:hash_lib",
"//source/common/common:perf_annotation_lib",
"//source/common/common:utility_lib",
"//source/common/config:well_known_names",
"//source/common/protobuf",
Expand Down
4 changes: 4 additions & 0 deletions source/common/stats/stats_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "envoy/common/exception.h"

#include "common/common/perf_annotation.h"
#include "common/common/utility.h"
#include "common/config/well_known_names.h"

Expand Down Expand Up @@ -108,6 +109,7 @@ TagExtractorPtr TagExtractorImpl::createTagExtractor(const std::string& name,
bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>& tags,
IntervalSet<size_t>& remove_characters) const {

PERF_OPERATION(perf);
std::smatch match;
// The regex must match and contain one or more subexpressions (all after the first are ignored).
if (std::regex_search(stat_name, match, regex_) && match.size() > 1) {
Expand All @@ -129,8 +131,10 @@ bool TagExtractorImpl::extractTag(const std::string& stat_name, std::vector<Tag>
std::string::size_type start = remove_subexpr.first - stat_name.begin();
std::string::size_type end = remove_subexpr.second - stat_name.begin();
remove_characters.insert(start, end);
PERF_RECORD(perf, "re-match", name_);
return true;
}
PERF_RECORD(perf, "re-miss", name_);
return false;
}

Expand Down
1 change: 1 addition & 0 deletions source/exe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ envoy_cc_library(
":envoy_common_lib",
"//source/common/api:os_sys_calls_lib",
"//source/common/common:compiler_requirements_lib",
"//source/common/common:perf_annotation_lib",
"//source/server:hot_restart_lib",
"//source/server:hot_restart_nop_lib",
"//source/server:proto_descriptors_lib",
Expand Down
5 changes: 5 additions & 0 deletions source/exe/main_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <memory>

#include "common/common/compiler_requirements.h"
#include "common/common/perf_annotation.h"
#include "common/event/libevent.h"
#include "common/network/utility.h"
#include "common/stats/stats_impl.h"
Expand Down Expand Up @@ -43,6 +44,7 @@ MainCommonBase::MainCommonBase(OptionsImpl& options) : options_(options) {
RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors());

switch (options_.mode()) {
case Server::Mode::InitOnly:
case Server::Mode::Serve: {
#ifdef ENVOY_HOT_RESTART
if (!options.hotRestartDisabled()) {
Expand Down Expand Up @@ -84,6 +86,9 @@ bool MainCommonBase::run() {
auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion());
return Server::validateConfig(options_, local_address, component_factory_);
}
case Server::Mode::InitOnly:
PERF_DUMP();
return true;
}
NOT_REACHED;
}
Expand Down
2 changes: 2 additions & 0 deletions source/server/options_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ OptionsImpl::OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_r
mode_ = Server::Mode::Serve;
} else if (mode.getValue() == "validate") {
mode_ = Server::Mode::Validate;
} else if (mode.getValue() == "init_only") {
mode_ = Server::Mode::InitOnly;
} else {
const std::string message = fmt::format("error: unknown mode '{}'", mode.getValue());
std::cerr << message << std::endl;
Expand Down
14 changes: 7 additions & 7 deletions test/common/common/perf_annotation_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ TEST_F(PerfAnnotationTest, testMacros) {
PERF_RECORD(perf, "beta", "3");
std::string report = PERF_TO_STRING();
EXPECT_TRUE(report.find(" alpha ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 0\n") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 0 ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" beta ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 1\n") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 1 ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" alpha ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 2\n") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 2 ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" beta ") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 3\n") != std::string::npos) << report;
EXPECT_TRUE(report.find(" 3 ") != std::string::npos) << report;
PERF_DUMP();
}

Expand All @@ -51,9 +51,9 @@ TEST_F(PerfAnnotationTest, testFormat) {
std::string report = context->toString();
EXPECT_EQ(
"Duration(us) # Calls Mean(ns) StdDev(ns) Min(ns) Max(ns) Category Description\n"
" 4600 4 1150000 129099 1000000 1300000 alpha 1\n"
" 200 1 200000 nan 200000 200000 gamma 2\n"
" 87 3 29000 1000 28000 30000 beta 3\n",
" 4600 4 1150000 129099 1000000 1300000 alpha 1 \n"
" 200 1 200000 nan 200000 200000 gamma 2 \n"
" 87 3 29000 1000 28000 30000 beta 3 \n",
context->toString());
}

Expand Down
54 changes: 41 additions & 13 deletions test/exe/main_common_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include <unistd.h>

#include "common/runtime/runtime_impl.h"

#include "exe/main_common.h"

#include "server/options_impl.h"
Expand Down Expand Up @@ -38,17 +40,33 @@ class MainCommonTest : public testing::Test {
MainCommonTest()
: config_file_(Envoy::TestEnvironment::getCheckedEnvVar("TEST_RUNDIR") +
"/test/config/integration/google_com_proxy_port_0.v2.yaml"),
random_string_(fmt::format("{}", static_cast<uint32_t>(getpid()))),
argv_({"envoy-static", "--base-id", random_string_.c_str(), nullptr}) {}
random_string_(fmt::format("{}", computeBaseId())),
argv_({"envoy-static", "--base-id", random_string_.c_str(), "-c", config_file_.c_str(),
nullptr}) {}

/**
* Computes a numeric ID to incorporate into the names of
* shared-memory segments and domain sockets, to help keep them
* distinct from other tests that might be running concurrently.
*
* The PID is needed to isolate namespaces between concurrent
* processes in CI. The random number generator is needed
* sequentially executed test methods fail with an error in
* bindDomainSocket if the the same base-id is re-used.
*
* @return uint32_t a unique numeric ID based on the PID and a random number.
*/
static uint32_t computeBaseId() {
Runtime::RandomGeneratorImpl random_generator_;
// Pick a prime number to give more of the 32-bits of entropy to the PID, and the
// remainder to the random number.
const uint32_t four_digit_prime = 7919;
return getpid() * four_digit_prime + random_generator_.random() % four_digit_prime;
}

char** argv() { return const_cast<char**>(&argv_[0]); }
int argc() { return argv_.size() - 1; }

void addConfig() {
addArg("-c");
addArg(config_file_.c_str());
}

// Adds an argument, assuring that argv remains null-terminated.
void addArg(const char* arg) {
ASSERT(!argv_.empty());
Expand All @@ -68,7 +86,6 @@ TEST_F(MainCommonTest, ConstructDestructHotRestartEnabled) {
if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) {
return;
}
addConfig();
VERBOSE_EXPECT_NO_THROW(MainCommon main_common(argc(), argv()));
}

Expand All @@ -77,11 +94,22 @@ TEST_F(MainCommonTest, ConstructDestructHotRestartDisabled) {
if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) {
return;
}
addConfig();
addArg("--disable-hot-restart");
VERBOSE_EXPECT_NO_THROW(MainCommon main_common(argc(), argv()));
}

// Exercise init_only explicitly.
TEST_F(MainCommonTest, ConstructDestructHotRestartDisabledNoInit) {
if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) {
return;
}
addArg("--disable-hot-restart");
addArg("--mode");
addArg("init_only");
MainCommon main_common(argc(), argv());
EXPECT_TRUE(main_common.run());
}

// Ensurees that existing users of main_common() can link.
TEST_F(MainCommonTest, LegacyMain) {
if (!Envoy::TestEnvironment::shouldRunTestForIpVersion(Network::Address::IpVersion::v4)) {
Expand All @@ -96,6 +124,9 @@ TEST_F(MainCommonTest, LegacyMain) {
std::unique_ptr<Envoy::OptionsImpl> options;
int return_code = -1;
try {
// Set the mode to init-only so main_common doesn't initiate a server-loop.
addArg("--mode");
addArg("init_only");
options = std::make_unique<Envoy::OptionsImpl>(argc(), argv(), &MainCommon::hotRestartVersion,
spdlog::level::info);
} catch (const Envoy::NoServingException& e) {
Expand All @@ -104,12 +135,9 @@ TEST_F(MainCommonTest, LegacyMain) {
return_code = EXIT_FAILURE;
}
if (return_code == -1) {
// Note that Envoy::main_common() will run an event loop if properly configured, which
// would hang the test. This is why we don't supply a config file in this testcase;
// we just want to make sure we wake up this code in a test.
return_code = Envoy::main_common(*options);
}
EXPECT_EQ(EXIT_FAILURE, return_code);
EXPECT_EQ(EXIT_SUCCESS, return_code);
}

} // namespace Envoy
Expand Down