Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
65 commits
Select commit Hold shift + click to select a range
da7f24a
Remove singletons for time-sources and inject them instead.
jmarantz Aug 16, 2018
7d59de4
checkpoint with more stuff compiling, a few tests still fail due to c…
jmarantz Aug 16, 2018
10422d4
checkpoinging: all of //test/common/... compiles, 4 tests still fail …
jmarantz Aug 16, 2018
282307f
//test/common/... all pass.
jmarantz Aug 16, 2018
fa5c8c8
More of //test/... builds, some of it works.
jmarantz Aug 17, 2018
8d67f55
All tests compiling and working.
jmarantz Aug 17, 2018
c9a7d62
More comments and TODOs.
jmarantz Aug 17, 2018
2d9f2aa
Pass the TimeSource into the WorkerFactory's ctor rather than into Wo…
jmarantz Aug 17, 2018
1a84061
Fix race found in coverage tests due to order of destruction. Uncomme…
jmarantz Aug 17, 2018
eb2347e
Don't bother using a whole TimeSource for TokenBucketImpl -- all it n…
jmarantz Aug 17, 2018
21ac418
More API cleanups and TODOs.
jmarantz Aug 17, 2018
0a2514c
Kick CI
jmarantz Aug 17, 2018
c642dbc
Merge branch 'master' into inject-time-source
jmarantz Aug 22, 2018
646a500
formatting.
jmarantz Aug 22, 2018
e65a754
Add comment about preserving entropy in the future, when we move to m…
jmarantz Aug 22, 2018
50dbb33
Merge branch 'master' into inject-time-source
jmarantz Aug 22, 2018
c1f0dba
Add check_format for instantiating time-sources in a new source file …
jmarantz Aug 22, 2018
5338985
Address review comments.
jmarantz Aug 23, 2018
edba780
Revert "Address review comments."
jmarantz Aug 23, 2018
be44f2d
Address review comments.
jmarantz Aug 23, 2018
1488d58
Kick CI
jmarantz Aug 23, 2018
44d880d
Merge branch 'master' into inject-time-source
jmarantz Aug 23, 2018
416917f
Address review comments.
jmarantz Aug 23, 2018
c33f66c
Merge branch 'inject-time-source' into limit-prod-timesource
jmarantz Aug 23, 2018
6d9fac6
Rename 'TestTime' to 'RealTestTime' and add TODO to change all tests …
jmarantz Aug 23, 2018
8855c5a
Merge branch 'inject-time-source' into limit-prod-timesource
jmarantz Aug 23, 2018
c717a28
half-baked TimerFactory concept
jmarantz Aug 24, 2018
944d75b
Don't copy TimeSource objects; it's easier to make them pure virtual …
jmarantz Aug 24, 2018
65d756b
Merge branch 'inject-time-source' into virtual-time-source
jmarantz Aug 24, 2018
970c28f
Makes TimeSource virtual, removing SystemTimeSource and MonotonicTime…
jmarantz Aug 24, 2018
28f41dd
Merge branch 'master' into inject-time-source
jmarantz Aug 24, 2018
b9a9cee
remove timeSource() as a cluster manager API (not compiling all tests…
jmarantz Aug 24, 2018
a285196
Resolve nits.
jmarantz Aug 24, 2018
63fb553
Merge branch 'inject-time-source' into remove-time-source-cluster-man…
jmarantz Aug 24, 2018
4f309ab
Got all tests passing.
jmarantz Aug 24, 2018
6209a53
rename RealTestTime to DangerousDeprecatedTestTime
jmarantz Aug 24, 2018
bc25b73
Merge branch 'remove-time-source-cluster-manager-api' into virtual-ti…
jmarantz Aug 24, 2018
7ea31e8
Merge branch 'master' into remove-time-source-cluster-manager-api
jmarantz Aug 24, 2018
656d40c
Merge branch 'master' into remove-time-source-cluster-manager-api
jmarantz Aug 24, 2018
47c7a4a
Merge branch 'virtual-time-source' into mock-time-for-gzip-test
jmarantz Aug 25, 2018
10ffc99
cleanup
jmarantz Aug 26, 2018
8a636ae
format
jmarantz Aug 26, 2018
200c70e
Merge branch 'master' into remove-time-source-cluster-manager-api
jmarantz Aug 26, 2018
0a6c3b2
Address review comments.
jmarantz Aug 26, 2018
51dd3f1
Merge branch 'remove-time-source-cluster-manager-api' into virtual-ti…
jmarantz Aug 26, 2018
da4f9f3
Having renamed the time-source classes, update the format test to match.
jmarantz Aug 26, 2018
916715c
fix typo
jmarantz Aug 26, 2018
2894051
Merge branch 'master' into virtual-time-source
jmarantz Aug 28, 2018
18f3e97
Fix compile errors due to merge.
jmarantz Aug 28, 2018
6741c72
Kick CI
jmarantz Aug 28, 2018
a6ea21f
Merge branch 'virtual-time-source' into mock-time-for-gzip-test
jmarantz Aug 29, 2018
b7cb5a9
Merge branch 'master' into mock-time-for-gzip-test
jmarantz Aug 29, 2018
88c7afe
Format, test tweaks.
jmarantz Aug 29, 2018
6fcfe70
Merge branch 'master' into mock-time-for-gzip-test
jmarantz Aug 29, 2018
85b28bf
check_format update to incorporate RealTimeSystem.
jmarantz Aug 29, 2018
f11342d
Merge branch 'master' into mock-time-for-gzip-test
jmarantz Aug 29, 2018
89e677a
Merge branch 'master' into mock-time-for-gzip-test
jmarantz Aug 30, 2018
5ecf9a4
rename TimerFactory to Scheduler, and changes for related spellings.
jmarantz Aug 30, 2018
68dfdf4
Merge branch 'master' into mock-time-for-gzip-test, and address revie…
jmarantz Sep 2, 2018
c060eb9
Fix comment.
jmarantz Sep 2, 2018
c5e33c2
Restore timer_impl.h and .cc as files to enable easier re-use later i…
jmarantz Sep 3, 2018
54bbe09
Kick CI
jmarantz Sep 3, 2018
3fe902f
add dispatcher to createScheduler interface, which makes it easier to…
jmarantz Sep 3, 2018
b5d64e4
Revert "add dispatcher to createScheduler interface, which makes it e…
jmarantz Sep 4, 2018
7f3c042
Add TODO to clean up use of duration_cast.
jmarantz Sep 4, 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
3 changes: 2 additions & 1 deletion include/envoy/api/api.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "envoy/common/time.h"
#include "envoy/event/dispatcher.h"
#include "envoy/event/timer.h"
#include "envoy/filesystem/filesystem.h"
#include "envoy/stats/store.h"
#include "envoy/thread/thread.h"
Expand All @@ -24,7 +25,7 @@ class Api {
* @param time_source the time source.
* @return Event::DispatcherPtr which is owned by the caller.
*/
virtual Event::DispatcherPtr allocateDispatcher(TimeSource& time_source) PURE;
virtual Event::DispatcherPtr allocateDispatcher(Event::TimeSystem& time_system) PURE;

/**
* Create/open a local file that supports async appending.
Expand Down
61 changes: 4 additions & 57 deletions include/envoy/common/time.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include "envoy/common/pure.h"

namespace Envoy {

/**
* Less typing for common system time and steady time type.
*
Expand All @@ -14,76 +15,22 @@ namespace Envoy {
typedef std::chrono::time_point<std::chrono::system_clock> SystemTime;
typedef std::chrono::time_point<std::chrono::steady_clock> MonotonicTime;

/**
* Abstraction for getting the current system time. Useful for testing.
*
* TODO(#4160): eliminate this class and pass TimeSource everywhere.
*/
class SystemTimeSource {
public:
virtual ~SystemTimeSource() {}

/**
* @return the current system time.
*/
virtual SystemTime currentTime() PURE;
};

/**
* Abstraction for getting the current monotonically increasing time. Useful for
* testing.
*
* TODO(#4160): eliminate this class and pass TimeSource everywhere.
*/
class MonotonicTimeSource {
public:
virtual ~MonotonicTimeSource() {}

/**
* @return the current monotonic time.
*/
virtual MonotonicTime currentTime() PURE;
};

/**
* Captures a system-time source, capable of computing both monotonically increasing
* and real time.
*
* TODO(#4160): currently this is just a container for SystemTimeSource and
* MonotonicTimeSource but we should clean that up and just have this as the
* base class. Once that's done, TimeSource will be a pure interface.
*/
class TimeSource {
public:
TimeSource(SystemTimeSource& system, MonotonicTimeSource& monotonic)
: system_(system), monotonic_(monotonic) {}
virtual ~TimeSource() {}

/**
* @return the current system time; not guaranteed to be monotonically increasing.
*/
SystemTime systemTime() { return system_.currentTime(); }

virtual SystemTime systemTime() PURE;
/**
* @return the current monotonic time.
*/
MonotonicTime monotonicTime() { return monotonic_.currentTime(); }

/**
* Compares two time-sources for equality; this is needed for mocks.
*/
bool operator==(const TimeSource& ts) const {
return &system_ == &ts.system_ && &monotonic_ == &ts.monotonic_;
}

// TODO(jmarantz): Eliminate these methods and the SystemTimeSource and MonotonicTimeSource
// classes, and change method calls to work directly off of TimeSource.
SystemTimeSource& system() { return system_; }
MonotonicTimeSource& monotonic() { return monotonic_; }

private:
// These are pointers rather than references in order to support assignment.
SystemTimeSource& system_;
MonotonicTimeSource& monotonic_;
virtual MonotonicTime monotonicTime() PURE;
};

} // namespace Envoy
6 changes: 5 additions & 1 deletion include/envoy/event/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ envoy_cc_library(
":deferred_deletable",
":file_event_interface",
":signal_interface",
":timer_interface",
"//include/envoy/common:time_interface",
"//include/envoy/event:timer_interface",
"//include/envoy/filesystem:filesystem_interface",
"//include/envoy/network:connection_handler_interface",
"//include/envoy/network:connection_interface",
Expand All @@ -44,4 +45,7 @@ envoy_cc_library(
envoy_cc_library(
name = "timer_interface",
hdrs = ["timer.h"],
deps = [
"//source/common/event:libevent_lib",
],
)
12 changes: 5 additions & 7 deletions include/envoy/event/dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <string>
#include <vector>

#include "envoy/common/time.h"
#include "envoy/event/file_event.h"
#include "envoy/event/signal.h"
#include "envoy/event/timer.h"
Expand Down Expand Up @@ -35,12 +36,9 @@ class Dispatcher {
/**
* Returns a time-source to use with this dispatcher.
*
* TODO(#4160) the implementations currently manage timer events that
* ignore the time-source, and thus can't be mocked or faked. So it's
* difficult to mock time in an integration test without mocking out
* the dispatcher.
* TODO(#4160) Rename to timeSystem().
*/
virtual TimeSource& timeSource() PURE;
virtual TimeSystem& timeSource() PURE;

/**
* Clear any items in the deferred deletion queue.
Expand Down Expand Up @@ -114,10 +112,10 @@ class Dispatcher {
bool hand_off_restored_destination_connections) PURE;

/**
* Allocate a timer. @see Event::Timer for docs on how to use the timer.
* Allocate a timer. @see Timer for docs on how to use the timer.
* @param cb supplies the callback to invoke when the timer fires.
*/
virtual TimerPtr createTimer(TimerCb cb) PURE;
virtual Event::TimerPtr createTimer(TimerCb cb) PURE;

/**
* Submit an item for deferred delete. @see DeferredDeletable.
Expand Down
32 changes: 32 additions & 0 deletions include/envoy/event/timer.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@
#include <memory>

#include "envoy/common/pure.h"
#include "envoy/common/time.h"

#include "common/event/libevent.h"

namespace Envoy {
namespace Event {

class TimerCB;

/**
* Callback invoked when a timer event fires.
*/
Expand All @@ -34,5 +39,32 @@ class Timer {

typedef std::unique_ptr<Timer> TimerPtr;

class Scheduler {
public:
virtual ~Scheduler() {}

/**
* Creates a timer.
*/
virtual TimerPtr createTimer(const TimerCb& cb) PURE;
};

typedef std::unique_ptr<Scheduler> SchedulerPtr;

/**
* Interface providing a mechanism to measure time and set timers that run callbacks
* when the timer fires.
*/
class TimeSystem : public TimeSource {
public:
virtual ~TimeSystem() {}

/**
* Creates a timer factory. This indirection enables thread-local timer-queue management,
* so servers can have a separate timer-factory in each thread.
*/
virtual SchedulerPtr createScheduler(Libevent::BasePtr&) PURE;
};

} // namespace Event
} // namespace Envoy
1 change: 1 addition & 0 deletions include/envoy/server/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ envoy_cc_library(
":options_interface",
"//include/envoy/access_log:access_log_interface",
"//include/envoy/api:api_interface",
"//include/envoy/event:timer_interface",
"//include/envoy/http:query_params_interface",
"//include/envoy/init:init_interface",
"//include/envoy/local_info:local_info_interface",
Expand Down
7 changes: 0 additions & 7 deletions include/envoy/server/filter_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,6 @@ class FactoryContext {
*/
virtual const envoy::api::v2::core::Metadata& listenerMetadata() const PURE;

/**
* @return SystemTimeSource& a reference to the system time source.
* TODO(#4160): This method should be eliminated, and call-sites and mocks should
* be converted to work with timeSource() below.
*/
virtual SystemTimeSource& systemTimeSource() PURE;

/**
* @return TimeSource& a reference to the time source.
*/
Expand Down
4 changes: 3 additions & 1 deletion include/envoy/server/instance.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include "envoy/access_log/access_log.h"
#include "envoy/api/api.h"
#include "envoy/event/timer.h"
#include "envoy/init/init.h"
#include "envoy/local_info/local_info.h"
#include "envoy/network/listen_socket.h"
Expand Down Expand Up @@ -194,8 +195,9 @@ class Instance {

/**
* @return the time source used for the server.
* TODO(#4160): rename this to timeSystem().
*/
virtual TimeSource& timeSource() PURE;
virtual Event::TimeSystem& timeSource() PURE;

/**
* @return the flush interval of stats sinks.
Expand Down
4 changes: 2 additions & 2 deletions source/common/api/api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
namespace Envoy {
namespace Api {

Event::DispatcherPtr Impl::allocateDispatcher(TimeSource& time_source) {
return Event::DispatcherPtr{new Event::DispatcherImpl(time_source)};
Event::DispatcherPtr Impl::allocateDispatcher(Event::TimeSystem& time_system) {
return Event::DispatcherPtr{new Event::DispatcherImpl(time_system)};
}

Impl::Impl(std::chrono::milliseconds file_flush_interval_msec)
Expand Down
3 changes: 2 additions & 1 deletion source/common/api/api_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <string>

#include "envoy/api/api.h"
#include "envoy/event/timer.h"
#include "envoy/filesystem/filesystem.h"

namespace Envoy {
Expand All @@ -17,7 +18,7 @@ class Impl : public Api::Api {
Impl(std::chrono::milliseconds file_flush_interval_msec);

// Api::Api
Event::DispatcherPtr allocateDispatcher(TimeSource& time_source) override;
Event::DispatcherPtr allocateDispatcher(Event::TimeSystem& time_system) override;
Filesystem::FileSharedPtr createFile(const std::string& path, Event::Dispatcher& dispatcher,
Thread::BasicLockable& lock,
Stats::Store& stats_store) override;
Expand Down
4 changes: 2 additions & 2 deletions source/common/common/perf_annotation.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class PerfAnnotationContext {
absl::string_view description);

/** @return MonotonicTime the current time */
MonotonicTime currentTime() { return time_source_.currentTime(); }
MonotonicTime currentTime() { return time_source_.monotonicTime(); }

/**
* Renders the aggregated statistics as a string.
Expand Down Expand Up @@ -141,7 +141,7 @@ class PerfAnnotationContext {
#else
DurationStatsMap duration_stats_map_;
#endif
ProdMonotonicTimeSource time_source_;
RealTimeSource time_source_;
};

/**
Expand Down
8 changes: 3 additions & 5 deletions source/common/common/token_bucket_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@

namespace Envoy {

TokenBucketImpl::TokenBucketImpl(uint64_t max_tokens, MonotonicTimeSource& monotonic_time_source,
double fill_rate)
TokenBucketImpl::TokenBucketImpl(uint64_t max_tokens, TimeSource& time_source, double fill_rate)
: max_tokens_(max_tokens), fill_rate_(std::abs(fill_rate)), tokens_(max_tokens),
last_fill_(monotonic_time_source.currentTime()),
monotonic_time_source_(monotonic_time_source) {}
last_fill_(time_source.monotonicTime()), time_source_(time_source) {}

bool TokenBucketImpl::consume(uint64_t tokens) {
if (tokens_ < max_tokens_) {
const auto time_now = monotonic_time_source_.currentTime();
const auto time_now = time_source_.monotonicTime();
tokens_ = std::min((std::chrono::duration<double>(time_now - last_fill_).count() * fill_rate_) +
tokens_,
max_tokens_);
Expand Down
5 changes: 2 additions & 3 deletions source/common/common/token_bucket_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ class TokenBucketImpl : public TokenBucket {
* @param fill_rate supplies the number of tokens that will return to the bucket on each second.
* The default is 1.
*/
explicit TokenBucketImpl(uint64_t max_tokens, MonotonicTimeSource& time_source,
double fill_rate = 1);
explicit TokenBucketImpl(uint64_t max_tokens, TimeSource& time_source, double fill_rate = 1);

bool consume(uint64_t tokens = 1) override;

Expand All @@ -28,7 +27,7 @@ class TokenBucketImpl : public TokenBucket {
const double fill_rate_;
double tokens_;
MonotonicTime last_fill_;
MonotonicTimeSource& monotonic_time_source_;
TimeSource& time_source_;
};

} // namespace Envoy
18 changes: 5 additions & 13 deletions source/common/common/utility.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,13 @@ class AccessLogDateTimeFormatter {
};

/**
* Production implementation of SystemTimeSource that returns the current time.
* Real-world time implementation of TimeSource.
*/
class ProdSystemTimeSource : public SystemTimeSource {
class RealTimeSource : public TimeSource {
public:
// SystemTimeSource
SystemTime currentTime() override { return std::chrono::system_clock::now(); }
};

/**
* Production implementation of MonotonicTimeSource that returns the current time.
*/
class ProdMonotonicTimeSource : public MonotonicTimeSource {
public:
// MonotonicTimeSource
MonotonicTime currentTime() override { return std::chrono::steady_clock::now(); }
// TimeSource
SystemTime systemTime() override { return std::chrono::system_clock::now(); }
MonotonicTime monotonicTime() override { return std::chrono::steady_clock::now(); }
};

/**
Expand Down
6 changes: 2 additions & 4 deletions source/common/config/grpc_mux_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,9 @@ GrpcMuxWatchPtr GrpcMuxImpl::subscribe(const std::string& type_url,
// TODO(gsagula): move TokenBucketImpl params to a config.
if (!api_state_[type_url].subscribed_) {
// Bucket contains 100 tokens maximum and refills at 5 tokens/sec.
api_state_[type_url].limit_request_ =
std::make_unique<TokenBucketImpl>(100, time_source_.monotonic(), 5);
api_state_[type_url].limit_request_ = std::make_unique<TokenBucketImpl>(100, time_source_, 5);
// Bucket contains 1 token maximum and refills 1 token on every ~5 seconds.
api_state_[type_url].limit_log_ =
std::make_unique<TokenBucketImpl>(1, time_source_.monotonic(), 0.2);
api_state_[type_url].limit_log_ = std::make_unique<TokenBucketImpl>(1, time_source_, 0.2);
api_state_[type_url].request_.set_type_url(type_url);
api_state_[type_url].request_.mutable_node()->MergeFrom(local_info_.node());
api_state_[type_url].subscribed_ = true;
Expand Down
2 changes: 1 addition & 1 deletion source/common/config/grpc_mux_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class GrpcMuxImpl : public GrpcMux,
std::list<std::string> subscriptions_;
Event::TimerPtr retry_timer_;
Runtime::RandomGenerator& random_;
TimeSource time_source_;
TimeSource& time_source_;
BackOffStrategyPtr backoff_strategy_;
};

Expand Down
Loading