From f3b89b0f2513c9538915e0b96c0f7f5cb4cb4bdc Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 24 Nov 2020 20:00:07 +0000 Subject: [PATCH 01/15] Create CDS speed test, mostly copied from EDS speed test. Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 36 +++++ test/common/upstream/cds_speed_test.cc | 196 +++++++++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 test/common/upstream/cds_speed_test.cc diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 65eeb64c10708..30dc3d5707831 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -146,6 +146,42 @@ envoy_benchmark_test( benchmark_binary = "eds_speed_test", ) +envoy_cc_benchmark_binary( + name = "cds_speed_test", + srcs = ["cds_speed_test.cc"], + external_deps = [ + "benchmark", + ], + deps = [ + ":utility_lib", + "//source/common/config:grpc_mux_lib", + "//source/common/config:grpc_subscription_lib", + "//source/common/config:protobuf_link_hacks", + "//source/common/config:utility_lib", + "//source/common/upstream:eds_lib", + "//source/extensions/transport_sockets/raw_buffer:config", + "//source/server:transport_socket_config_lib", + "//test/mocks/local_info:local_info_mocks", + "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/runtime:runtime_mocks", + "//test/mocks/server:admin_mocks", + "//test/mocks/server:instance_mocks", + "//test/mocks/ssl:ssl_mocks", + "//test/mocks/upstream:cluster_manager_mocks", + "//test/test_common:test_runtime_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", + ], +) + +envoy_benchmark_test( + name = "cds_speed_test_benchmark_test", + benchmark_binary = "cds_speed_test", +) + envoy_cc_test_library( name = "health_check_fuzz_utils_lib", srcs = [ diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc new file mode 100644 index 0000000000000..afa2c6423a4b3 --- /dev/null +++ b/test/common/upstream/cds_speed_test.cc @@ -0,0 +1,196 @@ +// Note: this should be run with --compilation_mode=opt, and would benefit from a +// quiescent system with disabled cstate power management. + +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/core/v3/health_check.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/service/discovery/v3/discovery.pb.h" +#include "envoy/stats/scope.h" + +#include "common/config/grpc_mux_impl.h" +#include "common/config/grpc_subscription_impl.h" +#include "common/config/utility.h" +#include "common/singleton/manager_impl.h" +#include "common/upstream/eds.h" + +#include "server/transport_socket_config_impl.h" + +#include "test/benchmark/main.h" +#include "test/common/upstream/utility.h" +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/protobuf/mocks.h" +#include "test/mocks/runtime/mocks.h" +#include "test/mocks/server/admin.h" +#include "test/mocks/server/instance.h" +#include "test/mocks/ssl/mocks.h" +#include "test/mocks/upstream/cluster_manager.h" +#include "test/test_common/test_runtime.h" +#include "test/test_common/utility.h" + +#include "benchmark/benchmark.h" + +using ::benchmark::State; +using Envoy::benchmark::skipExpensiveBenchmarks; + +namespace Envoy { +namespace Upstream { + +class CdsSpeedTest { +public: + CdsSpeedTest(State& state, bool v2_config) + : state_(state), v2_config_(v2_config), + type_url_(v2_config_ + ? "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment" + : "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"), + subscription_stats_(Config::Utility::generateStats(stats_)), + api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()), + grpc_mux_(new Config::GrpcMuxImpl( + local_info_, std::unique_ptr(async_client_), dispatcher_, + *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( + "envoy.service.endpoint.v3.EndpointDiscoveryService.StreamEndpoints"), + envoy::config::core::v3::ApiVersion::AUTO, random_, stats_, {}, true)) { + + resetCluster(R"EOF( + name: name + connect_timeout: 0.25s + type: EDS + eds_cluster_config: + service_name: fare + eds_config: + api_config_source: + cluster_names: + - eds + refresh_delay: 1s + )EOF", + Envoy::Upstream::Cluster::InitializePhase::Secondary); + + EXPECT_CALL(*cm_.subscription_factory_.subscription_, start(_, _)); + cluster_->initialize([this] { initialized_ = true; }); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(testing::Return(&async_stream_)); + subscription_->start({"fare"}); + } + + void resetCluster(const std::string& yaml_config, Cluster::InitializePhase initialize_phase) { + local_info_.node_.mutable_locality()->set_zone("us-east-1a"); + eds_cluster_ = parseClusterFromV3Yaml(yaml_config); + Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( + "cluster.{}.", + eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name())); + Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( + admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, stats_, + singleton_manager_, tls_, validation_visitor_, *api_); + cluster_ = std::make_shared(eds_cluster_, runtime_, factory_context, + std::move(scope), false); + EXPECT_EQ(initialize_phase, cluster_->initializePhase()); + eds_callbacks_ = cm_.subscription_factory_.callbacks_; + subscription_ = std::make_unique( + grpc_mux_, *eds_callbacks_, resource_decoder_, subscription_stats_, type_url_, dispatcher_, + std::chrono::milliseconds(), false); + } + + // Set up a bunch of clusters that vary only in name, and add them to the configuration. + void clusterHelper(bool ignore_unknown_dynamic_fields, size_t num_clusters) { + state_.PauseTiming(); + + auto response = std::make_unique(); + response->set_type_url(type_url_); + response->set_version_info(fmt::format("version-{}", version_++)); + + // make a pile of dynamic clusters and add them to the response + for (size_t i = 0; i < num_clusters; ++i) { + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; + cluster_load_assignment.set_cluster_name("fare_" + std::to_string(i)); + + // Add a whole bunch of hosts in a single place: + auto* endpoints = cluster_load_assignment.add_endpoints(); + endpoints->set_priority(1); + auto* locality = endpoints->mutable_locality(); + locality->set_region("region"); + locality->set_zone("zone"); + locality->set_sub_zone("sub_zone"); + + auto* resource = response->mutable_resources()->Add(); + resource->PackFrom(cluster_load_assignment); + if (v2_config_) { + RELEASE_ASSERT(resource->type_url() == + "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment", + ""); + resource->set_type_url("type.googleapis.com/envoy.api.v2.ClusterLoadAssignment"); + } + } + validation_visitor_.setSkipValidation(ignore_unknown_dynamic_fields); + + state_.SetComplexityN(num_clusters); + state_.ResumeTiming(); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + } + + TestDeprecatedV2Api _deprecated_v2_api_; + State& state_; + const bool v2_config_; + const std::string type_url_; + uint64_t version_{}; + bool initialized_{}; + Stats::IsolatedStoreImpl stats_; + Config::SubscriptionStats subscription_stats_; + Ssl::MockContextManager ssl_context_manager_; + envoy::config::cluster::v3::Cluster eds_cluster_; + NiceMock cm_; + NiceMock dispatcher_; + EdsClusterImplSharedPtr cluster_; + Config::SubscriptionCallbacks* eds_callbacks_{}; + Config::OpaqueResourceDecoderImpl + resource_decoder_{validation_visitor_, "cluster_name"}; + NiceMock random_; + NiceMock runtime_; + NiceMock local_info_; + NiceMock admin_; + Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()}; + NiceMock tls_; + ProtobufMessage::MockValidationVisitor validation_visitor_; + Api::ApiPtr api_; + Grpc::MockAsyncClient* async_client_; + NiceMock async_stream_; + Config::GrpcMuxImplSharedPtr grpc_mux_; + Config::GrpcSubscriptionImplPtr subscription_; +}; + +} // namespace Upstream +} // namespace Envoy + +static void addClusters(State& state) { + Envoy::Thread::MutexBasicLockable lock; + Envoy::Logger::Context logging_state(spdlog::level::warn, + Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); + for (auto _ : state) { + Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); + // if we've been instructed to skip tests, only run once no matter the argument: + uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); + speed_test.clusterHelper(state.range(1), clusters); + } +} + +BENCHMARK(addClusters) + ->Ranges({{false, true}, {false, true}, {64, 100000}}) + ->Unit(benchmark::kMillisecond) + ->Complexity(); + +static void duplicateUpdate(State& state) { + Envoy::Thread::MutexBasicLockable lock; + Envoy::Logger::Context logging_state(spdlog::level::warn, + Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); + + for (auto _ : state) { + Envoy::Upstream::CdsSpeedTest speed_test(state, false); + uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); + + speed_test.clusterHelper(true, clusters); + speed_test.clusterHelper(true, clusters); + } +} + +BENCHMARK(duplicateUpdate) + ->Range(64, 100000) + ->Unit(benchmark::kMillisecond) + ->Complexity(); From 589c94fec8e639e33d4184db4df07978152b5657 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 24 Nov 2020 20:02:47 +0000 Subject: [PATCH 02/15] mechanical formatting Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index afa2c6423a4b3..91f33e96fb9a8 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -190,7 +190,4 @@ static void duplicateUpdate(State& state) { } } -BENCHMARK(duplicateUpdate) - ->Range(64, 100000) - ->Unit(benchmark::kMillisecond) - ->Complexity(); +BENCHMARK(duplicateUpdate)->Range(64, 100000)->Unit(benchmark::kMillisecond)->Complexity(); From d55b81209f8480305c85850ca921d2975ad70943 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 24 Nov 2020 20:12:09 +0000 Subject: [PATCH 03/15] remove duplicate comments Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 91f33e96fb9a8..1b15f507ee58b 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -89,7 +89,6 @@ class CdsSpeedTest { std::chrono::milliseconds(), false); } - // Set up a bunch of clusters that vary only in name, and add them to the configuration. void clusterHelper(bool ignore_unknown_dynamic_fields, size_t num_clusters) { state_.PauseTiming(); @@ -102,7 +101,6 @@ class CdsSpeedTest { envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare_" + std::to_string(i)); - // Add a whole bunch of hosts in a single place: auto* endpoints = cluster_load_assignment.add_endpoints(); endpoints->set_priority(1); auto* locality = endpoints->mutable_locality(); From 5f1834fa67a95072d9d0dae79ea6fa8c5b7ccb63 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 25 Nov 2020 15:06:26 +0000 Subject: [PATCH 04/15] no lint Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 4 ++-- test/common/upstream/eds_speed_test.cc | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 1b15f507ee58b..5df2fb99c2da4 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -161,7 +161,7 @@ static void addClusters(State& state) { Envoy::Thread::MutexBasicLockable lock; Envoy::Logger::Context logging_state(spdlog::level::warn, Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); // if we've been instructed to skip tests, only run once no matter the argument: uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); @@ -179,7 +179,7 @@ static void duplicateUpdate(State& state) { Envoy::Logger::Context logging_state(spdlog::level::warn, Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::CdsSpeedTest speed_test(state, false); uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index 50ee28cdd0a47..d6d351a2db611 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -177,7 +177,7 @@ static void priorityAndLocalityWeighted(State& state) { Envoy::Thread::MutexBasicLockable lock; Envoy::Logger::Context logging_state(spdlog::level::warn, Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, state.range(0)); // if we've been instructed to skip tests, only run once no matter the argument: uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(2); @@ -195,7 +195,7 @@ static void duplicateUpdate(State& state) { Envoy::Logger::Context logging_state(spdlog::level::warn, Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, false); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); @@ -210,7 +210,7 @@ static void healthOnlyUpdate(State& state) { Envoy::Thread::MutexBasicLockable lock; Envoy::Logger::Context logging_state(spdlog::level::warn, Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, false); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); From ca24a4944cb340a8e7f0cd9bb3037c548d4a92d5 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 25 Nov 2020 15:08:51 +0000 Subject: [PATCH 05/15] stop spellchecking linter commands quite so much? Signed-off-by: Phil Genera --- tools/spelling/spelling_dictionary.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/spelling/spelling_dictionary.txt b/tools/spelling/spelling_dictionary.txt index 7911b3134fd75..f5cf418c88d28 100644 --- a/tools/spelling/spelling_dictionary.txt +++ b/tools/spelling/spelling_dictionary.txt @@ -523,6 +523,7 @@ deallocate deallocated deallocating deallocation +deadcode dec dechunk dechunked From 58cf3143cdd2f2ba82f4f47fc30562a330a6c746 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 1 Dec 2020 18:33:58 +0000 Subject: [PATCH 06/15] respond to review comments Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 8 +------- test/common/upstream/eds_speed_test.cc | 10 ---------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 5df2fb99c2da4..4922bd2b1bdda 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -158,9 +158,6 @@ class CdsSpeedTest { } // namespace Envoy static void addClusters(State& state) { - Envoy::Thread::MutexBasicLockable lock; - Envoy::Logger::Context logging_state(spdlog::level::warn, - Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); // if we've been instructed to skip tests, only run once no matter the argument: @@ -174,11 +171,8 @@ BENCHMARK(addClusters) ->Unit(benchmark::kMillisecond) ->Complexity(); +// Look for suboptimal behavior when receiving two identical updates static void duplicateUpdate(State& state) { - Envoy::Thread::MutexBasicLockable lock; - Envoy::Logger::Context logging_state(spdlog::level::warn, - Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::CdsSpeedTest speed_test(state, false); uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index d6d351a2db611..51b0ec9b99680 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -174,9 +174,6 @@ class EdsSpeedTest { } // namespace Envoy static void priorityAndLocalityWeighted(State& state) { - Envoy::Thread::MutexBasicLockable lock; - Envoy::Logger::Context logging_state(spdlog::level::warn, - Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, state.range(0)); // if we've been instructed to skip tests, only run once no matter the argument: @@ -191,10 +188,6 @@ BENCHMARK(priorityAndLocalityWeighted) ->Unit(benchmark::kMillisecond); static void duplicateUpdate(State& state) { - Envoy::Thread::MutexBasicLockable lock; - Envoy::Logger::Context logging_state(spdlog::level::warn, - Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, false); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); @@ -207,9 +200,6 @@ static void duplicateUpdate(State& state) { BENCHMARK(duplicateUpdate)->Range(1, 100000)->Unit(benchmark::kMillisecond); static void healthOnlyUpdate(State& state) { - Envoy::Thread::MutexBasicLockable lock; - Envoy::Logger::Context logging_state(spdlog::level::warn, - Envoy::Logger::Logger::DEFAULT_LOG_FORMAT, lock, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) Envoy::Upstream::EdsSpeedTest speed_test(state, false); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); From cc7f54e975a91d24c1f7762d1e6cc9f583e68d9b Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 2 Dec 2020 19:08:29 +0000 Subject: [PATCH 07/15] respond to review comments Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 11 ++++++++--- test/common/upstream/eds_speed_test.cc | 21 +++++++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 4922bd2b1bdda..fc527c398905d 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -90,7 +90,6 @@ class CdsSpeedTest { } void clusterHelper(bool ignore_unknown_dynamic_fields, size_t num_clusters) { - state_.PauseTiming(); auto response = std::make_unique(); response->set_type_url(type_url_); @@ -122,6 +121,7 @@ class CdsSpeedTest { state_.SetComplexityN(num_clusters); state_.ResumeTiming(); grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + state_.PauseTiming(); } TestDeprecatedV2Api _deprecated_v2_api_; @@ -158,11 +158,14 @@ class CdsSpeedTest { } // namespace Envoy static void addClusters(State& state) { + Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); + // timing is resumed within the helper: + state.PauseTiming(); // if we've been instructed to skip tests, only run once no matter the argument: uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); speed_test.clusterHelper(state.range(1), clusters); + state.ResumeTiming(); } } @@ -173,12 +176,14 @@ BENCHMARK(addClusters) // Look for suboptimal behavior when receiving two identical updates static void duplicateUpdate(State& state) { + Envoy::Upstream::CdsSpeedTest speed_test(state, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - Envoy::Upstream::CdsSpeedTest speed_test(state, false); + state.PauseTiming(); uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); speed_test.clusterHelper(true, clusters); speed_test.clusterHelper(true, clusters); + state.ResumeTiming(); } } diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index 51b0ec9b99680..3e516fe47bdda 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -92,8 +92,6 @@ class EdsSpeedTest { // they are loaded as expected. void priorityAndLocalityWeightedHelper(bool ignore_unknown_dynamic_fields, size_t num_hosts, bool healthy) { - state_.PauseTiming(); - envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("fare"); @@ -136,6 +134,7 @@ class EdsSpeedTest { } state_.ResumeTiming(); grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + state_.PauseTiming(); ASSERT(cluster_->prioritySet().hostSetsPerPriority()[1]->hostsPerLocality().get()[0].size() == num_hosts); } @@ -174,39 +173,45 @@ class EdsSpeedTest { } // namespace Envoy static void priorityAndLocalityWeighted(State& state) { + Envoy::Upstream::EdsSpeedTest speed_test(state, state.range(0)); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - Envoy::Upstream::EdsSpeedTest speed_test(state, state.range(0)); + state.PauseTiming(); // if we've been instructed to skip tests, only run once no matter the argument: uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(2); speed_test.priorityAndLocalityWeightedHelper(state.range(1), endpoints, true); + state.ResumeTiming(); } } BENCHMARK(priorityAndLocalityWeighted) - ->Ranges({{false, true}, {false, true}, {1, 100000}}) + ->Ranges({{false, true}, {false, true}, {64, 100000}}) ->Unit(benchmark::kMillisecond); static void duplicateUpdate(State& state) { + Envoy::Upstream::EdsSpeedTest speed_test(state, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - Envoy::Upstream::EdsSpeedTest speed_test(state, false); + state.PauseTiming(); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); + state.ResumeTiming(); } } -BENCHMARK(duplicateUpdate)->Range(1, 100000)->Unit(benchmark::kMillisecond); +BENCHMARK(duplicateUpdate)->Range(64, 100000)->Unit(benchmark::kMillisecond); static void healthOnlyUpdate(State& state) { + Envoy::Upstream::EdsSpeedTest speed_test(state, false); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - Envoy::Upstream::EdsSpeedTest speed_test(state, false); + state.PauseTiming(); uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); speed_test.priorityAndLocalityWeightedHelper(true, endpoints, false); + state.ResumeTiming(); } } -BENCHMARK(healthOnlyUpdate)->Range(1, 100000)->Unit(benchmark::kMillisecond); +BENCHMARK(healthOnlyUpdate)->Range(64, 100000)->Unit(benchmark::kMillisecond); From be6a36e02756f5d999fbe58e64012d4970960ee4 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 2 Dec 2020 23:34:45 +0000 Subject: [PATCH 08/15] respond to review comment Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 12 ++++++------ test/common/upstream/eds_speed_test.cc | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index fc527c398905d..40b0a71df0038 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -159,12 +159,12 @@ class CdsSpeedTest { static void addClusters(State& state) { Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); + // if we've been instructed to skip tests, only run once no matter the argument: + const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) // timing is resumed within the helper: state.PauseTiming(); - // if we've been instructed to skip tests, only run once no matter the argument: - uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); - speed_test.clusterHelper(state.range(1), clusters); + speed_test.clusterHelper(state.range(1), num_clusters); state.ResumeTiming(); } } @@ -177,12 +177,12 @@ BENCHMARK(addClusters) // Look for suboptimal behavior when receiving two identical updates static void duplicateUpdate(State& state) { Envoy::Upstream::CdsSpeedTest speed_test(state, false); + const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) state.PauseTiming(); - uint32_t clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); - speed_test.clusterHelper(true, clusters); - speed_test.clusterHelper(true, clusters); + speed_test.clusterHelper(true, num_clusters); + speed_test.clusterHelper(true, num_clusters); state.ResumeTiming(); } } diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index 3e516fe47bdda..dfaf9a136b348 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -174,12 +174,12 @@ class EdsSpeedTest { static void priorityAndLocalityWeighted(State& state) { Envoy::Upstream::EdsSpeedTest speed_test(state, state.range(0)); + // if we've been instructed to skip tests, only run once no matter the argument: + const uint32_t num_endpoints = skipExpensiveBenchmarks() ? 1 : state.range(2); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) state.PauseTiming(); - // if we've been instructed to skip tests, only run once no matter the argument: - uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(2); - speed_test.priorityAndLocalityWeightedHelper(state.range(1), endpoints, true); + speed_test.priorityAndLocalityWeightedHelper(state.range(1), num_endpoints, true); state.ResumeTiming(); } } @@ -190,12 +190,12 @@ BENCHMARK(priorityAndLocalityWeighted) static void duplicateUpdate(State& state) { Envoy::Upstream::EdsSpeedTest speed_test(state, false); + const uint32_t num_endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) state.PauseTiming(); - uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); - speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); - speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); + speed_test.priorityAndLocalityWeightedHelper(true, num_endpoints, true); + speed_test.priorityAndLocalityWeightedHelper(true, num_endpoints, true); state.ResumeTiming(); } } @@ -204,12 +204,12 @@ BENCHMARK(duplicateUpdate)->Range(64, 100000)->Unit(benchmark::kMillisecond); static void healthOnlyUpdate(State& state) { Envoy::Upstream::EdsSpeedTest speed_test(state, false); + const uint32_t num_endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) state.PauseTiming(); - uint32_t endpoints = skipExpensiveBenchmarks() ? 1 : state.range(0); - speed_test.priorityAndLocalityWeightedHelper(true, endpoints, true); - speed_test.priorityAndLocalityWeightedHelper(true, endpoints, false); + speed_test.priorityAndLocalityWeightedHelper(true, num_endpoints, true); + speed_test.priorityAndLocalityWeightedHelper(true, num_endpoints, false); state.ResumeTiming(); } } From db19c30a2c68c1dbe314c1702a1bad0cb0cdc084 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Mon, 7 Dec 2020 17:13:12 +0000 Subject: [PATCH 09/15] respond to review comments; split out v2 tests Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 2 + test/common/upstream/cds_speed_test.cc | 107 ++++++++++++------------- 2 files changed, 54 insertions(+), 55 deletions(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 30dc3d5707831..bc7605821e06c 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -158,9 +158,11 @@ envoy_cc_benchmark_binary( "//source/common/config:grpc_subscription_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:utility_lib", + "//source/common/upstream:cds_api_lib", "//source/common/upstream:eds_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:transport_socket_config_lib", + "//test/config:utility_lib", "//test/mocks/local_info:local_info_mocks", "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/runtime:runtime_mocks", diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 40b0a71df0038..ad042e449ff16 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -2,6 +2,7 @@ // quiescent system with disabled cstate power management. #include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/cluster/v3/cluster.pb.validate.h" #include "envoy/config/core/v3/health_check.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/config/endpoint/v3/endpoint_components.pb.h" @@ -13,11 +14,13 @@ #include "common/config/utility.h" #include "common/singleton/manager_impl.h" #include "common/upstream/eds.h" +#include "common/upstream/static_cluster.h" #include "server/transport_socket_config_impl.h" #include "test/benchmark/main.h" #include "test/common/upstream/utility.h" +#include "test/config/utility.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/runtime/mocks.h" @@ -40,82 +43,60 @@ class CdsSpeedTest { public: CdsSpeedTest(State& state, bool v2_config) : state_(state), v2_config_(v2_config), - type_url_(v2_config_ - ? "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment" - : "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment"), + type_url_(v2_config_ ? "type.googleapis.com/envoy.api.v2.Cluster" + : "type.googleapis.com/envoy.config.cluster.v3.Cluster"), subscription_stats_(Config::Utility::generateStats(stats_)), api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()), grpc_mux_(new Config::GrpcMuxImpl( local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( - "envoy.service.endpoint.v3.EndpointDiscoveryService.StreamEndpoints"), + "envoy.service.cluster.v3.ClusterDiscoveryService.StreamClusters"), envoy::config::core::v3::ApiVersion::AUTO, random_, stats_, {}, true)) { - resetCluster(R"EOF( - name: name - connect_timeout: 0.25s - type: EDS - eds_cluster_config: - service_name: fare - eds_config: - api_config_source: - cluster_names: - - eds - refresh_delay: 1s - )EOF", - Envoy::Upstream::Cluster::InitializePhase::Secondary); - - EXPECT_CALL(*cm_.subscription_factory_.subscription_, start(_, _)); + resetCluster(); + cluster_->initialize([this] { initialized_ = true; }); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(testing::Return(&async_stream_)); subscription_->start({"fare"}); } - void resetCluster(const std::string& yaml_config, Cluster::InitializePhase initialize_phase) { + void resetCluster() { local_info_.node_.mutable_locality()->set_zone("us-east-1a"); - eds_cluster_ = parseClusterFromV3Yaml(yaml_config); + static_cluster_ = ConfigHelper::buildStaticCluster("cluster", 1024, "127.0.0.1"); Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( - "cluster.{}.", - eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name())); + "cluster.{}.", static_cluster_.alt_stat_name().empty() ? static_cluster_.name() + : static_cluster_.alt_stat_name())); Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, stats_, singleton_manager_, tls_, validation_visitor_, *api_); - cluster_ = std::make_shared(eds_cluster_, runtime_, factory_context, - std::move(scope), false); - EXPECT_EQ(initialize_phase, cluster_->initializePhase()); - eds_callbacks_ = cm_.subscription_factory_.callbacks_; + cluster_ = std::make_shared(static_cluster_, runtime_, factory_context, + std::move(scope), false); + EXPECT_EQ(Envoy::Upstream::Cluster::InitializePhase::Primary, cluster_->initializePhase()); + callbacks_ = cm_.subscription_factory_.callbacks_; subscription_ = std::make_unique( - grpc_mux_, *eds_callbacks_, resource_decoder_, subscription_stats_, type_url_, dispatcher_, + grpc_mux_, *callbacks_, resource_decoder_, subscription_stats_, type_url_, dispatcher_, std::chrono::milliseconds(), false); } void clusterHelper(bool ignore_unknown_dynamic_fields, size_t num_clusters) { - auto response = std::make_unique(); response->set_type_url(type_url_); response->set_version_info(fmt::format("version-{}", version_++)); - // make a pile of dynamic clusters and add them to the response + // make a pile of static clusters and add them to the response for (size_t i = 0; i < num_clusters; ++i) { - envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; - cluster_load_assignment.set_cluster_name("fare_" + std::to_string(i)); - - auto* endpoints = cluster_load_assignment.add_endpoints(); - endpoints->set_priority(1); - auto* locality = endpoints->mutable_locality(); - locality->set_region("region"); - locality->set_zone("zone"); - locality->set_sub_zone("sub_zone"); + envoy::config::cluster::v3::Cluster cluster = ConfigHelper::buildStaticCluster( + "cluster_" + std::to_string(i), i % 60000, "10.0.1." + std::to_string(i / 60000)); auto* resource = response->mutable_resources()->Add(); - resource->PackFrom(cluster_load_assignment); + resource->PackFrom(cluster); if (v2_config_) { - RELEASE_ASSERT(resource->type_url() == - "type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment", - ""); - resource->set_type_url("type.googleapis.com/envoy.api.v2.ClusterLoadAssignment"); + RELEASE_ASSERT( + resource->type_url() == "type.googleapis.com/envoy.config.cluster.v3.Cluster", ""); + resource->set_type_url("type.googleapis.com/envoy.api.v2.Cluster"); } } + validation_visitor_.setSkipValidation(ignore_unknown_dynamic_fields); state_.SetComplexityN(num_clusters); @@ -133,13 +114,13 @@ class CdsSpeedTest { Stats::IsolatedStoreImpl stats_; Config::SubscriptionStats subscription_stats_; Ssl::MockContextManager ssl_context_manager_; - envoy::config::cluster::v3::Cluster eds_cluster_; + envoy::config::cluster::v3::Cluster static_cluster_; NiceMock cm_; NiceMock dispatcher_; - EdsClusterImplSharedPtr cluster_; - Config::SubscriptionCallbacks* eds_callbacks_{}; - Config::OpaqueResourceDecoderImpl - resource_decoder_{validation_visitor_, "cluster_name"}; + ClusterImplBaseSharedPtr cluster_; + Config::SubscriptionCallbacks* callbacks_{}; + Config::OpaqueResourceDecoderImpl resource_decoder_{ + validation_visitor_, "name"}; NiceMock random_; NiceMock runtime_; NiceMock local_info_; @@ -157,20 +138,36 @@ class CdsSpeedTest { } // namespace Upstream } // namespace Envoy -static void addClusters(State& state) { - Envoy::Upstream::CdsSpeedTest speed_test(state, state.range(0)); +static void addV3Clusters(State& state) { + Envoy::Upstream::CdsSpeedTest speed_test(state, false); // if we've been instructed to skip tests, only run once no matter the argument: - const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(2); + const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(1); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) // timing is resumed within the helper: state.PauseTiming(); - speed_test.clusterHelper(state.range(1), num_clusters); + speed_test.clusterHelper(state.range(0), num_clusters); + state.ResumeTiming(); + } +} + +BENCHMARK(addV3Clusters) + ->Ranges({{false, true}, {64, 100000}}) + ->Unit(benchmark::kMillisecond) + ->Complexity(); + +static void addV2Clusters(State& state) { + // this true is the only difference from addV3clusters above + Envoy::Upstream::CdsSpeedTest speed_test(state, true); + const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(1); + for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) + state.PauseTiming(); + speed_test.clusterHelper(state.range(0), num_clusters); state.ResumeTiming(); } } -BENCHMARK(addClusters) - ->Ranges({{false, true}, {false, true}, {64, 100000}}) +BENCHMARK(addV2Clusters) + ->Ranges({{false, true}, {64, 100000}}) ->Unit(benchmark::kMillisecond) ->Complexity(); From cd034a9dbf7037938e34b17b0a2e331c12dfd690 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Tue, 8 Dec 2020 15:42:28 +0000 Subject: [PATCH 10/15] remove v2 API benchmark Signed-off-by: Phil Genera --- test/common/upstream/cds_speed_test.cc | 38 ++++++-------------------- 1 file changed, 8 insertions(+), 30 deletions(-) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index ad042e449ff16..7d891e724744d 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -41,10 +41,8 @@ namespace Upstream { class CdsSpeedTest { public: - CdsSpeedTest(State& state, bool v2_config) - : state_(state), v2_config_(v2_config), - type_url_(v2_config_ ? "type.googleapis.com/envoy.api.v2.Cluster" - : "type.googleapis.com/envoy.config.cluster.v3.Cluster"), + CdsSpeedTest(State& state) + : state_(state), type_url_("type.googleapis.com/envoy.config.cluster.v3.Cluster"), subscription_stats_(Config::Utility::generateStats(stats_)), api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()), grpc_mux_(new Config::GrpcMuxImpl( @@ -90,11 +88,8 @@ class CdsSpeedTest { auto* resource = response->mutable_resources()->Add(); resource->PackFrom(cluster); - if (v2_config_) { - RELEASE_ASSERT( - resource->type_url() == "type.googleapis.com/envoy.config.cluster.v3.Cluster", ""); - resource->set_type_url("type.googleapis.com/envoy.api.v2.Cluster"); - } + RELEASE_ASSERT(resource->type_url() == "type.googleapis.com/envoy.config.cluster.v3.Cluster", + ""); } validation_visitor_.setSkipValidation(ignore_unknown_dynamic_fields); @@ -107,7 +102,6 @@ class CdsSpeedTest { TestDeprecatedV2Api _deprecated_v2_api_; State& state_; - const bool v2_config_; const std::string type_url_; uint64_t version_{}; bool initialized_{}; @@ -138,8 +132,8 @@ class CdsSpeedTest { } // namespace Upstream } // namespace Envoy -static void addV3Clusters(State& state) { - Envoy::Upstream::CdsSpeedTest speed_test(state, false); +static void addClusters(State& state) { + Envoy::Upstream::CdsSpeedTest speed_test(state); // if we've been instructed to skip tests, only run once no matter the argument: const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(1); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) @@ -150,30 +144,14 @@ static void addV3Clusters(State& state) { } } -BENCHMARK(addV3Clusters) - ->Ranges({{false, true}, {64, 100000}}) - ->Unit(benchmark::kMillisecond) - ->Complexity(); - -static void addV2Clusters(State& state) { - // this true is the only difference from addV3clusters above - Envoy::Upstream::CdsSpeedTest speed_test(state, true); - const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(1); - for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) - state.PauseTiming(); - speed_test.clusterHelper(state.range(0), num_clusters); - state.ResumeTiming(); - } -} - -BENCHMARK(addV2Clusters) +BENCHMARK(addClusters) ->Ranges({{false, true}, {64, 100000}}) ->Unit(benchmark::kMillisecond) ->Complexity(); // Look for suboptimal behavior when receiving two identical updates static void duplicateUpdate(State& state) { - Envoy::Upstream::CdsSpeedTest speed_test(state, false); + Envoy::Upstream::CdsSpeedTest speed_test(state); const uint32_t num_clusters = skipExpensiveBenchmarks() ? 1 : state.range(0); for (auto _ : state) { // NOLINT(clang-analyzer-deadcode.DeadStores) state.PauseTiming(); From 8c3b9c1ae40a69ad3533d0cd39ea65742d83be92 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 9 Dec 2020 20:22:11 +0000 Subject: [PATCH 11/15] respond to review comments Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 2 -- test/common/upstream/cds_speed_test.cc | 7 ++----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index fd0e6e6a469bf..f03bc674cbd37 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -160,7 +160,6 @@ envoy_cc_benchmark_binary( "//source/common/config:protobuf_link_hacks", "//source/common/config:utility_lib", "//source/common/upstream:cds_api_lib", - "//source/common/upstream:eds_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:transport_socket_config_lib", "//test/config:utility_lib", @@ -175,7 +174,6 @@ envoy_cc_benchmark_binary( "//test/test_common:utility_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", - "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", ], ) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 7d891e724744d..51ecf8831a29c 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -5,7 +5,6 @@ #include "envoy/config/cluster/v3/cluster.pb.validate.h" #include "envoy/config/core/v3/health_check.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" -#include "envoy/config/endpoint/v3/endpoint_components.pb.h" #include "envoy/service/discovery/v3/discovery.pb.h" #include "envoy/stats/scope.h" @@ -13,7 +12,6 @@ #include "common/config/grpc_subscription_impl.h" #include "common/config/utility.h" #include "common/singleton/manager_impl.h" -#include "common/upstream/eds.h" #include "common/upstream/static_cluster.h" #include "server/transport_socket_config_impl.h" @@ -60,7 +58,7 @@ class CdsSpeedTest { void resetCluster() { local_info_.node_.mutable_locality()->set_zone("us-east-1a"); - static_cluster_ = ConfigHelper::buildStaticCluster("cluster", 1024, "127.0.0.1"); + static_cluster_ = ConfigHelper::buildStaticCluster("staticcluster", 1024, "127.0.0.1"); Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( "cluster.{}.", static_cluster_.alt_stat_name().empty() ? static_cluster_.name() : static_cluster_.alt_stat_name())); @@ -100,12 +98,11 @@ class CdsSpeedTest { state_.PauseTiming(); } - TestDeprecatedV2Api _deprecated_v2_api_; State& state_; const std::string type_url_; uint64_t version_{}; bool initialized_{}; - Stats::IsolatedStoreImpl stats_; + Stats::TestUtil::TestStore stats_; Config::SubscriptionStats subscription_stats_; Ssl::MockContextManager ssl_context_manager_; envoy::config::cluster::v3::Cluster static_cluster_; From 4f797099de75db9aa66bc5da680e32207132dc59 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 9 Dec 2020 20:26:21 +0000 Subject: [PATCH 12/15] check_format.py fix Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index f03bc674cbd37..07a19bfd3322d 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -174,6 +174,7 @@ envoy_cc_benchmark_binary( "//test/test_common:utility_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", ], ) From 9289d187a70f399048360100a1e082f8f1d6e04a Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Wed, 9 Dec 2020 20:31:38 +0000 Subject: [PATCH 13/15] missed a vestigial import Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 1 - test/common/upstream/cds_speed_test.cc | 1 - 2 files changed, 2 deletions(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 07a19bfd3322d..f03bc674cbd37 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -174,7 +174,6 @@ envoy_cc_benchmark_binary( "//test/test_common:utility_lib", "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", - "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", ], ) diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 51ecf8831a29c..c625e10c5d721 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -4,7 +4,6 @@ #include "envoy/config/cluster/v3/cluster.pb.h" #include "envoy/config/cluster/v3/cluster.pb.validate.h" #include "envoy/config/core/v3/health_check.pb.h" -#include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/service/discovery/v3/discovery.pb.h" #include "envoy/stats/scope.h" From 1d3692f4222614a3686adde5c6df4f7ba02fd6f8 Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Fri, 11 Dec 2020 16:22:12 +0000 Subject: [PATCH 14/15] build clusters natively rather than via yaml Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 1 - test/common/upstream/cds_speed_test.cc | 23 ++++++++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index f03bc674cbd37..e8e2cfd9fe676 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -162,7 +162,6 @@ envoy_cc_benchmark_binary( "//source/common/upstream:cds_api_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:transport_socket_config_lib", - "//test/config:utility_lib", "//test/mocks/local_info:local_info_mocks", "//test/mocks/protobuf:protobuf_mocks", "//test/mocks/runtime:runtime_mocks", diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index c625e10c5d721..4ea8b3bd07529 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -17,7 +17,6 @@ #include "test/benchmark/main.h" #include "test/common/upstream/utility.h" -#include "test/config/utility.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/runtime/mocks.h" @@ -57,7 +56,7 @@ class CdsSpeedTest { void resetCluster() { local_info_.node_.mutable_locality()->set_zone("us-east-1a"); - static_cluster_ = ConfigHelper::buildStaticCluster("staticcluster", 1024, "127.0.0.1"); + static_cluster_ = buildStaticCluster("staticcluster", 1024, "127.0.0.1"); Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( "cluster.{}.", static_cluster_.alt_stat_name().empty() ? static_cluster_.name() : static_cluster_.alt_stat_name())); @@ -80,7 +79,7 @@ class CdsSpeedTest { // make a pile of static clusters and add them to the response for (size_t i = 0; i < num_clusters; ++i) { - envoy::config::cluster::v3::Cluster cluster = ConfigHelper::buildStaticCluster( + envoy::config::cluster::v3::Cluster cluster = buildStaticCluster( "cluster_" + std::to_string(i), i % 60000, "10.0.1." + std::to_string(i / 60000)); auto* resource = response->mutable_resources()->Add(); @@ -97,6 +96,24 @@ class CdsSpeedTest { state_.PauseTiming(); } + // this is ConfigHelper::buildStaticCluster, but without YAML, in the interest of efficiency. + envoy::config::cluster::v3::Cluster buildStaticCluster(const std::string& name, const int port, + const std::string& address) { + envoy::config::cluster::v3::Cluster cluster; + cluster.set_name(name); + cluster.mutable_connect_timeout()->set_seconds(5); + cluster.set_type(envoy::config::cluster::v3::Cluster::STATIC); + cluster.set_lb_policy(envoy::config::cluster::v3::Cluster::ROUND_ROBIN); + auto* load_assignment = cluster.mutable_load_assignment(); + load_assignment->set_cluster_name(name); + auto* endpoint = load_assignment->add_endpoints()->add_lb_endpoints()->mutable_endpoint(); + auto* socket_address = endpoint->mutable_address()->mutable_socket_address(); + socket_address->set_address(address); + socket_address->set_port_value(port); + + return cluster; + } + State& state_; const std::string type_url_; uint64_t version_{}; From 256b961e5b8e16d521d8042c8a510b71ef45e87b Mon Sep 17 00:00:00 2001 From: Phil Genera Date: Fri, 11 Dec 2020 20:11:19 +0000 Subject: [PATCH 15/15] use a more concrete stats implementation Signed-off-by: Phil Genera --- test/common/upstream/BUILD | 4 ++++ test/common/upstream/cds_speed_test.cc | 5 ++++- test/common/upstream/eds_speed_test.cc | 5 ++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index e8e2cfd9fe676..1a150ebaf7a2d 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -123,6 +123,7 @@ envoy_cc_benchmark_binary( "//source/common/config:grpc_subscription_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:utility_lib", + "//source/common/stats:thread_local_store_lib", "//source/common/upstream:eds_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:transport_socket_config_lib", @@ -132,6 +133,7 @@ envoy_cc_benchmark_binary( "//test/mocks/server:admin_mocks", "//test/mocks/server:instance_mocks", "//test/mocks/ssl:ssl_mocks", + "//test/mocks/stats:stats_mocks", "//test/mocks/upstream:cluster_manager_mocks", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", @@ -159,6 +161,7 @@ envoy_cc_benchmark_binary( "//source/common/config:grpc_subscription_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:utility_lib", + "//source/common/stats:thread_local_store_lib", "//source/common/upstream:cds_api_lib", "//source/extensions/transport_sockets/raw_buffer:config", "//source/server:transport_socket_config_lib", @@ -168,6 +171,7 @@ envoy_cc_benchmark_binary( "//test/mocks/server:admin_mocks", "//test/mocks/server:instance_mocks", "//test/mocks/ssl:ssl_mocks", + "//test/mocks/stats:stats_mocks", "//test/mocks/upstream:cluster_manager_mocks", "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", diff --git a/test/common/upstream/cds_speed_test.cc b/test/common/upstream/cds_speed_test.cc index 4ea8b3bd07529..d1cdf06eed1d7 100644 --- a/test/common/upstream/cds_speed_test.cc +++ b/test/common/upstream/cds_speed_test.cc @@ -11,6 +11,7 @@ #include "common/config/grpc_subscription_impl.h" #include "common/config/utility.h" #include "common/singleton/manager_impl.h" +#include "common/stats/thread_local_store.h" #include "common/upstream/static_cluster.h" #include "server/transport_socket_config_impl.h" @@ -118,7 +119,9 @@ class CdsSpeedTest { const std::string type_url_; uint64_t version_{}; bool initialized_{}; - Stats::TestUtil::TestStore stats_; + Stats::TestSymbolTable symbol_table_{}; + Stats::AllocatorImpl stats_allocator_{*symbol_table_}; + Stats::ThreadLocalStoreImpl stats_{stats_allocator_}; Config::SubscriptionStats subscription_stats_; Ssl::MockContextManager ssl_context_manager_; envoy::config::cluster::v3::Cluster static_cluster_; diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index 45c0e8fc54a3d..444ce7706fc5e 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -12,6 +12,7 @@ #include "common/config/grpc_subscription_impl.h" #include "common/config/utility.h" #include "common/singleton/manager_impl.h" +#include "common/stats/thread_local_store.h" #include "common/upstream/eds.h" #include "server/transport_socket_config_impl.h" @@ -145,7 +146,9 @@ class EdsSpeedTest { const std::string type_url_; uint64_t version_{}; bool initialized_{}; - Stats::TestUtil::TestStore stats_; + Stats::TestSymbolTable symbol_table_{}; + Stats::AllocatorImpl stats_allocator_{*symbol_table_}; + Stats::ThreadLocalStoreImpl stats_{stats_allocator_}; Config::SubscriptionStats subscription_stats_; Ssl::MockContextManager ssl_context_manager_; envoy::config::cluster::v3::Cluster eds_cluster_;