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
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Removed Config or Runtime
* compression: removed ``envoy.reloadable_features.enable_compression_without_content_length_header`` runtime guard and legacy code paths.
* http: removed ``envoy.reloadable_features.return_502_for_upstream_protocol_errors``. Envoy will always return 502 code upon encountering upstream protocol error.
* http: removed ``envoy.reloadable_features.treat_upstream_connect_timeout_as_connect_failure`` and legacy code paths.
* upstream: removed ``envoy.reloadable_features.upstream_host_weight_change_causes_rebuild`` and legacy code paths.

New Features
------------
Expand Down
1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.udp_listener_updates_filter_chain_in_place",
"envoy.reloadable_features.udp_per_event_loop_read_limit",
"envoy.reloadable_features.unquote_log_string_values",
"envoy.reloadable_features.upstream_host_weight_change_causes_rebuild",
"envoy.reloadable_features.use_observable_cluster_name",
"envoy.reloadable_features.validate_connect",
"envoy.reloadable_features.vhds_heartbeats",
Expand Down
13 changes: 5 additions & 8 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1607,14 +1607,11 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(
}
if (existing_host->second->weight() != host->weight()) {
existing_host->second->weight(host->weight());
if (Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.upstream_host_weight_change_causes_rebuild")) {
// We do full host set rebuilds so that load balancers can do pre-computation of data
// structures based on host weight. This may become a performance problem in certain
// deployments so it is runtime feature guarded and may also need to be configurable
// and/or dynamic in the future.
hosts_changed = true;
}
// We do full host set rebuilds so that load balancers can do pre-computation of data
// structures based on host weight. This may become a performance problem in certain
// deployments so it is runtime feature guarded and may also need to be configurable
// and/or dynamic in the future.
hosts_changed = true;
}

hosts_changed |=
Expand Down
50 changes: 16 additions & 34 deletions test/common/upstream/eds_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -355,9 +355,8 @@ TEST_F(EdsTest, EdsClusterFromFileIsPrimaryCluster) {
EXPECT_TRUE(initialized_);
}

namespace {

void endpointWeightChangeCausesRebuildTest(EdsTest& test, bool expect_rebuild) {
// Verify that host weight changes cause a full rebuild.
TEST_F(EdsTest, EndpointWeightChangeCausesRebuild) {
envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment;
cluster_load_assignment.set_cluster_name("fare");
auto* endpoints = cluster_load_assignment.add_endpoints();
Expand All @@ -366,45 +365,28 @@ void endpointWeightChangeCausesRebuildTest(EdsTest& test, bool expect_rebuild) {
endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80);
endpoint->mutable_load_balancing_weight()->set_value(30);

test.initialize();
test.doOnConfigUpdateVerifyNoThrow(cluster_load_assignment);
EXPECT_TRUE(test.initialized_);
EXPECT_EQ(0UL, test.stats_.counter("cluster.name.update_no_rebuild").value());
EXPECT_EQ(30UL,
test.stats_.gauge("cluster.name.max_host_weight", Stats::Gauge::ImportMode::Accumulate)
.value());
auto& hosts = test.cluster_->prioritySet().hostSetsPerPriority()[0]->hosts();
initialize();
doOnConfigUpdateVerifyNoThrow(cluster_load_assignment);
EXPECT_TRUE(initialized_);
EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value());
EXPECT_EQ(
30UL,
stats_.gauge("cluster.name.max_host_weight", Stats::Gauge::ImportMode::Accumulate).value());
auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts();
EXPECT_EQ(hosts.size(), 1);
EXPECT_EQ(hosts[0]->weight(), 30);

endpoint->mutable_load_balancing_weight()->set_value(31);
test.doOnConfigUpdateVerifyNoThrow(cluster_load_assignment);
EXPECT_EQ(expect_rebuild ? 0UL : 1UL,
test.stats_.counter("cluster.name.update_no_rebuild").value());
EXPECT_EQ(31UL,
test.stats_.gauge("cluster.name.max_host_weight", Stats::Gauge::ImportMode::Accumulate)
.value());
auto& new_hosts = test.cluster_->prioritySet().hostSetsPerPriority()[0]->hosts();
doOnConfigUpdateVerifyNoThrow(cluster_load_assignment);
EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value());
EXPECT_EQ(
31UL,
stats_.gauge("cluster.name.max_host_weight", Stats::Gauge::ImportMode::Accumulate).value());
auto& new_hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts();
EXPECT_EQ(new_hosts.size(), 1);
EXPECT_EQ(new_hosts[0]->weight(), 31);
}

} // namespace

// Verify that host weight changes cause a full rebuild.
TEST_F(EdsTest, EndpointWeightChangeCausesRebuild) {
endpointWeightChangeCausesRebuildTest(*this, true);
}

// Verify that host weight changes do not cause a full rebuild when the feature flag is disabled.
TEST_F(EdsTest, EndpointWeightChangeCausesRebuildDisabled) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.upstream_host_weight_change_causes_rebuild", "false"}});

endpointWeightChangeCausesRebuildTest(*this, false);
}

// Validate that onConfigUpdate() updates the endpoint metadata.
TEST_F(EdsTest, EndpointMetadata) {
envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment;
Expand Down