-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
overload: create scaled timers via the dispatcher #14679
Changes from all commits
842d5fa
9d167b1
e39717a
cf94145
4856934
db50f12
c2e548a
58cc3c5
ec55d5f
053a315
4d6d288
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
#pragma once | ||
|
||
#include <chrono> | ||
|
||
#include "common/common/interval_value.h" | ||
|
||
#include "absl/container/flat_hash_map.h" | ||
#include "absl/types/variant.h" | ||
|
||
namespace Envoy { | ||
namespace Event { | ||
|
||
/** | ||
* Describes a minimum timer value that is equal to a scale factor applied to the maximum. | ||
*/ | ||
struct ScaledMinimum { | ||
explicit constexpr ScaledMinimum(UnitFloat scale_factor) : scale_factor_(scale_factor) {} | ||
inline bool operator==(const ScaledMinimum& other) const { | ||
return other.scale_factor_.value() == scale_factor_.value(); | ||
} | ||
const UnitFloat scale_factor_; | ||
}; | ||
|
||
/** | ||
* Describes a minimum timer value that is an absolute duration. | ||
*/ | ||
struct AbsoluteMinimum { | ||
explicit constexpr AbsoluteMinimum(std::chrono::milliseconds value) : value_(value) {} | ||
inline bool operator==(const AbsoluteMinimum& other) const { return other.value_ == value_; } | ||
const std::chrono::milliseconds value_; | ||
}; | ||
|
||
/** | ||
* Class that describes how to compute a minimum timeout given a maximum timeout value. It wraps | ||
* ScaledMinimum and AbsoluteMinimum and provides a single computeMinimum() method. | ||
*/ | ||
class ScaledTimerMinimum { | ||
public: | ||
// Forward arguments to impl_'s constructor. | ||
constexpr ScaledTimerMinimum(AbsoluteMinimum arg) : impl_(arg) {} | ||
constexpr ScaledTimerMinimum(ScaledMinimum arg) : impl_(arg) {} | ||
|
||
// Computes the minimum value for a given maximum timeout. If this object was constructed with a | ||
// - ScaledMinimum value: | ||
// the return value is the scale factor applied to the provided maximum. | ||
// - AbsoluteMinimum: | ||
// the return value is that minimum, and the provided maximum is ignored. | ||
std::chrono::milliseconds computeMinimum(std::chrono::milliseconds maximum) const { | ||
struct Visitor { | ||
explicit Visitor(std::chrono::milliseconds value) : value_(value) {} | ||
std::chrono::milliseconds operator()(ScaledMinimum scale_factor) { | ||
return std::chrono::duration_cast<std::chrono::milliseconds>( | ||
scale_factor.scale_factor_.value() * value_); | ||
} | ||
std::chrono::milliseconds operator()(AbsoluteMinimum absolute_value) { | ||
return absolute_value.value_; | ||
} | ||
const std::chrono::milliseconds value_; | ||
}; | ||
return absl::visit(Visitor(maximum), impl_); | ||
} | ||
|
||
inline bool operator==(const ScaledTimerMinimum& other) const { return impl_ == other.impl_; } | ||
|
||
private: | ||
absl::variant<ScaledMinimum, AbsoluteMinimum> impl_; | ||
}; | ||
|
||
enum class ScaledTimerType { | ||
// Timers created with this type will never be scaled. This should only be used for testing. | ||
UnscaledRealTimerForTest, | ||
// The amount of time an HTTP connection to a downstream client can remain idle (no streams). This | ||
// corresponds to the HTTP_DOWNSTREAM_CONNECTION_IDLE TimerType in overload.proto. | ||
HttpDownstreamIdleConnectionTimeout, | ||
// The amount of time an HTTP stream from a downstream client can remain idle. This corresponds to | ||
// the HTTP_DOWNSTREAM_STREAM_IDLE TimerType in overload.proto. | ||
HttpDownstreamIdleStreamTimeout, | ||
}; | ||
|
||
using ScaledTimerTypeMap = absl::flat_hash_map<ScaledTimerType, ScaledTimerMinimum>; | ||
using ScaledTimerTypeMapConstSharedPtr = std::shared_ptr<const ScaledTimerTypeMap>; | ||
|
||
} // namespace Event | ||
} // namespace Envoy |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,13 @@ Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name) { | |
return std::make_unique<Event::DispatcherImpl>(name, *this, time_system_, watermark_factory_); | ||
} | ||
|
||
Event::DispatcherPtr | ||
Impl::allocateDispatcher(const std::string& name, | ||
const Event::ScaledRangeTimerManagerFactory& scaled_timer_factory) { | ||
return std::make_unique<Event::DispatcherImpl>(name, *this, time_system_, scaled_timer_factory, | ||
watermark_factory_); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying to think about what could be done to avoid this separate API method, although I think this new method is fine. I see that Server::InstanceImpl owns both the api::Impl and the OverloadManager. OverloadManager depends on a dispatcher so it must be created well after api::Impl which is used to allocate the dispatcher. It would be useful to have Api::Impl provide the access to OverloadManager needed by the dispatcher allocation function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that would violate the expectation on API, which is that it is (roughtly) for providing platform-level functionality. Overload management doesn't feel like something on that level. |
||
} | ||
|
||
Event::DispatcherPtr Impl::allocateDispatcher(const std::string& name, | ||
Buffer::WatermarkFactoryPtr&& factory) { | ||
return std::make_unique<Event::DispatcherImpl>(name, *this, time_system_, std::move(factory)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is passing in a factory the right thing, or should we prefer passing in an std::unique_ptr ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(assuming you mean std::unique_ptr) we can't pass in a pointer because the manager is backed by the dispatcher so we can't construct one without constructing the Dispatcher first or at at the same time.