Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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
7 changes: 6 additions & 1 deletion api/envoy/config/cluster/v3/cluster.proto
Original file line number Diff line number Diff line change
Expand Up @@ -363,12 +363,17 @@ message Cluster {
// By tuning the parameter, is possible to achieve polynomial or exponential shape of ramp-up curve.
//
// During slow start window, effective weight of an endpoint would be scaled with time factor and aggression:
// `new_weight = weight * time_factor ^ (1 / aggression)`,
// `new_weight = weight * max(min_weight_percent, time_factor ^ (1 / aggression))`,
// where `time_factor=(time_since_start_seconds / slow_start_time_seconds)`.
//
// As time progresses, more and more traffic would be sent to endpoint, which is in slow start window.
// Once host exits slow start, time_factor and aggression no longer affect its weight.
core.v3.RuntimeDouble aggression = 2;

// Configures the minimum percentage of origin weight that avoids too small new weight,
// which may cause endpoints in slow start mode receive no traffic in slow start window.
// Range between (0.0, 1.0). If not specified, the default is 0.1.
double min_weight_percent = 3 [(validate.rules).double = {lt: 1.0 gt: 0.0 ignore_empty: true}];

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The type should be changed to Percent (see api style).
Also note that this implies that the range should be (0, 100).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Another option is to use a double wrapper (as the default value isn't 0), and then use PROTOBUF_GET_WRAPPED_OR_DEFAULT to set the value in the c'tor.

}

// Specific configuration for the RoundRobin load balancing policy.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ During slow start window, load balancing weight of a particular endpoint will be

.. math::

NewWeight = {Weight*TimeFactor}^\frac{1}{Aggression}
NewWeight = {Weight}*{max(MinWeightPercent,{TimeFactor}^\frac{1}{Aggression})}

where,

Expand All @@ -25,6 +25,8 @@ where,

As time progresses, more and more traffic would be sent to endpoint within slow start window.

:ref:`MinWeightPercent parameter<envoy_v3_api_field_config.cluster.v3.Cluster.SlowStartConfig.min_weight_percent>` specifies the minimum percent of origin weight to make sure the ebf scheduler has a reasonable deadline, range between (0.0, 1.0), default is 0.1.

:ref:`Aggression parameter<envoy_v3_api_field_config.cluster.v3.Cluster.SlowStartConfig.aggression>` non-linearly affects endpoint weight and represents the speed of ramp-up.
By tuning aggression parameter, one could achieve polynomial or exponential speed for traffic increase.
Below simulation demonstrates how various values for aggression affect traffic ramp-up:
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Bug Fixes
* data plane: fixing error handling where writing to a socket failed while under the stack of processing. This should genreally affect HTTP/3. This behavioral change can be reverted by setting ``envoy.reloadable_features.allow_upstream_inline_write`` to false.
* eds: fix the eds cluster update by allowing update on the locality of the cluster endpoints. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.support_locality_update_on_eds_cluster_endpoints`` to false.
* tls: fix a bug while matching a certificate SAN with an exact value in ``match_typed_subject_alt_names`` of a listener where wildcard ``*`` character is not the only character of the dns label. Example, ``baz*.example.net`` and ``*baz.example.net`` and ``b*z.example.net`` will match ``baz1.example.net`` and ``foobaz.example.net`` and ``buzz.example.net``, respectively.
* upstream: cluster slow start config add ``min_weight_percent`` field to avoid too big edf deadline, default 0.1.
* xray: fix the AWS X-Ray tracer extension to not sample the trace if ``sampled=`` keyword is not present in the header ``x-amzn-trace-id``.

Removed Config or Runtime
Expand Down
15 changes: 14 additions & 1 deletion source/common/upstream/load_balancer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,19 @@ EdfLoadBalancerBase::EdfLoadBalancerBase(
recalculateHostsInSlowStart(hosts_added);
}
});

min_weight_percent_ = 0.1;
if (slow_start_config.has_value()) {
if (slow_start_config.value().min_weight_percent() > 0.0 &&
slow_start_config.value().min_weight_percent() < 1.0) {
min_weight_percent_ = slow_start_config.value().min_weight_percent();
} else {
ENVOY_LOG(warn,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that if PGV rejects the config (due to wrong value), this code will never be reached.

"Invalid value {} provided for min_weight_percent parameter, must be in range "
"(0.0, 1.0), so use default value 0.1",
slow_start_config.value().min_weight_percent());
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Once the field is of type Percent, you can initialize the min_weight_percent_ field in the MIL, and use PROTOBUF_PERCENT_TO_ROUNDED_INTEGER_OR_DEFAULT

Something similar to:

PROTOBUF_PERCENT_TO_DOUBLE_OR_DEFAULT(
         slow_start_config, min_weight_percent, 10))/100.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks for advice. i have change to use Percent. please have a look.

}

void EdfLoadBalancerBase::initialize() {
Expand Down Expand Up @@ -967,7 +980,7 @@ double EdfLoadBalancerBase::applySlowStartFactor(double host_weight, const Host&
auto time_factor = static_cast<double>(std::max(std::chrono::milliseconds(1).count(),
host_create_duration.count())) /
slow_start_window_.count();
return host_weight * applyAggressionFactor(time_factor);
return host_weight * std::max(applyAggressionFactor(time_factor), min_weight_percent_);
} else {
return host_weight;
}
Expand Down
1 change: 1 addition & 0 deletions source/common/upstream/load_balancer_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ class EdfLoadBalancerBase : public ZoneAwareLoadBalancerBase,
const absl::optional<Runtime::Double> aggression_runtime_;
TimeSource& time_source_;
MonotonicTime latest_host_added_time_;
double min_weight_percent_;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's clear what this is for in the context of this PR, but in the load balancer file the name can be confusing. Can you change it to something that makes it clear this is specifically for the slow-start weight modifications? Something like slow_start_min_weight_pct_

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

};

/**
Expand Down
61 changes: 61 additions & 0 deletions test/common/upstream/cds_api_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,67 @@ TEST_F(CdsApiImplTest, FailureSubscription) {
EXPECT_EQ("", cds_->versionInfo());
}

// Validate behavior when the config is delivered but it fails PGV validation for slow start config.
TEST_F(CdsApiImplTest, FailureInvalidSlowStartConfigMinWeightPercentEQ1) {
InSequence s;

setup();

const std::string response1_yaml = R"EOF(
version_info: '0'
resources:
- "@type": type.googleapis.com/envoy.config.cluster.v3.Cluster
name: cluster1
type: EDS
eds_cluster_config:
eds_config:
ads: {}
resource_api_version: V3
service_name: cluster1
round_robin_lb_config:
slow_start_config:
min_weight_percent: 1.0
)EOF";
auto response1 =
TestUtility::parseYaml<envoy::service::discovery::v3::DiscoveryResponse>(response1_yaml);

EXPECT_THROW(TestUtility::decodeResources<envoy::config::cluster::v3::Cluster>(response1),
EnvoyException);
}

TEST_F(CdsApiImplTest, SlowStartConfigMinWeightPercentDefault) {
InSequence s;

setup();

const std::string response1_yaml = R"EOF(
version_info: '0'
resources:
- "@type": type.googleapis.com/envoy.config.cluster.v3.Cluster
name: cluster1
type: EDS
eds_cluster_config:
eds_config:
ads: {}
resource_api_version: V3
service_name: cluster1
round_robin_lb_config:
slow_start_config:
slow_start_window: 30s
)EOF";
auto response1 =
TestUtility::parseYaml<envoy::service::discovery::v3::DiscoveryResponse>(response1_yaml);

EXPECT_CALL(cm_, clusters()).WillOnce(Return(makeClusterInfoMaps({})));
expectAdd("cluster1", "0");
EXPECT_CALL(initialized_, ready());
EXPECT_EQ("", cds_->versionInfo());
const auto decoded_resources =
TestUtility::decodeResources<envoy::config::cluster::v3::Cluster>(response1);
cds_callbacks_->onConfigUpdate(decoded_resources.refvec_, response1.version_info());
EXPECT_EQ("0", cds_->versionInfo());
}

} // namespace
} // namespace Upstream
} // namespace Envoy
Loading